[RFR]: Lock instrumentation
Man Cao
manc at google.com
Tue Apr 23 20:23:22 UTC 2019
In synchronizer.cpp, in ReleaseJavaMonitorsClosure::do_monitor(), the TSAN
instrumentation should be inside the code block of the conditional "if
(mid->owner() == THREAD) {".
Otherwise looks good. No need for a new webrev after fixing this.
Regarding the "#if INCLUDE_TSAN" macro, I think we could define a macro in
macros.hpp like the following:
#if INCLUDE_TSAN
#define TSAN_RUNTIME_ONLY(code) if (ThreadSanitizer) { code }
#else
#define TSAN_RUNTIME_ONLY(code)
#endif
Then it could simplify some (but not all) of the instrumentation.
-Man
On Tue, Apr 23, 2019 at 9:49 AM Jean Christophe Beyler <jcbeyler at google.com>
wrote:
> Hi Arthur,
>
> Looks good to me, I was looking at if we could factorize a bit the
> assertions and what-not but for now it's fine.
>
> (I did notice that we let creep in the #endif // INCLUDE_TSAN which does
> not seem to be that much the style of the general code base so we might
> want to remove them at some point)
>
> But that's a detail, LGTM,
> Jc
>
>
> On Tue, Apr 23, 2019 at 9:29 AM Arthur Eubanks <aeubanks at google.com>
> wrote:
>
> > This changes adds instrumentation for various locks in the JVM. (not
> > including JVMTI)
> > http://cr.openjdk.java.net/~aeubanks/tsanlock/webrev.00
> >
> > Note the NonRacy tests still don't pass for some reason. I believe it's
> due
> > to dynamic linking of libtsan.so.
> >
>
>
> --
>
> Thanks,
> Jc
>
More information about the tsan-dev
mailing list