RFR (S) 8148787: StringConcatFactory exactness check produces bad bytecode when a non-arg concat is requested

Aleksey Shipilev aleksey.shipilev at oracle.com
Tue Feb 2 13:13:18 UTC 2016


On 02/02/2016 01:33 PM, Vladimir Ivanov wrote:
> Looks good.

Thanks Vladimir!

> Please, add -Xverify:all to all tests on string concat.

There is little need: all string concat tests are calling String concat
stubs from non-privileged callers, and hence are verified. E.g. new
regression test does fail with verification error even without -Xverify:all.

But it is a good idea to force verification in concat tests just in case
that ever fails:
  http://cr.openjdk.java.net/~shade/8148787/webrev.02/

This new patch flies through full JPRT without a hitch.

Cheers,
-Aleksey


> Best regards,
> Vladimir Ivanov
> 
> On 2/2/16 10:00 AM, Aleksey Shipilev wrote:
>> Anyone? This looks like a trivial fix.
>>
>> -Aleksey
>>
>> On 02/01/2016 10:47 PM, Aleksey Shipilev wrote:
>>> Hi,
>>>
>>> Please review the fix for a corner case in StringConcatFactory exactness
>>> check, which produces invalid bytecode:
>>>    https://bugs.openjdk.java.net/browse/JDK-8148787
>>>
>>> Note that this happens when all three things align:
>>>    a) BSM is called directly, as Java method -- javac would never
>>> produce
>>> such a String concat shape;
>>>    b) BC_SB_SIZED_EXACT strategy is used, so exactness check can be
>>> applied;
>>>    c) -Djava.lang.invoke.stringConcat.debug=true is set, forcing
>>> exactness check to run;
>>>
>>> The issue is with exactness debug check using a temporary local variable
>>> when the local variable table is small (like it is in non-arg case). The
>>> code can be reformulated without using temporary variables, with a
>>> creative use of "swap" instruction.
>>>
>>> Ironically, the bug is within the debugging code, so we don't care about
>>> its performance at all:
>>>    http://cr.openjdk.java.net/~shade/8148787/webrev.01/
>>>
>>> Cheers,
>>> -Aleksey
>>>
>>
>>





More information about the core-libs-dev mailing list