- User Since
- Dec 4 2015, 11:34 AM (68 w, 4 d)
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).
Updated that diff, and now check-all passes locally on darwin and linux.
For more context, D31307 is the diff which would begin causing linux-i386 failures without this patch.
Also explicitly enable lsan on i386 leak tests
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.
Ok, pending D31430, this now passes tests.
Mon, Mar 27
Account for default detect_leaks value when setting asan default value
I primarily wrote this because getArchFlag() accounts for the possibility that getCompiler() can be None. But my problem was unrelated, so I don't need this (and I agree that it would be surprising if anyone did).
Fri, Mar 24
Thu, Mar 23
This code has already been reviewed in D29994, but uploading here for posterity. Will merge pending a disable of use_tls_dynamic.cc on linux-i386.
Wed, Mar 22
Actually, let's make it even smaller.
Split revision into smaller pieces
I've tried everything I can think of in terms of a repro here - different gcc/clang versions, etc. I also looked up the zorg config and ran exactly the way it was run on the buildbot (standalone build + i386 test), and still no failure. Perhaps I can split this into two changes, to try to see where the failure is springing up.
Tue, Mar 21
Going to try to get the failure to repro locally.
Mon, Mar 20
Actually, looks like the buildbot isn't failing currently. I won't revert the patch for now.
This patch is having a real rough time. @vitalybuka thinks it probably caused this failure, and I wouldn't be too surprised: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/1117/steps/test%20standalone%20compiler-rt/logs/stdio
Works for me now, thanks.
I think @beanz is correct, I actually end up seeing a few of my architectures disappear with the current version of this patch, although I didn't look into it yet.
Yep, that patch does appear to fix my issue in its current form. I'll abandon this, with the assumption that will go in.
Fix commit message
This still fails on aarch64, even with the recent work by @kubamracek:
Sun, Mar 19
Now that the allocator patch is in, this should be good to go. I'll upload the patch to enable builds (but not tests) by default separately.
Sat, Mar 18
Thanks for looking into it!
Working on running the tests locally, I'll commit tomorrow if things pass.
Mon, Mar 13
Reverted due to failure with D30267
My assumption is that a repro of the failure will also require D29994, as that's the patch that seems to affect the flakiness.
I was able to run this on a linux aarch64 emulator, but I'm not able to reproduce the failure. I do still see the undefined behavior popping up. Wondering if it's worth disabling the particular test case that uses the small stack size, since it seems to be passing primarily due to this undefined behavior anyway.
Wed, Mar 8
Committed as r297315
Ping - this is required for Darwin LSAN builds to be enabled by default.
Sorry for the delay, I've been going through a job change. Looks like the link is now dead: http://lab.llvm.org:8011/builders/clang-cmake-aarch64-42vma/builds/5218
Wed, Mar 1
Reverted because this fails on aarch64. I don't have access to an aarch64 linux device, so I'm not sure the best way to debug.
There's a more detailed explanation in D29994, but essentially, some debugging found that 64k isn't actually being used for the stack size consistently, and the test never passes when 64k is actually used. (actual stack size used determined with pthread_attr_getstacksize)
Feb 22 2017
Given @kcc's comment, LGTM.
I found something very odd and very relevant. I added a line in test-use-after-return.cc to call pthread_attr_getstacksize immediately after pthread_attr_setstacksize. On master, the returned stack size is 8388608. With my change, it's 65536. I assume this means that we're intercepting something inside call to pthread_attr_setstacksize. But regardless, this means that 65536 was not large enough to begin with, unless we expect the sanitizers to intercept that call and increase the stack size. I assume we don't, since there's no direct interceptor for pthread_attr_setstacksize.
That does look like it could be a stack-overflow seg fault though.
The stack trace from the debugger isn't particularly helpful: