[RFR] Instrumentation for basic Unsafe put/get

[Man Cao]

For Unsafe_GetReference/Unsafe_PutReference, do you really need the
backslashes "\" for code within TSAN_RUNTIME_ONLY?

In Unsafe_GetReference:
void* addr = index_oop_from_field_offset_long(JNIHandles::resolve(obj),
offset);
"JNIHandles::resolve(obj)" could be replaced with "p", right?

For RacyUnsafe*Test, the stack trace check could be a bit more relaxed to
avoid flaky tests. E.g. in RacyUnsafePutIntLoopTest.java:
.shouldContain(" #1 sun.misc.Unsafe.getInt(Ljava/lang/Object;J)I
Unsafe.java:");
could become:
.shouldMatch(" #1 sun.misc.Unsafe.(get|put)Int\(Ljava/lang/Object;JI?)I?
Unsafe.java:");

In RacyUnsafePutStringLoopTest.java, Character.toString(c) may not create
new String object or change the reference value of the field x.
Can we update the value of the character as in the original test, so it
changes? I.e. char c = (char) (x.charAt(0) + 1);


Nits:

 337 UNSAFE_ENTRY(void, Unsafe_Put##Type(JNIEnv *env, jobject unsafe,
jobject obj, jlong offset, java_type x)) { \ 338   TSAN_RUNTIME_ONLY(
\ 339   void* addr =
index_oop_from_field_offset_long(JNIHandles::resolve(obj), offset); \
340     __tsan_write##size##_pc(addr,
SharedRuntime::tsan_code_location(0, 0)) \ 341   ); \

Missing a ";" after __tsan_write##size##_pc.
Indention for "void* addr = ..." is off.

In AbstractLoop.java:
* The second thread calls this method, defaults to calling run().
I think the original comments for run() and run2() are better:
For run(): Implement only this method for symmetric behavior.
For run2(): Override this method for asymmetric behavior.

-Man


On Tue, Sep 17, 2019 at 9:37 AM Arthur Eubanks <aeubanks at google.com> wrote:

> Forgot to mention:
> This does slow down usage of Unsafe in the interpreter when the TSAN
> feature is enabled at compile time, but since the interpreter is so slow,
> it doesn't matter anyway. C1/C2 have their own implementation of Unsafe, so
> those should not be affected.
>
> On Tue, Sep 17, 2019 at 9:34 AM Arthur Eubanks <aeubanks at google.com>
> wrote:
>
> > webrev:
> >
> http://cr.openjdk.java.net/~aeubanks/tsannormalunsafe/webrev.00/index.html
> >
> > This change adds instrumentation for the interpreter Unsafe
> > implementation, only for Unsafe_{Get,Put}{Reference,primitive}. Other
> > Unsafe instrumentation will come later.
> >
>