> On Jul 6, 2019, at 9:53 AM, Daniel D. Daugherty <daniel.daughe...@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. ------------------------------------------------------------------------------