Squash MSAN false positives

[jonathanmetzman]

Some of these may have been caused by the compiler roll.
CC @maflcko

[jonathanmetzman]

Related: #11886

[johnstiles-google]

This is causing issues while fuzzing libjpeg-turbo. See libjpeg-turbo/libjpeg-turbo#761

[johnstiles-google]

This might be driven by a new flag: https://reviews.llvm.org/D134669

-fsanitize-memory-param-retval

[maflcko]

This might be driven by a new flag: https://reviews.llvm.org/D134669

-fsanitize-memory-param-retval

Yes, this was intentional, so it is a true positive. The flag is turned on by default since clang 16, according to https://releases.llvm.org/16.0.0/tools/clang/docs/ReleaseNotes.html#sanitizers . It is possible to turn off the flag on a per-project basis, or in the base image flags, but I am not sure that'd be a good idea, as that may be hiding real bugs.

[maflcko]

The CI-Fuzz issue is a false positive and should be fixed when the image was re-built, according to #11886 (comment) ?

Are there any other issues that I am not aware of?

[jonathanmetzman]

The CI-Fuzz issue is a false positive and should be fixed when the image was re-built, according to #11886 (comment) ?

Sorry the CIFuzz comment is a bit hard to parse. I was sayign that the images are building, so the problem is not simply due to the builder image being out of date.

I haven't looked into this too closely but I think John is saying it is a false positive.

[maflcko]

Presumably this was https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=68704#c3

[johnstiles-google]

I haven't looked into this too closely but I think John is saying it is a false positive.

It wasn't entirely a "false positive", but it was confusing because MSAN's new behavior violates the documentation's explanation of how MSAN works.

https://github.com/google/sanitizers/wiki/MemorySanitizer#introduction

MSan is bit-exact: it can track uninitialized bits in a bitfield. It will tolerate copying of uninitialized memory, and also simple logic and arithmetic operations with it. In general, MSan silently tracks the spread of uninitialized data in memory, and reports a warning when a code branch is taken (or not taken) depending on an uninitialized value.

The new behavior violates this rule. Passing an uninitialized value to a function is "spreading" it, not "branching" on it. I understand that it is UB, but MSAN historically hasn't made its mission to report UB—it has been to track the spread of uninitialized data, and report when the code makes decisions based on it.

So it's a false positive from that perspective, but apparently this is intentional and everyone agrees that it's a good change. In that case it's not a false positive, but the docs are wrong, so they should be updated.

[johnstiles-google]

Filed google/sanitizers#1755. If you are OK with the amended phrasing that I made up in the bug,I can send a PR.