HomePhabricator
Diffusion LLVM Github Monorepo 61e59ebfa645

[LSAN] Enabled only with __ANDROID_API__ >= 28
Audited61e59ebfa645
Actions

Authored by vitalybuka on Nov 4 2020, 9:42 PM.

Description

[LSAN] Enabled only with ANDROID_API >= 28

Code does not work as-is with emulated tls even if
lsan is disabled with runtime flag.

Details

Auditors
oontvoo
Committed
vitalybukaNov 4 2020, 10:18 PM
Parents
rG4455f3ce72ba: Capture the name for mlir::OpPassManager in std::string instead of StringRef…
Branches
Unknown
Tags
Unknown

Event Timeline

vitalybuka committed rG61e59ebfa645: [LSAN] Enabled only with __ANDROID_API__ >= 28 (authored by vitalybuka).Nov 4 2020, 10:18 PM
vitalybuka added an auditor: oontvoo.Nov 4 2020, 10:19 PM
This commit now requires audit.Nov 4 2020, 10:19 PM
vitalybuka added a comment.Nov 4 2020, 10:20 PM

It crashes on ubsan tests on "bool DisabledInThisThread() { return disable_counter > 0; }"

vitalybuka added a comment.Nov 4 2020, 10:20 PM

Stack overflow in __emutls_get_address

vitalybuka added a comment.Nov 4 2020, 10:23 PM

Runtime flag around "disable_counter" kind of works, but it requires some investigation to be sure. I landed this to fix the bot.

if (!common_flags()->detect_leaks)
    return true;
oontvoo accepted this commit.Nov 5 2020, 6:06 AM
oontvoo added a subscriber: oontvoo.

(sorry for the late response)

All concerns with this commit have now been addressed.Nov 5 2020, 6:06 AM
oontvoo added a comment.Nov 5 2020, 6:15 AM
In rG61e59ebfa645caa3b89baaf51dd26a56625bc39f#957756, @vitalybuka wrote:

Runtime flag around "disable_counter" kind of works, but it requires some investigation to be sure. I landed this to fix the bot.

if (!common_flags()->detect_leaks)
    return true;

I've tried it before. Didn't seem to work.
I think the issue is we can't have a TLS variable if you set -fno-emulated-tls AND the itself platform doesn't have ELF-TLS.

It crashes on ubsan tests on "bool DisabledInThisThread() { return disable_counter > 0; }"

Are you sure it crashes *only* in ubsan tests? This should be a problem for all leak-related tests, no?

vitalybuka added a comment.Nov 5 2020, 12:47 PM
In rG61e59ebfa645caa3b89baaf51dd26a56625bc39f#957815, @oontvoo wrote:
In rG61e59ebfa645caa3b89baaf51dd26a56625bc39f#957756, @vitalybuka wrote:

Runtime flag around "disable_counter" kind of works, but it requires some investigation to be sure. I landed this to fix the bot.

if (!common_flags()->detect_leaks)
    return true;

I've tried it before. Didn't seem to work.
I think the issue is we can't have a TLS variable if you set -fno-emulated-tls AND the itself platform doesn't have ELF-TLS.

It crashes on ubsan tests on "bool DisabledInThisThread() { return disable_counter > 0; }"

Are you sure it crashes *only* in ubsan tests? This should be a problem for all leak-related tests, no?

No, it's not only them. I just used them for verification.

Changes (1)

PathSize
compiler-rt/
lib/
lsan/

rG61e59ebfa645

View Options

compiler-rt/lib/lsan/lsan_common.h

Loading...