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

[Doug Simon]

> On 11 Jan 2019, at 10:02, Doug Simon <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 <http://cr.openjdk.java.net/~dnsimon/8215313>
>>> >> https://bugs.openjdk.java.net/browse/JDK-8215313 <https://bugs.openjdk.java.net/browse/JDK-8215313>
>>> >>
>>> >> -Doug
>>> >
>>> 
>> 
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20190112/8379b5be/attachment.html>