RFR (JDK11) 8137326: Methods for comparing CharSequence, StringBuilder, and StringBuffer

[Roger Riggs]

+1; looks good

On 2/27/2018 3:15 PM, Xueming Shen wrote:
> +1
>
> On 2/27/18, 11:37 AM, Joe Wang wrote:
>> Hi Sherman and all,
>>
>> Thanks for the further reviews!
>>
>> Here's the latest webrev with boundary checks in 
>> StringLatin1/StringUTF16:
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
>> webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/
>>
>> Best,
>> Joe
>>
>> On 2/22/2018 6:07 PM, Joe Wang wrote:
>>>
>>>
>>> On 2/22/18, 12:51 PM, Xueming Shen wrote:
>>>> On 2/22/18, 12:04 PM, Joe Wang wrote:
>>>>> Hi Sherman,
>>>>>
>>>>> Thanks for reviewing the change.
>>>>>
>>>>> Taking a local copy of the count field, but the boundary check 
>>>>> would be almost immediately done against the field itself. Are you 
>>>>> worrying about the count field may be out of sync with the byte 
>>>>> array? I would think that's unlikely to happen. Whether it's 
>>>>> StringBuilder or StringBuffer, it's not advisable/practical to use 
>>>>> in multiple threads. In a valid usage, the count is always 
>>>>> consistent with the byte array.
>>>>>
>>>>
>>>> Hi Joe,
>>>>
>>>> It might not be a "valid usage" but it did happen and when it 
>>>> happens it might just crash the
>>>> vm without those boundary checks. It's especially true for those 
>>>> intrinsics methods with explicit
>>>> comments "intrinsic performs no bounds checks". In this case, the 
>>>> StringUTF16.getChar() is being
>>>> called in new public method StringUTF16.compareTo(byte[], byte[], 
>>>> int, int) without appropriate
>>>> boundary check. In the "old" code the "index" is guaranteed to be 
>>>> within [0, len) in
>>>> StringUTF16.compareTo(byte[], byte[]), so it's safe. A real case 
>>>> for such scenario can be found in
>>>> JDK-8158168 [1], for example.
>>>
>>> Thanks for the pointer! The email thread helps a lot. I've updated 
>>> the webrev with a boundary check in ASB (AbstractStringBuilder line 
>>> 106, 107), and then a note to StringUTF16.compareTo (StringUTF16 
>>> line 280). Hopefully this is sufficient. Didn't want to add any 
>>> check in StringUTF16 since that may affect the original two-arg method.
>>>
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
>>> webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/
>>>
>>> -Joe
>>>
>>>>
>>>> -Sherman
>>>>
>>>>
>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8158168
>>
>