nasa / LC

The Core Flight System (cFS) Limit Checker (LC) application.
Apache License 2.0
30 stars 21 forks source link

LC "IsNAN" check relies on platform-defined behavior (non-standard) #57

Closed jphickey closed 1 year ago

jphickey commented 1 year ago

Checklist (Please check before submitting)

Describe the bug With floating point watchpoints, the watch point checking code looks for the IEEE754 not-a-number (NaN) value before doing other comparisons.

However, the method used for checking this involves accessing the float value as a uint32, and checking the bits per IEEE754.

This type of action is not standardized by the C language, and results of doing this are platform-defined.

To Reproduce N/A (it does work as intended on most compilers, it is just not standard or portable "by the book")

Expected behavior Should only rely on behavior that is specified by ISO C.

Regarding NaN, the C standard does guarantee that a NaN is never equal to any other value, even itself. Therefore, the generally recommended, portable way to check for NaN is by checking that, e.g.:

if (value == value)
{
    /* value is valid */
}
else
{
    /* value is NaN */
}

Code snips Code at issue is here: https://github.com/nasa/LC/blob/358fc484e93d0c2c63e4a2faf1d61883962f0c0b/fsw/src/lc_watch.c#L1071-L1097

System observed on: Code Inspection when doing EDS implementation

Reporter Info Joseph Hickey, Vantage Systems, Inc.