OpenCyphal / libcanard

A compact implementation of the Cyphal/CAN protocol in C for high-integrity real-time embedded systems
http://opencyphal.org
MIT License
327 stars 192 forks source link

add add sanitizers to unit test cmake build #205

Closed jvishnefske closed 1 year ago

jvishnefske commented 1 year ago
jvishnefske commented 1 year ago

I am attempting to build with and without including the FIX-204 to see if the issue shows up in the log. currently a sanitizer runtime warning does not cause the unit test to fail.

jvishnefske commented 1 year ago

I was able to get the cmake unit tests to fail from an actual undefined behavior on gcc.

the 32 bit version of the symbol needed to enable undefined sanitizer on clang appear to be avaliable as a static library on ubuntu docker.

user@80a9238469f6:/usr/lib32$ grep -r __mulodi4 /usr Binary file /usr/lib/x86_64-linux-gnu/libLLVM-12.so.1 matches Binary file /usr/lib/llvm-12/lib/clang/12.0.0/lib/linux/libclang_rt.builtins-i386.a matches Binary file /usr/lib/llvm-12/lib/clang/12.0.0/lib/linux/libclang_rt.builtins-x86_64.a matches

pavel-kirienko commented 1 year ago

Please tag me when ready for review.

jvishnefske commented 1 year ago

@pavel-kirienko the basic concept here seems to be working, and was able to fail on the #203 issue in my local tests. I considered adding support with toolchain files, however that would have been (possbily)even more invasive since the entire build matrix would have to be broken into several toolchain files. This implementation uses runtime compile checks to query the compiler for basic support, and then sets the resulting binary to fail if any of a list of checks are encountered. The disadvantage of this approach is that compilers which are new enough to support -fsanitize=undefined, but do not support one of the long list of "fatal" checks may have compile errors. since the cmake build is recomended to only be used for unit tests. I don't know if this is a disadvantage for any use case.