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

[Daniel D. Daugherty]

Serguei,

Thanks for the quick re-review!

Dan


On 7/9/19 12:34 PM, serguei.spit...@oracle.com wrote:

Hi Dan,

This looks good too.

Thanks,
Serguei


n 7/9/19 07:09, Daniel D. Daugherty wrote:

Greetings,

I've made one minor tweak based on Kim's code review.

Here's the full webrev:

http://cr.openjdk.java.net/~dcubed/8227338-webrev/1_for_jdk14.full/

Here's the incremental webrev:

http://cr.openjdk.java.net/~dcubed/8227338-webrev/1_for_jdk14.inc/

Here's the context diff:

$ hg diff

diff -r 32fe92d8b539 src/hotspot/share/interpreter/templateInterpreter.cpp --- a/src/hotspot/share/interpreter/templateInterpreter.cpp Mon Jul 08 16:58:27 2019 -0400 +++ b/src/hotspot/share/interpreter/templateInterpreter.cpp Tue Jul 09 10:02:46 2019 -0400

@@ -283,7 +283,7 @@
   // Copy non-overlapping tables.
   if (SafepointSynchronize::is_at_safepoint()) {

     // Nothing is using the table at a safepoint so skip atomic word copy.

-    while (size-- > 0) *to++ = *from++;
+    Copy::disjoint_words((HeapWord*)from, (HeapWord*)to, (size_t)size);
   } else {
     // Use atomic word copy when not at a safepoint for safety.

     Copy::disjoint_words_atomic((HeapWord*)from, (HeapWord*)to, (size_t)size);

Thanks, in advance, for questions, comments or suggestions.

Dan


On 7/6/19 9:53 AM, Daniel D. Daugherty 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