[RFR] Java memory instrumentation for TSAN

[Arthur Eubanks]

On Fri, May 24, 2019 at 2:20 PM Man Cao <manc at google.com> wrote:

> We can add the "ThreadSanitizerJavaMem" flag, so it is consistent with our
> internal implementation.
>
Added a new flag named "ThreadSanitizerJavaMemory" (no point in shortening
the name by 3 chars).

> We can set ThreadSanitizerJavaMem's default value to true here, because
> all the uses of this flag should be guarded by "if (ThreadSanitizer)".
>
> Only the following places need to be changed to use ThreadSanitizerJavaMem:
> In sharedRuntime.cpp:
> #define TSAN_MEMORY_ACCESS(name)
> ...
> assert(ThreadSanitizer, "Need -XX:+ThreadSanitizer");
>
Done (asserted both ThreadSanitizer and ThreadSanitizerJavaMem).

>
> In templateTable_x86.cpp:
> TemplateTable::tsan_observe_get_or_put()
> ....
>  if (ThreadSanitizer) {
>   __ jmp(Done);
>
Done.

>
> TemplateTable::tsan_observe_load_or_store()
> It can add an assert for ThreadSanitizer, and check
> for ThreadSanitizerJavaMem instead.
>
Done.

>
> Other minor issues:
> In arguments.cpp, can we change it to:
> TSAN_RUNTIME_ONLY(
>   set_mode_flags..
>   FLAG_SET_ERGO..
>   FLAG_SET_ERGO..
> );
>
Done. I didn't want to do this initially because I think it's ugly to have
a semicolon in the statements and at the end of the macro, but oh well.

>
> In instanceKlass.cpp:
> Can we move "#if INCLUDE_TSAN \n #include "runtime/sharedRuntime.hpp""to
> the end of the include statements, so to consistent with other conditional
> includes?
>
Done.

>
> In templateTable_x86.cpp:
> In TemplateTable::tsan_release_acquire_method()
>  785   ShouldNotReachHere();
>  786   return NULL;
>
> Can we remove "return NULL"? Or would it trigger a compiler warning?
>
Yeah it triggers a compiler warning.

>
> In TemplateTable::load_field_cp_cache_entry():
> 2883 #ifdef INCLUDE_TSAN
> 2884 if (ThreadSanitizer) {
> 2885  // Draw a happens-before edge from the class's static initializer to
> 2886  // this lookup.
>
> This should use TSAN_RUNTIME_ONLY instead.
>
> -Man
>

also added a test to check that there are no races when
ThreadSanitizerJavaMemory is turned off.

new webrev: http://cr.openjdk.java.net/~aeubanks/tsanjavamem/webrev.02/