Disable asan leak tests on i386 linux

[fjricci]

Summary

Since leak detection is not enabled by default, we should not be
running asan tests that assume leak detection is enabled.

We can enable this once i386 lsan is enabled by default.

[m.ostapenko]

Could you elaborate, what kind of issue are you trying to solve? LSan in ASan seems to work fine on x86 Linux, it looks like a wrong idea to disable tests there.

[fjricci]

Well, the issue is that currently, we aren't respecting the default flags as set in sanitizer_flags.inc when we run on asan. asan_flags.cc overrides the defaults and replaces them with CAN_SANITIZE_LEAKS. This means that leak checking will always run on asan builds, even when the default is set to disabled. This isn't causing issues for linux-i386, as most of the tests pass even though it's technically supposed to be disabled. However, for Darwin, the tests will all fail, so we need to ensure that the default values are respected.

When the defaults are respected, asan will not have leak detection by default on linux-i386, which means that asan tests requiring leak-detection will fail.

[fjricci]

Also explicitly enable lsan on i386 leak tests

[fjricci]

For more context, D31307 is the diff which would begin causing linux-i386 failures without this patch.

[m.ostapenko]

In D31430#712387, @fjricci wrote:

Well, the issue is that currently, we aren't respecting the default flags as set in sanitizer_flags.inc when we run on asan. asan_flags.cc overrides the defaults and replaces them with CAN_SANITIZE_LEAKS. This means that leak checking will always run on asan builds, even when the default is set to disabled. This isn't causing issues for linux-i386, as most of the tests pass even though it's technically supposed to be disabled. However, for Darwin, the tests will all fail, so we need to ensure that the default values are respected.

So why not make LSan on Darwin pass these tests instead of disabling them on Linux?

When the defaults are respected, asan will not have leak detection by default on linux-i386, which means that asan tests requiring leak-detection will fail.

Why would we want to disable LSan on x86 Linux by default? It would lead to inconsistency with other architectures default value that may confuse users.

[fjricci]

In D31430#712410, @m.ostapenko wrote:

In D31430#712387, @fjricci wrote:

Well, the issue is that currently, we aren't respecting the default flags as set in sanitizer_flags.inc when we run on asan. asan_flags.cc overrides the defaults and replaces them with CAN_SANITIZE_LEAKS. This means that leak checking will always run on asan builds, even when the default is set to disabled. This isn't causing issues for linux-i386, as most of the tests pass even though it's technically supposed to be disabled. However, for Darwin, the tests will all fail, so we need to ensure that the default values are respected.

So why not make LSan on Darwin pass these tests instead of disabling them on Linux?

Because it hasn't been fully implemented yet, and we want build-only coverage to ensure that nobody breaks the build in the meantime.

When the defaults are respected, asan will not have leak detection by default on linux-i386, which means that asan tests requiring leak-detection will fail.

Why would we want to disable LSan on x86 Linux by default? It would lead to inconsistency with other architectures default value that may confuse users.

All x86 builds for lsan are currently disabled by default, due to the very high false-negative rate (I believe that patch blames to you actually).

[fjricci]

To be clear, my patch doesn't change the default set by sanitizer_flags.inc for detect_leaks, it just forces asan to respect it. So if we want to change the default, that's something that could be done (although not my decision).

[m.ostapenko]

In D31430#712412, @fjricci wrote:

In D31430#712410, @m.ostapenko wrote:

In D31430#712387, @fjricci wrote:

Well, the issue is that currently, we aren't respecting the default flags as set in sanitizer_flags.inc when we run on asan. asan_flags.cc overrides the defaults and replaces them with CAN_SANITIZE_LEAKS. This means that leak checking will always run on asan builds, even when the default is set to disabled. This isn't causing issues for linux-i386, as most of the tests pass even though it's technically supposed to be disabled. However, for Darwin, the tests will all fail, so we need to ensure that the default values are respected.

So why not make LSan on Darwin pass these tests instead of disabling them on Linux?

Because it hasn't been fully implemented yet, and we want build-only coverage to ensure that nobody breaks the build in the meantime.

When the defaults are respected, asan will not have leak detection by default on linux-i386, which means that asan tests requiring leak-detection will fail.

Why would we want to disable LSan on x86 Linux by default? It would lead to inconsistency with other architectures default value that may confuse users.

All x86 builds for lsan are currently disabled by default, due to the very high false-negative rate (I believe that patch blames to you actually).

True, but during enabling LSan on x86 Linux (during review) it was decided to make it on by default. And it's simply wrong to disable tests on Linux just for enabling build on Darwin.
Can't you just modify detect_leaks default value to be off on Darwin and on on Linux?

[fjricci]

Ahh, ok. In that case, I can upload a patch to enable lsan 32-bit by default in 'sanitizer_flags.inc' instead. For some reason, that file still defaults 'detect_leaks' to 'WORDSIZE == 64', which is why I assumed we wanted lsan disabled on 32-bit Linux by default.

[fjricci]

Replaced by D31462

test/asan/lit.cfg

 config.substitutions.append( ("%xdynamiclib_namespec", '$(basename %t).dynamic') )
 
 # Allow tests to use REQUIRES=stable-runtime.  For use when you cannot use XFAIL
 # because the test hangs. Adding armhf as we now have two modes.
 if config.target_arch != 'arm' and config.target_arch != 'armhf' and config.target_arch != 'aarch64':
   config.available_features.add('stable-runtime')
 
 # Turn on leak detection on 64-bit Linux.
-if config.host_os == 'Linux' and (config.target_arch == 'x86_64' or config.target_arch == 'i386'):
+if config.host_os == 'Linux' and config.target_arch == 'x86_64':
   config.available_features.add('leak-detection')
 
 # Set LD_LIBRARY_PATH to pick dynamic runtime up properly.
 push_dynamic_library_lookup_path(config, config.compiler_rt_libdir)
 
 # GCC-ASan uses dynamic runtime by default.
 if config.compiler_id == 'GNU':
   gcc_dir = os.path.dirname(config.clang)