+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