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.spitsyn at 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
>>>
>>
>