Snaipe / Criterion

A cross-platform C and C++ unit testing framework for the 21st century
MIT License
2.03k stars 182 forks source link

malloc in messages.c #303

Open kugel- opened 5 years ago

kugel- commented 5 years ago

There is a malloc() call in messages.c. Is this appropriate or should cr_malloc() be used?

I'm asking because one of my tests interposes malloc() and my test is acting weird (seems to freeze, and when I want to check in gdb the inferior process is SIGKILLed once I hit ctrl+c.

https://github.com/Snaipe/Criterion/blob/cee1a47f4547b9e51c460705986c9fd353b96269/src/protocol/messages.c#L66

Snaipe commented 5 years ago

It's technically appropriate -- cr_malloc is for memory allocated in the runner that need to be inherited in the test workers -- but indeed questionable for cases like these.

Maybe we should be instantiating and using a dedicated arena to send messages here. After all, the facilities are already there.

kugel- commented 4 years ago

Coming back to this, do you think it will be fixed? In the meantime I figured out that -Os apparently makes a difference (-Os crashes the test deep inside criterion, without -Ox it seems to work fine)


#0  0x00007ffff70b4f9f in buf_write () from /usr/local/lib/libcriterion.so.3
#1  0x00007ffff70b50aa in pb_write () from /usr/local/lib/libcriterion.so.3
#2  0x00007ffff70b5acd in pb_encode_varint () from /usr/local/lib/libcriterion.so.3
#3  0x00007ffff70b5ba4 in pb_encode_tag () from /usr/local/lib/libcriterion.so.3
#4  0x00007ffff70b5c58 in pb_encode_tag_for_field () from /usr/local/lib/libcriterion.so.3
#5  0x00007ffff70b553e in encode_basic_field () from /usr/local/lib/libcriterion.so.3
#6  0x00007ffff70b5776 in encode_field () from /usr/local/lib/libcriterion.so.3
#7  0x00007ffff70b596a in pb_encode () from /usr/local/lib/libcriterion.so.3
#8  0x00007ffff70b2d28 in write_message () from /usr/local/lib/libcriterion.so.3
#9  0x00007ffff70b2e3f in cr_send_to_runner () from /usr/local/lib/libcriterion.so.3
#10 0x00007ffff70ad90c in criterion_send_assert () from /usr/local/lib/libcriterion.so.3
#11 0x0000000000400f03 in overlay_libc_impl () at hosttools/tests/overlaytest.c:164
#12 0x00007ffff70a8216 in criterion_internal_test_main () from /usr/local/lib/libcriterion.so.3
#13 0x0000000000400d6f in overlay_libc_jmp () at hosttools/tests/overlaytest.c:156
#14 0x00007ffff70a6971 in run_test_child () from /usr/local/lib/libcriterion.so.3
#15 0x00007ffff70f1e4b in bxfi_main () from /usr/local/lib/libcriterion.so.3
#16 0x00007ffff6d1e2e1 in __libc_start_main (main=0x400b80 <main@plt>, argc=1, argv=0x7fffffffe918, init=<optimized out>,
    fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffe908) at ../csu/libc-start.c:291
#17 0x0000000000400bfa in _start ()
kugel- commented 4 years ago

-O2 also seems to work but it may be coincidence.

Snaipe commented 2 years ago

To update this, I'm not actively looking at replacing the malloc calls in the assertion calls. It's hard to not rely on vital functions like that without massively increasing the complexity. The only workaround I can think of is to ensure that the interposition stops before calling the assertions.