search

paulscherrerinstitute / StreamDevice

EPICS Driver for message based I/O
GNU General Public License v3.0
28 stars 42 forks source link

redirection-to-records fails when PV name starts with a number #20

Closed kmpeters closed 5 years ago

kmpeters commented 5 years ago

I wrote stream support for a pyrometer. The protocol file, Metis_M322.proto, contains a function that reads in three temperatures: 

getBuff01Data
{
  out "\$1bup";
  in "%04x%(\$2:temp2Raw)04x%(\$2:temp2cRaw)04x";
}

This is the relevant portion of Metis_M322.db:

record(ai, "$(P)$(R):temp1:01") {
  field(DTYP, "stream")
  field(INP,  "@Metis_M322.proto getBuff01Data($(A),$(P)$(R)) $(PORT)")
  field(SCAN, "Passive")
  field(LINR, "SLOPE")
  field(ESLO, "0.1")
  field(EOFF, "0.0")
  field(PREC, "3")
  field(FLNK, "$(P)$(R):temp1")
}
record(longout, "$(P)$(R):temp2Raw") {}
record(longout, "$(P)$(R):temp2cRaw") {}

If P=kmp3:, then temp2Raw and temp2cRaw update each time temp1:01 processes. If P=3kmp:, then temp2Raw and temp2cRaw fail to update when temp1:01 processes.

I can recreate the problem the following versions of EPICS modules:

dirk-zimoch commented 5 years ago

Hi Kevin, Thanks for the bug report. I have to admit that I have never tried such a name.

dirk-zimoch commented 5 years ago

Fixed in commit 493dc19.

kmpeters commented 5 years ago

That commit fixed the problem for me on Linux with base-3.14.12.5. On Windows with base-3.15.5, an IOC with a prefix that starts with a number crashes at iocInit if it includes a database that uses the redirect-to-records feature.

jlmais commented 5 years ago

Hi Kevin and Dirk, Few hours ago I had found a similar problem, but the Stream Device with calcout records. In my case I have a PV name starting by "LA", the same name of the calcout field. Then I realized that any PV name starting with any record field name cause the problem. For example, "LA-BIH01RACK2:TI-EVE:", "LB-BIH01RACK2:TI-EVE:" or "INPA-BIH01RACK2:TI-EVE:". The commit mentioned above fixed the problem.

dirk-zimoch commented 5 years ago

It seems that dbNameToAddr() only parses the field name as long as it matches [_a-zA-Z][_a-zA-Z0-9]* and ignores anything from a non-matching character on. In Kevin's case it was the leading number, in Joao's case it was the minus. It then matches this part against the fields, empty string being equivalent to the VAL field or "LA" being a valid field name of a calcout. As a result, the redirection used a local field instead of the other record. The fix now does not believe the success return value any more but cross checks it the found field is really what we were looking for. Sorry that I have not seen that earlier. Our prefixes are typically 5 chars long and never start with a number. Thus this case never happened here and I never suspected any problems with dbNameToAddr() giving false positives.

dirk-zimoch commented 5 years ago

On Windows with base-3.15.5, an IOC with a prefix that starts with a number crashes at iocInit if it includes a database that uses the redirect-to-records feature.

Only on Windows? I need to test that. May take a while because I am not normally using Windows.

kmpeters commented 5 years ago

I've only seen the crash on Windows so far.

The IOC does not crash on Linux (linux-x86_64 on RHEL7) with base-3.15.5; IOCs with and without PV prefixes that start with a number both work as expected.

I haven't done any testing with VxWorks.

dirk-zimoch commented 5 years ago

Can you find out where it crashes? I am not familiar with debugging on Windows.

kmpeters commented 5 years ago

If I can successfully build stream for windows-x64-debug, I should be able to find out where the IOC crashes. Currently the build fails with this error:

cl -EHsc -GR                -nologo -D__STDC__=0 -D_CRT_SECURE_NO_DEPRECATE -D_CRT_NONSTDC_NO_DEPRECATE -DUSE_TYPED_RSET   -RTCsu -Zi   -W3 -w44355 -w44344
   -MDd -DEPICS_BUILD_DLL -DEPICS_CALL_DLL -TP  -I. -I../O.Common -I. -I. -I.. -I../../../include/compiler/msvc -I../../../include/os/WIN32 -I../../../include
    -ID:/epics/synApps_6_0-10/support/asyn-R4-33/include   -ID:/epics/synApps_6_0-10/support/calc-R3-7-1/include   -ID:/epics/synApps_6_0-10/support/sscan-R2-11
-1/include   -ID:/epics/synApps_6_0-10/support/seq-2-2-5/include -ID:/epics/base-3.15.5-10/include/compiler/msvc -ID:/epics/base-3.15.5-10/include/os/WIN32 -ID:
/epics/base-3.15.5-10/include        -c ../DebugInterface.cc
DebugInterface.cc
d:\epics\synapps_6_0-10\support\stream-git\streamdevice\src\StreamBusInterface.h(29) : error C2051: case expression not constant
d:\epics\synapps_6_0-10\support\stream-git\streamdevice\src\StreamBusInterface.h(29) : warning C4065: switch statement contains 'default' but no 'case' labels
D:/epics/base-3.15.5-10/configure/RULES_BUILD:233: recipe for target 'DebugInterface.obj' failed
make[3]: *** [DebugInterface.obj] Error 2
make[3]: Leaving directory 'D:/epics/synApps_6_0-10/support/stream-git/StreamDevice/src/O.windows-x64-debug'
D:/epics/base-3.15.5-10/configure/RULES_ARCHS:58: recipe for target 'install.windows-x64-debug' failed
make[2]: *** [install.windows-x64-debug] Error 2
make[2]: Leaving directory 'D:/epics/synApps_6_0-10/support/stream-git/StreamDevice/src'
D:/epics/base-3.15.5-10/configure/RULES_DIRS:84: recipe for target 'src.install' failed
make[1]: *** [src.install] Error 2
make[1]: Leaving directory 'D:/epics/synApps_6_0-10/support/stream-git/StreamDevice'
configure/RULES_DIRS:88: recipe for target 'StreamDevice.install' failed
make: *** [StreamDevice.install] Error 2

D:\epics\synApps_6_0-10\support\stream-git>
dirk-zimoch commented 5 years ago

Hmm... It seems building for windows-x64-debug does not like my MacroMagic. That's a different issue though. Also I found a bug that makes StreamDevice crash on Windows which may or may not be related to this crash.

jlmais commented 5 years ago

It seems that dbNameToAddr() only parses the field name as long as it matches [_a-zA-Z][_a-zA-Z0-9]* and ignores anything from a non-matching character on.

Exactly.

dirk-zimoch commented 5 years ago

I have tested redirects to records beginning with numbers now on windows-x64 with EPICS releases 3.15.5 and 3.16.1. I do not get a crash. So please try again with the current commit which fixed a crash bug that had nothing to do with redirects.

kmpeters commented 5 years ago

I've updated stream to 9ef1653e73d26c489aacc873f8f5c7c461fced7b, and my Windows IOC (base-3.15.5, windows-x64-static) no longer crashes when the PV prefix begins with a number and a record that uses the redirect-to-record feature is processed. This issue can be closed. Thanks!

dirk-zimoch commented 5 years ago

Tagged as 2.8.7