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

[Daniel D. Daugherty]

On 7/6/19 6:06 PM, David Holmes wrote:
> Hi Dan,
>
> On 6/07/2019 11:53 pm, 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/
>
> So the original code uses a loop to copy, while the new code calls 
> Copy::disjoint_words_atomic, but the implementation of that on x64 is 
> just a loop same as the original AFAICS:

Yup. I figure Erik O. will jump in here with his reasoning... :-)

Dan


>
> static void pd_disjoint_words_atomic(const HeapWord* from, HeapWord* 
> to, size_t count) {
> #ifdef AMD64
>   switch (count) {
>   case 8:  to[7] = from[7];
>   case 7:  to[6] = from[6];
>   case 6:  to[5] = from[5];
>   case 5:  to[4] = from[4];
>   case 4:  to[3] = from[3];
>   case 3:  to[2] = from[2];
>   case 2:  to[1] = from[1];
>   case 1:  to[0] = from[0];
>   case 0:  break;
>   default:
>     while (count-- > 0) {
>       *to++ = *from++;
>     }
>     break;
>   }
> #else
>
> David
> -----
>
>> 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