[RFR] Fix clobbered address passed to TSAN

[Man Cao]

I see the instrumentation for stores (iastore, lastore, ...) is moved from
pre-barrier to post-barrier. Is that intended?

A minor issue is that the comment for TSAN in TemplateTable::aaload() could
be updated.

If the update is just for the above two issues, no need for another webrev
and looks good.

However, I'm now a bit skeptical if TSAN read instrumentation in
TemplateTable::getfield_or_static() are still correct, as they all follow a
call to access_load_at(), which could invoke GC's read barrier and clobber
unknown number of registers. Our internal JDK8 TSAN works because there
were no read barrier in JDK8. Fortunately we at least have the
verify_oop_index() for checking now.

-Man



On Wed, Jun 26, 2019 at 5:36 PM Arthur Eubanks <aeubanks at google.com> wrote:

>
>
> On Wed, Jun 26, 2019 at 4:20 PM Man Cao <manc at google.com> wrote:
>
>> Offline chat with Arthur, for passing parameter to
>> SharedRuntime::verify_oop_index, it might be better to do the following
>> instead, so it accounts for field._disp:
>> __ leaq(c_rarg0, field);
>> __ subq(c_rarg0, field.base());
>>
> Done.
>
>>
>> And why does it need "__ get_method(c_rarg2)", as c_rarg2 seems unused?
>>
> Sorry, that's from my debugging, removed.
>
>>
>> -Man
>>
>>
>> On Wed, Jun 26, 2019 at 4:07 PM Jean Christophe Beyler <
>> jcbeyler at google.com> wrote:
>>
>>> Hi Arthur,
>>>
>>> LGTM,  I was curious if the DEBUG code you added made sense to go
>>> upstream
>>> to help diagnose issues or was this a failure that is only provoked by
>>> TSAN's instrumentation before this patch and can never happen otherwise?
>>>
>> I suppose it's possible to add this to upstream. It's something we'd
> probably have to put it behind the VerifyOops flag since I think it
> significantly slow down execution time. And I think the code around
> loading/storing from an array/field in interpreter code is pretty stable :P
>
>>
>>> Thanks,
>>> Jc
>>>
>>
> New webrev:
> http://cr.openjdk.java.net/~aeubanks/tsanarrayclobber/webrev.01/index.html
>
>