[llvm-dev] Meet at MTV to discuss TSAN/android/aarch64?

[Jason Kim via llvm-dev]

On Thu, Sep 24, 2015 at 6:10 PM, Renato Golin <renato.golin at linaro.org>
wrote:

> Hi Jason,
>
> I have a few additional points:
>
> 1. It works on glibc but not in bionic because glibc calls internal
> allocation functions while bionic calls malloc/free, which get
> instrumented when the thread is being destroyed. Calling bionic_malloc
> on those cases should be enough to remove the problem altogether, but
> we should not call bionic_malloc everywhere, since we want all other
> calls to be instrumented and we don't want user facing behaviour to
> change.
>
>
The calls that are changed would be purely bionic-to-bionic calls of XXXX()
(XXXX() being malloc() or whatever else). There would be no user noticeable
diffferences.  If XXXX() is called by a user routine, it would still be
intercepted.



> 2. The changes to Android should be made in the implementation, not in
> the header files. We're trying to avoid include nightmares and
> pre-processor race conditions, as well as protecting users from
> implementation details.
>

???? There are no nightmares here. Of what do you speak?


>
> 3. Google will prioritise pthread_barrier and other functionality that
> helps TSAN works with bionic without the need for the TLS workaround
> in compiler-rt. They'll also set up some internal bots to make sure we
> test those things from now on.
>
>
Sure. That sounds fine.


> cheers,
> --renato
>
> On 24 September 2015 at 11:18, Jason Kim <jasonk at codeaurora.org> wrote:
> > Hi everyone.
> >
> > Thanks to everyone for taking the time to meet yesterday. I think it was
> > very productive!
> >
> > I am writing to keep a "minutes" of the meet, as I remember them.
> >
> > Action Items:
> >
> > Jasonk:  split up bionic patch to explicitly replace CPP macros with
> actual
> > calls, one file at a time.
> > Google/Android: pthread_barrier interface in bionic
> > Enh:  1 paragraph to explain "namespacing" in bionic?? (in case we need
> to
> > reconsider macro replacement again?)
> >
> > Some technical notes/explanations that possibly weren't made clear at the
> > meeting.
> >
> > The current compller-rt patch (on phabricator)  is already a pretty
> minimal
> > set. The only real bit of actual functionality (other than the
> > infrastructure bits to enable TSAN on android/aarch64) is the TLS
> > workaround.
> >
> > i.e. the major pieces for my patch to compiler-rt are:
> >
> > infrastructure (new definitions, cmake changes etc..)
> > TLS workaround
> > pthread_barrier-like interface for tests
> > disabling of some tests on android-aarch64
> >
> > My current hypothesis is that the 120 tests (out of 200 or so)  that are
> > failing are due the behavioral mismatch of my "quick hack" to enable
> > barrier-like behavior on android. Assuming that Google's implementation
> of
> > the barrier interface on android/aarch64 is a better fit with
> linux-x86_64,
> > this will hopefully result in more tests passing.
> >
> > Very important info regarding why __bionic_XXX() replacement for calls to
> > XXX() will most likely need to be "global" (within bionic) in most cases:
> >
> > deadlocks, crashes in TSAN were all due to unexpected interceptions
> WHILE a
> > call was already being intercepted by TSAN.
> > The most obvious TSAN-only workaround is to ignore interceptions when
> this
> > is taking place. This works for two threads, but   does not scale to more
> > threads without the danger of TSAN missing events.
> > Any unexpected nested call chains in bionic that TSAN currently
> intercepts
> > will either need custom coding in TSAN to explicitly handle (for all
> > possible cases in which such call chains can occur), OR replace those
> calls
> > in bionic with non-interceptible ones. The former will obviously add much
> > more complexity to TSAN (to wit, additional testing failure
> vulnerability),
> > while the latter will mean more (mechanical) changes to BIONIC.
> Currently,
> > my thinking is that the latter, (being mechanical in nature) is
> preferable.
> > To ENH: The same holds for calls to tcgetattr() --> ioctl(), and
> isatty() ->
> > tcgetattr() --> ioctl() call chains. TSAN intercepts  tcgetattr() and
> > ioctl(), and is not expecting recursive interceptions during them. Its
> not
> > at all clear how to handle this within TSAN, especially among multiple
> > threads. Even if a logic can be worked out, it's fundamentally much
> clearer
> > to simply disallow such interceptions to occur.
> >
> > Things that might require future discussion:
> >
> > need a build-bot/test bot on labs.llvm.org for
> android-bionic-aarch64-tsan
> > tests to prevent insanity inducing regressions :-)
> >
> > Thanks for reading!
> >
> > -Jason
> >
> >
> >
> >
>
> --
> You received this message because you are subscribed to the Google Groups
> "thread-sanitizer" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to thread-sanitizer+unsubscribe at googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150925/fa1d0b62/attachment.html>