RFR(XS) 8129897: Old verifier fails to reject erroneous cast from boolean[] to byte[]

[harold seigel]

Thanks David!

Harold

On 7/22/2015 9:32 PM, David Holmes wrote:
> On 23/07/2015 1:06 AM, harold seigel wrote:
>> Hi David,
>>
>> Thanks for the review!
>>
>> I updated lines 2256- 2261 in a new webrev:
>> http://cr.openjdk.java.net/~hseigel/bug_8129897.2/
>>
>> Here are the new lines:
>>
>> 2256                     case 'B':    /* array of bytes or booleans */
>> 2257                         if (top_type != MAKE_FULLINFO(ITEM_Byte, 1,
>> 0) &&
>> 2258                             top_type != MAKE_FULLINFO(ITEM_Boolean,
>> 1, 0))
>> 2259                             CCerror(context,
>> 2260                                   "Expecting to find array of bytes
>> or Booleans on stack");
>> 2261                         break;
>>
>>
>> File jdk/src/java.base/share/native/libverify/opcodes.in_out defines the
>> first operand for opcodes baload and bastore as [B.  But, according to
>> the JVM Spec, the first operand for these bytecodes can be either [B or
>> [Z.  This code checks that the opcodes, that opcodes.in_out says expect
>> [B, do not throw VerifyError if their actual operand is either [B or
>> [Z.  Opcodes baload and bastore are the only opcodes in opcodes.in_out
>> with [B operands, and that's unlikely to change.
>>
>> I removed "case 'Z': because there are no [Z operands for any opcodes in
>> opcodes.in_out.
>
> Yeah they were too miserly to give booleans their own bytecodes :)
>
> Thanks for clarifying and changing.
>
> David
>
>
>> Thanks, Harold
>>
>> On 7/21/2015 7:44 PM, David Holmes wrote:
>>> Hi Harold,
>>>
>>> On 22/07/2015 6:21 AM, harold seigel wrote:
>>>> Hi,
>>>>
>>>> Please review this small change to fix bug 8129897.  The old verifier
>>>> treats arrays of Booleans as arrays of bytes, allowing assignability
>>>> between them.  This fix distinguishes between arrays of Booleans and
>>>> arrays of bytes and so disallows assignability between them. 
>>>> Additional
>>>> changes were needed to make sure operands to baload and similar 
>>>> opcodes
>>>> could still be either Boolean or byte arrays.
>>>>
>>>> Open webrev: http://cr.openjdk.java.net/~hseigel/bug_8129897/
>>>
>>> Seems reasonable. Only query I have was why you didn't split this:
>>>
>>> 2256                     case 'B': case 'Z':   /* array of bytes or
>>> booleans */
>>> 2257                         if (top_type != MAKE_FULLINFO(ITEM_Byte,
>>> 1, 0) &&
>>> 2258                             top_type !=
>>> MAKE_FULLINFO(ITEM_Boolean, 1, 0))
>>> 2259                             CCerror(context,
>>> 2260                                   "Expecting to find array of
>>> bytes or Booleans on stack");
>>> 2261                         break;
>>>
>>> into two distinct cases?
>>>
>>> Thanks,
>>> David
>>>
>>>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8129897
>>>>
>>>> The fix was tested with JCK Lang and VM tests, the UTE quick and split
>>>> verifier tests, and the hotspot, and JDK vm, java/io, java/lang, and
>>>> java/util JTreg tests.
>>>>
>>>> This scenario will be tested by JCK-9.  So no regression test was
>>>> included in this webrev.
>>>>
>>>> Thanks, Harold
>>