Skip to content

Fix false positives in constant time tests using MSan with Clang 16 #9942

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

Open
wants to merge 3 commits into
base: development
Choose a base branch
from

Conversation

gilles-peskine-arm
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm commented Jan 29, 2025

Fix false positives of constant-flow testing when using MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN on Clang 16 and above. Fixes #9921.

This isn't needed for the CI (in the short to medium term) because still we do this testing on an old platform with an old Clang, but is needed on typical developer machines nowadays.

PR checklist

New CMake build types CFMemSan, CFMemSanDbg to take care of differing
compiler command lines with Clang <15 and Clang >=16.

Signed-off-by: Gilles Peskine <[email protected]>
Support Clang >=16 in component_test_memsan_constant_flow and
component_test_memsan_constant_flow_psa. Modern versions of Clang require an
extra compiler option to avoid false positives.

Fixes Mbed-TLS#9921.

Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
@gilles-peskine-arm gilles-peskine-arm added needs-backports Backports are missing or are pending review and approval. needs-ci Needs to pass CI tests component-test Test framework and CI scripts priority-medium Medium priority - this can be reviewed as time permits size-xs Estimated task size: extra small (a few hours at most) labels Jan 29, 2025
@gilles-peskine-arm gilles-peskine-arm changed the title Msancf clang16 dev Fix false positives in constant time tests using MSan with Clang 16 Jan 29, 2025
@@ -286,10 +286,23 @@ function(set_clang_base_compile_options target)
set_target_properties(${target} PROPERTIES LINK_FLAGS_ASAN "-fsanitize=address -fsanitize=undefined")
target_compile_options(${target} PRIVATE $<$<CONFIG:ASanDbg>:-fsanitize=address -fno-common -fsanitize=undefined -fno-sanitize-recover=all -O1 -g3 -fno-omit-frame-pointer -fno-optimize-sibling-calls>)
set_target_properties(${target} PROPERTIES LINK_FLAGS_ASANDBG "-fsanitize=address -fsanitize=undefined")

set(sanitize_memory_debug_flags "-O1 -g3 -fno-omit-frame-pointer -fno-optimize-sibling-calls -fsanitize-memory-track-origins=2")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to reviewers: the CMake code is completely different in 3.6. I suspect what I wrote here isn't very idiomatic CMake, but I don't know how to do better without significant changes to what was already there.

@gilles-peskine-arm gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members, and removed needs-ci Needs to pass CI tests labels Jan 30, 2025
Copy link
Contributor

@waleed-elmelegy-arm waleed-elmelegy-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component-test Test framework and CI scripts needs-backports Backports are missing or are pending review and approval. needs-review Every commit must be reviewed by at least two team members, priority-medium Medium priority - this can be reviewed as time permits size-xs Estimated task size: extra small (a few hours at most)
Projects
Status: In Development
Development

Successfully merging this pull request may close these issues.

False positives in constant time tests using MSan with Clang 16
2 participants