-
Notifications
You must be signed in to change notification settings - Fork 1k
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
False positives in constant-time tests when using MSan on Clang >= 16 #1516
Labels
Comments
real-or-random
added a commit
that referenced
this issue
May 27, 2024
ebfb82e ci: Add job with -fsanitize-memory-param-retval (Tim Ruffing) e1bef09 configure: Move "experimental" warning to bottom (Tim Ruffing) 55e5d97 autotools: Disable eager MSan in ctime_tests (Tim Ruffing) Pull request description: This is the autotools solution for #1516. Alternatively, we could have a full-blown `--enable-msan` option, but it's more work, and I'm not convinced that it's necessary or at least much better. hebasto If you're Concept ACK, are you willing to work on an equivalent PR for CMake? ACKs for top commit: hebasto: ACK ebfb82e, tested on Ubuntu 24.04 with different clang versions (from 15 to 18) and different build configurations. CI changes look OK as well. Tree-SHA512: c083d778fd50bd35c2e29b7fe0d92b98d912ee5ac7809ae73067d050a0d3c42b3483260f1286d0023cdb802a3c3006bf932ecf60ce81b942de1c9824374c0132
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
(Description partly copied from #1512 (comment))
The default of what is considered a "use" of uninitialized memory was changed in clang 16. Returning an uninitialized variables from a function, or passing uninitialized values to a function as a parameter is now considered, and MSan will report it by default. See the Clang 16.0.09 Release Notes:
This makes sense for actual memory checking, in the sense that uninitialized memory at the boundary of a function call should probably be considered a bug or at least a smell. But that's certainly not what we want if we (ab)use MSan for constant-time checking, and this gives us false positives.
We should pass
-fno-sanitize-memory-param-retval
to clang >=16, but probably only for thectime_test
target.Example false positives, `clang version 17.0.6` on d831168, and `./configure --enable-dev-mode CFLAGS='-fsanitize=memory -fsanitize-recover=memory -g'`:
cc @hebasto
The text was updated successfully, but these errors were encountered: