[12] RFR(S): 8215313: [AOT] java/lang/String/Split.java fails with AOTed java.base

[Tom Rodriguez]

Looks good.

tom

Doug Simon wrote on 1/12/19 4:57 AM:
> 
> 
>> On 11 Jan 2019, at 10:02, Doug Simon <doug.simon at oracle.com 
>> <mailto:doug.simon at oracle.com>> wrote:
>>
>> Hi Josef,
>>
>>> On 10 Jan 2019, at 22:52, Josef Haider <josef.haider at khg.jku.at 
>>> <mailto:josef.haider at khg.jku.at>> wrote:
>>>
>>> Agreed, cmpw/cmpb would make more sense here, i just wanted
>>> to keep the changeset minimal, since the entire method may soon be
>>> changed again, anyway.
>>>
>> Can you please say more about this? Would you recommend applying your 
>> current patch as is to fix the crash or will you have the changes you 
>> mention ready soon?
> 
> Josef has updated his fix to use cmpw/cmpb:
> 
> http://cr.openjdk.java.net/~dnsimon/8215313/
> 
> Previous webrev is now at 
> http://cr.openjdk.java.net/~dnsimon/8215313.old/20190112_1342
> 
> Dean, can you please re-review.
> 
> -Doug
> 
>>
>> -Doug
>>>> Taking another look, it seems like cmpl could be replaced with the
>>>> size-appropriate cmpb, cmpw, or cmpl based on byteMode(kind) and
>>>> findTwoCharPrefix, but I guess AMD64Assembler only supports cmpl and
>>>> cmpq right now.
>>>>
>>>> dl
>>>>
>>>> On 1/10/19 10:04 AM,dean.long at oracle.com 
>>>> <https://mail.openjdk.java.net/mailman/listinfo/hotspot-compiler-dev>  wrote:
>>>> >/Is it OK to modify the values of searchValue[i]?  If the search value />/is already sign-extended, how about sign-extending cmpResult instead />/of zero-extending searchValue? />//>/dl />//>/On 1/10/19 7:09 AM, Doug Simon wrote: />>/Please review this fix supplied by Josef Haider for an incorrect />>/compilation of String.split. />>//>>/When the String.indexOf intrinsic on AMD64 reaches the end of a />>/string, it tries to vectorize the last compare operations by reading />>/past the bounds of the character/byte array. This is not safe if the />>/out-of-bounds read would cross a page boundary, so in that case />>/characters are compared one-by-one. This is done with a />>/`cmpl`-instruction, which only works as long as the bytes/chars are />>/not sign extended. />>//>>/The fix is to simply `and` the characters we are searching for with />>/`0xff`/`0xffff` in order to eliminate any erroneous sign extensions. />>//>>/http://cr.openjdk.java.net/~dnsimon/8215313 />>/https://bugs.openjdk.java.net/browse/JDK-8215313 />>//>>/-Doug />//
>>>>
>>>
>>>
>>
>