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

[Joe Wang]

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