[12] RFR(S): 8215313: [AOT] java/lang/String/Split.java fails with AOTed java.base
Doug Simon
doug.simon at oracle.com
Mon Jan 14 09:16:31 UTC 2019
> On 13 Jan 2019, at 23:07, dean.long at oracle.com wrote:
>
> Looks good.
Thanks for the review.
> Please update copyright with 2019 end year.
Done.
-Doug
>
> On 1/12/19 4:57 AM, Doug Simon wrote:
>>
>>
>>> 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/ <http://cr.openjdk.java.net/~dnsimon/8215313/>
>>
>> Previous webrev is now at http://cr.openjdk.java.net/~dnsimon/8215313.old/20190112_1342 <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/20190114/54b8c598/attachment.html>
More information about the hotspot-compiler-dev
mailing list