clicon / cligen

CLIgen is a Command-Line Interface generator
http://www.cligen.se
Other
33 stars 37 forks source link

Compiling with `-DNDEBUG` results in failure of test applications. #76

Closed rcmcdonald91 closed 2 years ago

rcmcdonald91 commented 2 years ago

I stumbled onto an interesting bug related to the use of asserts() and the -DNDEBUG compiler flag. I have been experimenting with using CMake to build Cligen (you can find that attempt here: https://github.com/theonemcdonald/cligen/tree/cmake).

The CMake default CFLAGS for each build type are as follows:

CMAKE_C_FLAGS_DEBUG: -g
CMAKE_C_FLAGS_RELEASE: -O3 -DNDEBUG
CMAKE_C_FLAGS_RELWITHDEBINFO: -O2 -g -DNDEBUG
CMAKE_C_FLAGS_MINSIZEREL: -Os -DNDEBUG

The failure case can be readily observed by compiling Cligen with -DNDEBUG and running the cligen_hello test application. In this case, typing hello<tab><tab><tab>... causes Cligen to just keep repeating the hello keyword:

image

building without -DNDEBUG yields the correct behavior:

image

The issue appears to be related to an assert() call having undesired side-effects.

After consulting with Olof, we determined this to be a likely culprit that should be explored further:

https://github.com/clicon/cligen/blob/9e6f5ad5568d3fdbb1f9e0a585bffbff0204c700/cligen_match.c#L238

cligen_match.c:239:18: warning: variable 'levels' is uninitialized when used here [-Wuninitialized]
    if (level >= levels)
                 ^~~~~~
cligen_match.c:236:15: note: initialize the variable 'levels' to silence this warning
    int levels;
              ^
               = 0
olofhagsand commented 2 years ago

Should be fixed by the patch, please verify

rcmcdonald91 commented 2 years ago

Verified. This looks good now.