[9] RFR(S): 8155608: String intrinsic range checks are not strict enough

[Vladimir Kozlov]

Thank you, Tobias, for answering my questions.
Changes looks good.

Vladimir

On 4/29/16 12:56 AM, Tobias Hartmann wrote:
> Hi Vladimir,
>
> On 29.04.2016 01:07, Vladimir Kozlov wrote:
>> In StringUTF16.java should getChars() throw when srcBegin > srcEnd? Or there is check in other place?
>
> Yes, there is another range check in String::getChars() that throws an exception if srcBegin > srcEnd but we don't want to throw an exception in this case if StringUTF16::getChars() is invoked directly.
>
>> Hotspot test changes seems fine but do you really need new helper methods? Why not allocate dst array in TestStringIntrinsicRangeChecks.java?
>
> StringUTF16 and StringLatin1 are package private classes in java.lang because they are part of the String internals. Therefore, the test (which is not part of the java.lang package) cannot invoke the intrinsified methods. To circumvent this, we use the "patch-library approach" to inject a wrapper class into java.lang that allows us to call the package private methods from the test:
>
> 26 package java.lang;
> 27
> 28 /**
> 29  * A helper class to get access to package-private members
> 30  */
> 31 public class Helper {
>
> We use the same approach for other compiler tests as well, for example the tests in test/compiler/jsr292/.
>
> Thanks,
> Tobias
>
>> Thanks,
>> Vladimir
>>
>> On 4/28/16 3:45 AM, Tobias Hartmann wrote:
>>> Hi
>>>
>>> please review the following patch:
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8155608
>>> http://cr.openjdk.java.net/~thartmann/8155608/jdk/webrev.00/
>>> http://cr.openjdk.java.net/~thartmann/8155608/hotspot/webrev.00/
>>>
>>> Some String API methods use StringUTF16.putChar/getChar to read a char value from a byte array. For performance reasons, putChar/getChar is intrinsified by C1/C2 without range checks (like the Unsafe counterparts). The Java callers are responsible for adding the corresponding explicit range checks if necessary.
>>>
>>> I noticed that the Java level range checks in StringUTF16::compress(), StringUTF16::getChars() and StringLatin1::inflate() are not strong enough. Offset and length need to be multiplied by two because they index a char value in a byte array. I added a regression test that triggers the problem and also checks the other relevant intrinsics by invoking the methods with different arguments.
>>>
>>> Tested with regression test (-Xint/-Xcomp) and RBT (running).
>>>
>>> Thanks,
>>> Tobias
>>>