Page MenuHomePhabricator

Disable asan leak tests on i386 linux
AbandonedPublic

Authored by fjricci on Mar 28 2017, 11:41 AM.

Details

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.

Event Timeline

fjricci created this revision.Mar 28 2017, 11:41 AM
m.ostapenko edited edge metadata.Mar 28 2017, 11:52 AM

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.

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 updated this revision to Diff 93267.Mar 28 2017, 11:58 AM

Also explicitly enable lsan on i386 leak tests

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

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.

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).

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).

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?

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 abandoned this revision.Mar 29 2017, 7:47 AM

Replaced by D31462