[llvm-dev] TSAN hack on AArch64 for Android

[Renato Golin via llvm-dev]

On 26 August 2015 at 20:13, Jason Kim <jasonk at codeaurora.org> wrote:
> I had been discussing the TSAN on android/aarch64 issues with the folks at thread-sanitizer at googlegroups.com for several months now. I had not explicitly been including llvm-dev (and you), and that was a mistake on my part. Palm-on-face? :-(

Yup, that would have saved a lot of emails... :)


> Patch Size!
> The patch in question is a bit smaller now thanks to some dovetailing with http://reviews.llvm.org/D11484 being merged.
> I have already split up the patch into three chunks. I can do more as needed.

Before I have any opinion on the current patch, we need to know what's
going to change.


> 1. use pthread_get/setspecific()  (more on this below!!)

If the re-creation of the thread state disappears after the bionic
changes, I'd seriously investigate this one, as it seems the most
logic.


> 2. use some other workaround that does not use pthread API

I'm trying to avoid this at all costs. :)


> 3. use the new TLS slot from Dan's recent push to android. https://android-review.googlesource.com/#/c/167420/

I asked somewhere else, but, will this give you a reasonably looking
thread state without resorting to pthread_get/specific calls? Will
this be a unique and non-redundant state for each thread?


> 4. use the emutls work http://reviews.llvm.org/D12001

I only suggested this because Joerg/Dan mentioned it. I haven't looked
at that patch in detail. From what I'm seeing, options 1 and 3 above
may very well suit us.


> TESTS!
>  Currently, about 2/3 tests for tsan fail/flake on android+aarch64.
> They aren't xfailed, because some of them are flaky, and thus unexpectedly pass at times.
> So I decided to mark them as unsupported on android+aarch64, and thus is "green".

So, if they currently pass on the production buildbots, you can't mark
them as unstable because of your patch, as that will mean your patch
is making them unstable, and we can't just make dozens of tests
unstable for any amount of features.

If your patch introduces the instability, you'll have to fix them
before commit. That's why I suggested splitting it into many small
chunks, none of which will leave the tree in an unstable state,
because you'll have time to look at each instability individually, and
fix them before commit. The bots will also be happier, and every one
wins.

I have no idea how to split your patch so that this happens, but since
you know what the instability is, we could work together to test and
validate *before* the patches go live. I have a fast AArch64 machine
just for that purpose, so we can do this as fast as possible.


> I have not yet had a chance to test this out. Assuming this works,  what is your opinion on the "proper" TLS emulation in TSAN?
>
> a) always use pthread api?
> b) leave the use of __thread keyword there and use some variant of arch+os  conditionals to select the appropriate technique?

I'm not familiar with pthread enough to give you a good answer. I'm
hoping Joerg/Dan could chime in. Having said that, I'd rather use
something that is arch/os agnostic, even if it slows it down a few
percent. If the slow down is too great, then we can discuss case by
case.

cheers,
-renato