RFR(XXS): 8227338: templateInterpreter.cpp: copy_table() needs to be safer

Kim Barrett kim.barrett at oracle.com
Mon Jul 8 23:08:19 UTC 2019


> On Jul 6, 2019, at 9:53 AM, Daniel D. Daugherty <daniel.daugherty at oracle.com> wrote:
> 
> Greetings,
> 
> During the code review for the following fix:
> 
>     JDK-8227117 normal interpreter table is not restored after single stepping with TLH
>     https://bugs.openjdk.java.net/browse/JDK-8227117
> 
> Erik O. noticed a potential race with templateInterpreter.cpp: copy_table()
> depending on C++ compiler optimizations. The following bug is being used
> to fix this issue:
> 
>     JDK-8227338 templateInterpreter.cpp: copy_table() needs to be safer
>     https://bugs.openjdk.java.net/browse/JDK-8227338
> 
> Here's the webrev URL:
> 
>     http://cr.openjdk.java.net/~dcubed/8227338-webrev/0_for_jdk14/
> 
> This fix has been tested via Mach5 Tier[1-3] on Oracle's usual platforms.
> Mach5 tier[4-6] is running now. It has also been tested with the manual
> jdb test from JDK-8227117 using 'release' and 'fastdebug' bits.
> 
> Thanks, in advance, for questions, comments or suggestions.
> 
> Dan

[This review is ignoring the issues around the current implementation
of atomic copies discussed elsewhere in this thread. I assume those
will be addressed elsewhere.]

------------------------------------------------------------------------------ 
src/hotspot/share/interpreter/templateInterpreter.cpp
 286     while (size-- > 0) *to++ = *from++;

[pre-existing]

This ought to be using Copy::disjoint_words.  That's even more obvious
in conjunction with the change to use Copy::disjoint_words_atomic in
the non-safepoint case.

------------------------------------------------------------------------------ 
src/hotspot/share/interpreter/templateInterpreter.cpp
 284   if (SafepointSynchronize::is_at_safepoint()) {

I wonder how much benefit we really get from having distinct safepoint
and non-safepoint cases, rather than just unconditionally using
Copy::disjoint_words_atomic.

------------------------------------------------------------------------------



More information about the serviceability-dev mailing list