[msan] Do not use 77 as exit code, instead use 1

[Flow]

Summary

MSan uses 77 as exit code since it appeared with c5033786ba34 ("[msan]
MemorySanitizer runtime."). However, Test runners like the one from
Meson use the GNU standard approach where a exit code of 77 signals
that the test should be skipped [1]. As a result Meson's test runner
reports tests as skipped if MSan is enabled and finds issues:

build $ meson test
ninja: Entering directory `/home/user/code/project/build'
ninja: no work to do.
1/1 PROJECT:all / SimpleTest SKIP 0.09s

I could not find any rationale why 77 was initially chosen, and I
found no other clang sanitizer that uses this value as exit
code. Hence I believe it is safe to change this to a safe
default. You can restore the old behavior by setting the environment
variable MSAN_OPTIONS to "exitcode=77", e.g.

export MSAN_OPTIONS="exitcode=77"

1: https://mesonbuild.com/Unit-tests.html#skipped-tests-and-hard-errors

[vitalybuka]

What is going to be the default?

[kcc]

This worked for us for many years.
Changing the default is likely to break some of the existing users.

I am not aware of Meson and don't know how widely it is used, especially combined with msan.
And, there is an easy workaround: Meson can change the default error code via MSAN_OPTIONS

[Flow]

In D92490#2429404, @vitalybuka wrote:

What is going to be the default?

The default is going to be 1, and is taken from https://github.com/llvm/llvm-project/blob/61a06c071dd16a9725d3b7bfac806520dc1b95aa/compiler-rt/lib/sanitizer_common/sanitizer_flags.inc#L236

In D92490#2429413, @kcc wrote:

This worked for us for many years.
Changing the default is likely to break some of the existing users.

I can not argue with that. But I believe the change should be done nevertheless. Note that the special semantic of an exit value of 77 is not meson-specific but instead widely used in unit test frameworks (for example, CMake does the same). I did not do much archaeology where this seems from, but it is probably due to GNU automake (see https://www.gnu.org/software/automake/manual/html_node/Scripts_002dbased-Testsuites.html). Eventually, I think changing the MSan behavior is sensible in a major release of LLVM. I am also happy to write a changelog entry for this, which hints at users affected by the change towards the MSAN_OPTIONS environment variable and the possibility to change the exit code used by MSAN to a value of their choice.

I think, in this case, what is more important than backward compatibility is that MSan works out-of-the-box with test runners. It took me quite a few hours to get MSan to work correctly with our project. I also believe that the number of users affected by this change is probably very small (and there is an easy way for those affected users to restore the old behavior).

In D92490#2429413, @kcc wrote:

I am not aware of Meson and don't know how widely it is used, especially combined with msan.

Meson is for example used by systemd and has explicit support for clang's and gcc's santiziers via the Meson base option b_sanitize.

In D92490#2429413, @kcc wrote:

And, there is an easy workaround: Meson can change the default error code via MSAN_OPTIONS

Unfortunately that is not so easy as one may think: Meson has to first check if the MSAN_OPTIONS environment is already set, and if, do some modifications to it while attempting to keep the semantic of its pre-existing content. This approach is inherently fragile.

[eugenis]

I don't mind this change. If anyone feels strongly against it, please let us know.

See also: https://github.com/google/sanitizers/issues/1225.

(this change is simple enough, but in the future please upload diffs with full context, see https://llvm.org/docs/Phabricator.html)

[eugenis]

LGTM

Are there any tests that need updating?

[Flow]

In D92490#2443573, @eugenis wrote:

LGTM

I am unable to commit this myself. The latest version of this change can be found in the msan-exit-code branch on my LLVM fork.

compiler-rt/lib/msan/msan.cpp

SetCommonFlagsDefaults();
     CommonFlags cf;
     cf.CopyFrom(*common_flags());
     cf.external_symbolizer_path = GetEnv("MSAN_SYMBOLIZER_PATH");
     cf.malloc_context_size = 20;
     cf.handle_ioctl = true;
     // FIXME: test and enable.
     cf.check_printf = false;
     cf.intercept_tls_get_addr = true;
-    cf.exitcode = 77;
     OverrideCommonFlags(cf);
   }
 
   Flags *f = flags();
   f->SetDefaults();
 
   FlagParser parser;
   RegisterMsanFlags(&parser, f);