The call that actually models the memory access always uses CALLERPC. Is this how it should be?

Thanks Roy! I put my suggestions, but none of them are blocking.

This the follow-up I promised here:
https://reviews.llvm.org/D146264#4202953

Thanks for the quick reviews everyone! :)

LGTM, thanks!

Hi, thanks Vitaly!

LGTM, looks harmless enough!

I will land this later today.

Mentioned removal of flag in release notes.

Okay, so dladdr() just takes an address and should always work irrespective if the function that lives at that address was exported etc., yes?

Taking a step back, please check my understanding:

In D144830#4157118, @vitalybuka wrote:

[... ] __sanitizer_print_stack_trace? Seems like more stable bet.

Thank you @dmaclach!

Looks like this is a follow-up to D141550.

• 1 whitespace nit
• abc package available everywhere? If not, can we do without?

Initially, my main question was: would this functionality make sense for reports other than the xml one too? Probably not, because the use case is pretty specific, right? Anyways, I don't think we need to implement that now and leave that work to whoever needs it.

Thanks everyone for pointing out the build failures on Linux and reverting the change.

LGTM, with a few small nits. Thanks Mariusz!

Looks harmless enough, even though I am not sure why we have a completely separate code path on Darwin. The other toolchain driver seem to have shared code for this in clang/lib/Driver/ToolChains/CommonArgs.cpp.

Thanks so much for this effort and seeing it through! I think this materially improves LIT feature handling and reduces confusion.

Looks harmless enough.

Thanks Blue!

If we are not doing the optional cleanup, then I think I would prefer this:

#  if (SANITIZER_ANDROID && defined(__aarch64__)) || SANITIZER_FUCHSIA
#    define SANITIZER_CAN_USE_ALLOCATOR64 1
# elif defined(__mips64) || defined(__arm__) || defined(__i386__) || \
      SANITIZER_RISCV64 || defined(__hexagon__) || SANITIZER_IOS
#    define SANITIZER_CAN_USE_ALLOCATOR64 0
#  else

I think we can drop && !defined(SANITIZER_APPLE) altogether and let the "default" (last else) case handle things. It definitely doesn't make sense in it's current form: && takes precedence so the code is currently saying: xxx || (hexagon && !apple)

@wrotki: you touched similar code recently. Can you do a post-merge review to convince yourself that this is doing the right thing? Thanks!

Thank you! LGTM, with one nit.

In D139030#3961494, @rsundahl wrote:

The change as I originally made it was to use the 32 bit allocator in all of the cases that it had been used in up until things broke. I was conservative and explicit about enabling the 64 bit allocator for macOS because I tested it and it worked. The reviews here tell me that we want to go further but have questions so I'm going to revert to the original behavior of using the 32 bit allocator for SANITIZER_APPLE on aarch64.

In D139030#3961252, @rsundahl wrote:

In D139030#3961123, @thetruestblue wrote:

Can we not do (SANITIZER_APPLE && SANITIZER_IOS) ?

I didn't want to depend on the overloading of SANITIZER_IOS which means both actual iOS and similar to iOS, so I chose to come at the logic the way I did.

LGTM & thanks from my side!

Looks harmless enough.. LGTM!

Can you give an explanation of the scenario (false positive, false negative?) which this patch fixes. Please put this information also in the commit message.

@lechenyu: do you mind signing off on this?

LGTM, thanks for seeing this through @wrotki!

H @lechenyu, thanks for helping out! :)

Add include

@protze.joachim does this fix the error you see?

In D129157#3806107, @protze.joachim wrote:

This change bricks the old tsan runtime. Should we remove the call from rtl-old as well? Or is rtl-old not maintained at all?

LGTM, with nits: