Commit 52f2faac authored by Caleb Raitto's avatar Caleb Raitto Committed by Commit Bot

Detect leaks when using ASan.

I have no idea what I'm doing (I'm not sure how to test this change),
please review this carefully :)

See the discussion at [1] for more context.

Chromium documentation claims that LeakSanitizer runs on all ASan trybots [2],
but I have been unable to replicate this behavior myself.

To test, I created a CL (https://crrev.com/c/1550630/4) that intentionally
leaks memory inside of a test. None of the trybots, including the ASan
trybot, were able to find the leak.

In contrast, a similar test that had a gtest expectation failure
(https://crrev.com/c/1588945/1) and another test that had a
use-after-free bug (https://crrev.com/c/1588858/1) were both
successfully caught by the ASan trybot.

Despite the inability to catch leaks using trybots, I was able to catch
the leak locally on my workstation by following the instructions in [2].

I noticed that the trybot test was built with 'is_lsan=true' and
'is_asan=true' [3].  However, the runtime ASAN_OPTIONS listed in [4] do
not include 'detect_leaks=1' as required by the documentation [2].

I found that running locally without the 'detect_leaks=1' in
ASAN_OPTIONS, my leak test passed (that is, the leak wasn't caught).

The src/testing/test_env.py script appears to add 'detect_leaks=1' to
ASAN_OPTIONS if the script is passed the '--lsan' flag [5][6].

However, the script that calls test_env.py, tools/mb/mb.py, doesn't seem
to ever pass this flag [7] -- that is what this CL attempts to fix.

[1]
https://groups.google.com/a/chromium.org/forum/#!msg/chromium-dev/UEnRGM43MYE/3LnWa4k5BwAJ

[2]
https://www.chromium.org/developers/testing/leaksanitizer

[3]
https://ci.chromium.org/p/chromium/builders/try/linux_chromium_asan_rel_ng/264888

[4]
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8914804258249981008/+/steps/cronet_tests__with_patch_/0/stdout

[5]
https://cs.chromium.org/chromium/src/testing/test_env.py?l=95&rcl=fba6691ffca770dd0c916418601b9c9c019a2929

[6]
https://cs.chromium.org/chromium/src/testing/test_env.py?l=294&rcl=fba6691ffca770dd0c916418601b9c9c019a2929

[7]
https://cs.chromium.org/chromium/src/tools/mb/mb.py?l=1229&rcl=57ddea756f68c9d95dcdec9e02b86fd1f5860b60

Bug: 948939
Change-Id: I7237e76aad395271ddc2403612a62aaaa18ae0f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1590404Reviewed-by: default avatarJohn Budorick <jbudorick@chromium.org>
Commit-Queue: Caleb Raitto <caraitto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#668443}
parent b62cd901
...@@ -1251,6 +1251,7 @@ class MetaBuildWrapper(object): ...@@ -1251,6 +1251,7 @@ class MetaBuildWrapper(object):
'./' + str(executable) + executable_suffix, './' + str(executable) + executable_suffix,
'--test-launcher-bot-mode', '--test-launcher-bot-mode',
'--asan=%d' % asan, '--asan=%d' % asan,
'--lsan=%d' % asan, # Enable lsan when asan is enabled.
'--msan=%d' % msan, '--msan=%d' % msan,
'--tsan=%d' % tsan, '--tsan=%d' % tsan,
'--cfi-diag=%d' % cfi_diag, '--cfi-diag=%d' % cfi_diag,
...@@ -1261,6 +1262,7 @@ class MetaBuildWrapper(object): ...@@ -1261,6 +1262,7 @@ class MetaBuildWrapper(object):
'./' + str(executable) + executable_suffix, './' + str(executable) + executable_suffix,
'--test-launcher-bot-mode', '--test-launcher-bot-mode',
'--asan=%d' % asan, '--asan=%d' % asan,
'--lsan=%d' % asan, # Enable lsan when asan is enabled.
'--msan=%d' % msan, '--msan=%d' % msan,
'--tsan=%d' % tsan, '--tsan=%d' % tsan,
'--cfi-diag=%d' % cfi_diag, '--cfi-diag=%d' % cfi_diag,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment