search

mittinatten / freesasa

C-library for calculating Solvent Accessible Surface Areas
http://freesasa.github.io/
MIT License
107 stars 36 forks source link

memerr tests fail because a malloc call is replaced by a built-in #94

Closed dtelsing closed 1 year ago

dtelsing commented 1 year ago

The memerr tests rely on replacing malloc with a wrapper which fails after a certain number of calls. If the malloc call is replaced by the compiler however, this breaks some tests.

When building with gcc version 11.3.0 for i686-unknown-linux-gnu with -O2 optimizations, this lead test_memerr in tests/test_selection.c to dereference a null pointer, because select_atoms (in src/selection.c) continues further than intended and dereferences selection_dummy.atom (==NULL).

Therefore, it would be good to build the library with -fno-builtin-malloc or build the tests separately with that flag enabled.

mittinatten commented 1 year ago

Yes, I have also noticed that compilers/linkers have gotten pickier about this since I wrote the test 7 years ago. Thanks for your suggestions. I think I will hide these tests behind a configure flag (--enable-memerr-tests) and add some directions to the docs.

Either way, since this is not systems code, the functionality that is verified by these tests isn't that crucial either, so I have also considered simply removing these tests.

mittinatten commented 1 year ago

I added the flag and far as I can tell it hides all the tests, it also avoids compiling the helper code that does the replacement.

I think using the flag -fno-builtin-malloc could be beneficial, but I don't know if it's worth it to set up a complete separate build for the tests.