idlc fails assert and aborts on delete_const_expr

[Aposhian]

Version: Reproducible on both master and 0.10.3
I have built idlc in debug mode and in release mode.

I am using the cmake function idlc_generate to do code generation for a long list of IDL files, some of which include others. I keep getting these aborts, but inconsistently. It doesn't consistently fail on the same messages. I have yet to reproduce it with just a single ad-hoc invocation, even using the exact command that failed when invoked by CMake. So I'm suspecting it has to do with many repeated invocations, although I'm not sure why. I've reproduced it with both a clean output directory and an output directory with generated files already in it.

I'm not familiar with how this part of the code works, but I'd be happy to grab more info or relevant variable states, since I can reproduce it in debug mode. I wish I could make a minimal reproducible example but I don't know what is triggering it.

The commands that idlc_generate is running look like this (I am using colcon to invoke CMake).

/home/u/workspace/install/cyclonedds/bin/idlc -Wno-implicit-extensibility -b/path/to/idl -o/home/u/workspace/build/my_package -I /path/to/idl /path/to/idl/std_msgs/msg/Time.idl"

Stacktrace:

#0  __pthread_kill_implementation (no_tid=0, signo=6, threadid=140620224417664) at ./nptl/pthread_kill.c:44
#1  __pthread_kill_internal (signo=6, threadid=140620224417664) at ./nptl/pthread_kill.c:78
#2  __GI___pthread_kill (threadid=140620224417664, signo=signo@entry=6) at ./nptl/pthread_kill.c:89
#3  0x00007fe4b2642476 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#4  0x00007fe4b26287f3 in __GI_abort () at ./stdlib/abort.c:79
#5  0x00007fe4b262871b in __assert_fail_base (fmt=0x7fe4b27dd130 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", 
    assertion=0x7fe4b28d86f9 "next == const_expr", file=0x7fe4b28d8620 "/home/u/workspace/src/cyclonedds/src/idl/src/tree.c", 
    line=343, function=<optimized out>) at ./assert/assert.c:92
#6  0x00007fe4b2639e96 in __GI___assert_fail (assertion=0x7fe4b28d86f9 "next == const_expr", 
    file=0x7fe4b28d8620 "/home/u/workspace/src/cyclonedds/src/idl/src/tree.c", line=343, 
    function=0x7fe4b28da0a0 <__PRETTY_FUNCTION__.200> "delete_const_expr") at ./assert/assert.c:101
#7  0x00007fe4b28b901b in delete_const_expr (node=0x559210346d20, const_expr=0x559210346c80)
    at /home/u/workspace/src/cyclonedds/src/idl/src/tree.c:343
#8  0x00007fe4b28c209c in delete_annotation_appl_param (ptr=0x559210346d20)
    at /home/u/workspace/src/cyclonedds/src/idl/src/tree.c:3397
#9  0x00007fe4b28b915f in idl_delete_node (ptr=0x559210346d20) at /home/u/workspace/src/cyclonedds/src/idl/src/tree.c:369
#10 0x00007fe4b28b90ac in idl_delete_node (ptr=0x559210346b40) at /home/u/workspace/src/cyclonedds/src/idl/src/tree.c:357
#11 0x00007fe4b28c2367 in delete_annotation_appl (ptr=0x559210346a00)
    at /home/u/workspace/src/cyclonedds/src/idl/src/tree.c:3460
#12 0x00007fe4b28b915f in idl_delete_node (ptr=0x559210346a00) at /home/u/workspace/src/cyclonedds/src/idl/src/tree.c:369
#13 0x00007fe4b28c6f2a in yydestruct (yymsg=0x7fe4b28dceac "Cleanup: popping", yykind=YYSYMBOL_annotations, yyvaluep=0x559210326aa0, 
    yylocationp=0x559210327890, pstate=0x559210326760, result=0x7ffdfbea20fc) at src/parser.y:216
#14 0x00007fe4b28ccfb7 in idl_yypush_parse (yyps=0x559210326830, yypushed_char=123, yypushed_val=0x7ffdfbea2100, yypushed_loc=0x7ffdfbea2170, 
    pstate=0x559210326760, result=0x7ffdfbea20fc) at /home/adam/firefly/m220a_robot_ws/build/cyclonedds/src/idl/parser.c:3693
#15 0x00007fe4b28ae26e in parse_grammar (pstate=0x559210326760, tok=0x7ffdfbea2150)
    at /home/u/workspace/src/cyclonedds/src/idl/src/processor.c:328
#16 0x00007fe4b28aeb70 in idl_parse (pstate=0x559210326760) at /home/u/workspace/src/cyclonedds/src/idl/src/processor.c:536
#17 0x000055920e9ffbfc in idlc_putn (str=0x7ffdfbea22c6 "p", len=1)
    at /home/u/workspace/src/cyclonedds/src/tools/idlc/src/idlc/idlc.c:100
#18 0x000055920e9fff41 in idlc_putc (chr=112, od=MCPP_OUT)
    at /home/u/workspace/src/cyclonedds/src/tools/idlc/src/idlc/idlc.c:150
#19 0x000055920ea08dee in parse_line () at /home/u/workspace/src/cyclonedds/src/tools/idlpp/src/support.c:1719
#20 0x000055920ea087fe in get_ch () at /home/u/workspace/src/cyclonedds/src/tools/idlpp/src/support.c:1580
#21 0x000055920ea03b0b in mcpp_main () at /home/u/workspace/src/cyclonedds/src/tools/idlpp/src/main.c:637
#22 0x000055920ea037b1 in mcpp_lib_main (argc=10, argv=0x559210326540)
    at /home/u/workspace/src/cyclonedds/src/tools/idlpp/src/main.c:430
#23 0x000055920ea009f3 in idlc_parse (generator_annotations=0x0)
    at /home/u/workspace/src/cyclonedds/src/tools/idlc/src/idlc/idlc.c:375
#24 0x000055920ea018b6 in main (argc=7, argv=0x7ffdfbea2a48)
    at /home/u/workspace/src/cyclonedds/src/tools/idlc/src/idlc/idlc.c:772

[Aposhian]

Ok, I should have looked closer at the error messages right before this happened:

./idl/std_msgs/msg/Header.idl:13:5: Declaration 'Header_' collides with earlier an declaration of 'Header_'
idlc: ./src/idl/src/tree.c:343: delete_const_expr: Assertion `next == const_expr' failed.
[1]    671226 IOT instruction (core dumped)  idlc -Iidl -bidl -o output ./idl/nav_msgs/msg/Path.idl

So it looks like the problem is and IDL message being included multiple times. Adding include C-style include guards fixed the issue.

Would it be worth adding a section to the documentation about what preprocessor directives are used and can be useful? Just because I had #include in my IDL files, I didn't automatically assume that I also needed to add include guards with #ifndef and #define.

[eboasson]

Adding include C-style include guards fixed the issue.

That's good! It is not good that it crashes, so I would like the crash to be fixed. Unfortunately I haven't managed to reproduce it quickly by doing some obvious permutations of multiple definitions for a very simple type. I probably should try it with a ROS message or two, but perhaps you have some further insight as to what triggers it.

Would it be worth adding a section to the documentation about what preprocessor directives are used and can be useful?

It uses a pretty standard-compliant C99 preprocessor, so anything that is supported by a C99 preprocessor should work. It predefines __IDLC__, __IDLC_MINOR__ and __IDLC_PATCHLEVEL__, and I think that's about it.

But yes, it would be good for the documentation to point this out!

Just because I had #include in my IDL files, I didn't automatically assume that I also needed to add include guards with #ifndef and #define.

I suppose on our side everyone assumed that everyone knew that IDL doesn't allow redefinitions ... I think it would've been fine if the error message hadn't been obscured by the crash, but recommending the use of include guards in the documentation would no doubt be a good addition.

[Aposhian]

I should be able to come up with a simple reproduction I think

[Aposhian]

Here is a reproduction of the bug: https://github.com/fireflyautomatix/repro-cyclonedds-1911

[eboasson]

Thank you so much! After a bit of digging, I've managed to reduce it to this:

struct a { long b; };
@verbatim(text="y")
struct a { long b; };

But then I found another few interesting ones ... this also fails:

@annotation x { long y; };
struct a { long b; };
const long z = 1;
@x(y=1)
struct a { long b; };

but this doesn't, and I think that's because while the constant "x" and the constant 1 are not definitions and never got their parent assigned, here it references a definition and those are treated a bit differently:

@annotation x { long y; };
struct a { long b; };
const long z = 1;
@x(y=z)
struct a { long b; };

With that observation, one would think:

struct a { long b; };
const string y = "y";
@verbatim(text=y)
struct a { long b; };

should be ok, too, and indeed it is. I need to study this a bit more, but I have at least some idea now ...