[RFR]: Lock instrumentation

[Arthur Eubanks]

With JC's fix to the tests and the ReleaseJavaMonitorsClosure::do_monitor()
fix, now all the tests pass!

I'm mixed on the TSAN_RUNTIME_ONLY() suggestion, but either way it can come
in a later change.

On Tue, Apr 23, 2019 at 1:23 PM Man Cao <manc at google.com> wrote:

> 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
>>
>