[RFR] Java memory instrumentation for TSAN
Man Cao
manc at google.com
Fri May 24 21:20:15 UTC 2019
We can add the "ThreadSanitizerJavaMem" flag, so it is consistent with our
internal implementation.
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");
In templateTable_x86.cpp:
TemplateTable::tsan_observe_get_or_put()
....
if (ThreadSanitizer) {
__ jmp(Done);
TemplateTable::tsan_observe_load_or_store()
It can add an assert for ThreadSanitizer, and check
for ThreadSanitizerJavaMem instead.
Other minor issues:
In arguments.cpp, can we change it to:
TSAN_RUNTIME_ONLY(
set_mode_flags..
FLAG_SET_ERGO..
FLAG_SET_ERGO..
);
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?
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?
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
On Wed, May 22, 2019 at 4:11 PM Arthur Eubanks <aeubanks at google.com> wrote:
> Sorry about that, this change was done before all this was in place and I
> forgot to update it. Updated in
> http://cr.openjdk.java.net/~aeubanks/tsanjavamem/webrev.01/index.html.
>
> On Wed, May 22, 2019 at 11:13 AM Man Cao <manc at google.com> wrote:
>
>> Much of the code is not guarded by the INCLUDE_TSAN or TSAN_RUNTIME_ONLY
>> macro.
>> Can we update it accordingly?
>>
>> -Man
>>
>>
>> On Tue, May 21, 2019 at 3:37 PM Arthur Eubanks <aeubanks at google.com>
>> wrote:
>>
>>> webrev:
>>> http://cr.openjdk.java.net/~aeubanks/tsanjavamem/webrev.00/index.html
>>>
>>> This change adds instrumentation for Java memory accesses.
>>>
>>
More information about the tsan-dev
mailing list