Draft
Conversation
Tested with clang-18.1.6 on Fedora 40. According https://github.com/google/sanitizers/wiki/MemorySanitizer#using-instrumented-libraries all the code in the program must be built with it, otherwise false positives are reported. Disable zstd when compiling with the memory sanitizer, because this is an external library, that is unlikely to have been compiled with it. Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Contributor
Author
|
clang14 from Ubuntu22 might be too old for this, there is a false positive from calling |
Member
|
(Is this still a draft?) |
Contributor
Author
|
Yes, I have a private/edvint/msan-doc-only.2 that is an update of this one, but it is not quite ready yet (in that branch I split the sanitizer script into 3, one for each sanitizer, but that makes it difficult to review, so I'll add some CLI flags/env vars instead). I'll try to find some time to work on this later this week. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split from #13290.
Back in 2018 tools/ci/inria/sanitizers/script claimed that MSAN output is impossible to debug.
clang has improved a lot since then, and in fact with clang-18 on Fedora40 it only reports 2 bug, both of which I was able to track down.
First of all zstd has to be disabled with -fsanitize=memory, because all external libraries would have to be compiled with -fsanitize=memory, otherwise the instrumentation won't see the writes performed by these and consider all memory written to by these functions uninitialized.
With these 2 changes the testsuite now passes.
I've included some compile flag improvements as well, the manual recommends -O1 or higher, not -O0.
ocamlopt has to be kept disabled, it'd need a similar solution to TSAN to enable it (ocamlopt would have to emit calls to instrumentation functions compatible with Clang's, otherwise it won't see any of the writes from OCaml and consider them uninitialized).
I've kept the memory sanitizer commented out for now, but if desired it could be enabled.
Draft, because it conflicts with the other sanitizer CI PRs and needs to be rebased once they are merged.