In D142888#4225117, @Chia-hungDuan wrote:

Right, Scudo works on arm32 on Android. Android doesn't use cmake so I guess there's a proper unwind lib linked on arm32 Android in Android.bp when Scudo+gwp_asan is enabled. I didn't try it on arm32 Android, maybe @hctim can comment more.

In D146935#4232088, @hctim wrote:

Yeah, dlopen-ing a symbol that's already defined in the main executable would definitely do it. You shouldn't just disable ODR checking, as this is a real bug.

I had a look at our internal codebase, and saw three symbols (including the one you listed in your bug) being allow-listed with the following flags:

-Wl,--export-dynamic-symbol=llvm_orc_registerJITLoaderGDBWrapper
-Wl,--export-dynamic-symbol=llvm_orc_registerEHFrameSectionWrapper
-Wl,--export-dynamic-symbol=llvm_orc_deregisterEHFrameSectionWrapper

Do you think that'd do it for you instead of exporting every linked symbol from the executable?

In D146935#4231773, @awarzynski wrote:

Thanks for the pointers! I do see that both mlir-cpu-runner and libmlir_async_runtime.so define EnableABIBreakingChecks:

$ nm libmlir_async_runtime.so | grep EnableABIBreakingChecks

000000000038cb80 B _ZN4llvm23EnableABIBreakingChecksE

$ nm mlir-cpu-runner | grep EnableABIBreakingChecks
000000000474364c B _ZN4llvm23EnableABIBreakingChecksE

IIUC, the problem comes from this RUN line:

// RUN: | mlir-cpu-runner                                                      \
// RUN:     -e main -entry-point-result=void -O0                               \
// RUN:     -shared-libs=%mlir_c_runner_utils  \
// RUN:     -shared-libs=%mlir_runner_utils    \
// RUN:     -shared-libs=%mlir_async_runtime   \

I'm guessing that this is due to dlopen (which is what -shared-libs translates to). Which makes me think - would ASAN_OPTIONS=detect_odr_violation=0 be the right solution/work-around? Not ideal 🤔 . I could add a GitHub issue to let people know that longer term we should consider some other fix. WDYT?

This might've broken builds? Either that, or one of your other three patches.

No problems at all! In case you're not familiar, ODR is explained a little in our docs (and the other stuff that's linked there). Best bet to track it down is to figure out why llvm/lib/Support/ABIBreak.cpp is linked twice.

Already reverted in https://reviews.llvm.org/rG270067d69532448c4f6d7956e68d7a392857f9ce :)

Hi, looks like this re-land triggered an ASan-detected heap leak: https://lab.llvm.org/buildbot/#/builders/5/builds/32516

Hey, looks like this breaks check-sanitizer on all the bots. Easily reproducible on a vanilla build, just do check-sanitizer on something that builds compiler-rt.

Hi folks, looks like this broke ASan buildbots because of an ODR. Full reproduction instructions can be found at https://github.com/google/sanitizers/wiki/SanitizerBotReproduceBuild (either using buildbot_fast.sh or buildbot_bootstrap_asan.sh), or alternatively this is quickly reproducible without a multi-stage build using my example cmake config below (which definitely has some stuff you don't need, like libc):

Hi, yep, I've reverted upstream. If you could please also integrate https://reviews.llvm.org/rG8c7c1f11ffaacf762e612c65440fd2cbb58ee426 in the relanding, that would be great.

Add some extra edge case tests.

Any ideas why this was IE in the first place? Don't see any notes in the patch that added it (https://reviews.llvm.org/D37156)?

Address jhenderson@'s comments.

Address MaskRay and jhenderson's comments.

we don't support llvm-symbolizer and the default libbacktraace symbolizer does not support location info.

@fmayer / @jhenderson / @MaskRay , would you folks mind taking a second pass? thanks.

@fmayer / @MaskRay , would you mind taking a quick second pass? thanks

Update with jhenderson@'s comments: used arrayref in place of vector, some minor reformatting and renaming.

Update the test a little to be explicit about warning/error, remove a newline.

In D143769#4120139, @MaskRay wrote:

Considering adding some documentation to lld/docs/ld.lld.1. It can contain more information than lld/ELF/Options.td

Address Ray's comments.

Add a comment describing the use of printNotesHelper.

Add test that --memtag doesn't print non-MTE dynamic entries.

Hey folks, looks like this caused a failure on the msan buildbot: https://lab.llvm.org/buildbot/#/builders/237/builds/785

Hey, looks like this broke the x86_64-linux sanitizer buildbot: https://lab.llvm.org/buildbot/#/builders/37/builds/19910

Hey, looks like this might've broken the windows sanitizer bots. Would you be able to take a look? https://lab.llvm.org/buildbot/#/builders/127/builds/43077

Ah, looks like you beat me to it. Thanks for reverting early :).

Hey, looks like this broke the asan-aarch64 buildbot. https://lab.llvm.org/buildbot/#/builders/239/builds/771

Hey, looks like this probably broke the ASan buildbot: https://lab.llvm.org/buildbot/#/builders/5/builds/31141/steps/13/logs/stdio

Hey, FYI - it looks like this broke the ASan buildbot:

In D133392#4094524, @dyung wrote:

Hi @hctim, the test you added is failing on buildbots where AArch64 is not built, can you either update the test to not require that backend, or mark the test as requiring it?

https://lab.llvm.org/buildbot/#/builders/139/builds/35253

error: unable to create target: 'No available targets are compatible with triple "aarch64-unknown-linux-android31"'
1 error generated.

Rebase for submit.

Looks like this patch still causes errors on the ASan buildbot, even with D142883 patched in.

Thanks for the patch, I think this is a really good addition to scudo. Clearly you've found some bugs. An overall comment, there's a lot of mixed non-functional and functional changes here. For example, it's my understanding that this patch is primarily to add thread safety annotations, but there's functional changes in unmapTestOnly, malloc_info, and other places.

Update with some fixes discovered from internal testing: