Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add add sanitizers to unit test cmake build #205

Merged
merged 8 commits into from
Nov 3, 2022

Conversation

jvishnefske
Copy link
Contributor

@jvishnefske jvishnefske commented Nov 1, 2022

  • currently enabled on 64 bit unit test build. (edit: sanitizers added to x32 build by beaf0a8)
  • silently falls back if -fsanitize=undefined support is not detected
  • verifies bug-203 fix

@jvishnefske
Copy link
Contributor Author

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
Copy link
Contributor Author

jvishnefske commented Nov 2, 2022

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

  • Clang currently has a link issue blocking this. I will have another go at it tomorrow. (resolved by beaf0a8)

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

tests/CMakeLists.txt Outdated Show resolved Hide resolved
@pavel-kirienko
Copy link
Member

Please tag me when ready for review.

@jvishnefske
Copy link
Contributor Author

@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.

@pavel-kirienko pavel-kirienko merged commit 5c69d45 into OpenCyphal:master Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

-fsanitize=undefined runtime error: member access within null pointer of type 'struct TxItem'
2 participants