RFR(S) 7127066: Class verifier accepts an invalid class file

[Karen Kinnear]

Harold,

Looks really good. Thank you very much for the revision.

thanks,
Karen

On Mar 20, 2015, at 1:52 PM, harold seigel wrote:

> Hi Karen,
> 
> Thank you for your comments.
> 
> Bytecode istore can change the type of a local slot.  So, here's a new webrev that uses the incoming type state for all of the astore*, dstore*, fstore*, istore*, and lstore* bytecodes.  The webrev also contains a new test for the istore bytecode case, called BadMapIstore.jasm.
> 
> Could you please review this webrev, when you have a chance?
> 
> New open webrev:  http://cr.openjdk.java.net/~hseigel/bug_7127066.2/
> 
> Thanks! Harold
> 
> On 3/19/2015 3:33 PM, Karen Kinnear wrote:
>> Harold,
>> 
>> I get it - this only affects instructions that set the type of a local slot since the operand stack is cleared.
>> 
>> So - what about istore?
>> If we have a local slot that contains a long or double, Frederic reminded me that JVMS 2.6.1 does allow stores to those separate slots,
>> e.g. an istore. My mental model of this is that it would change the type of the local slot.
>> 
>> Would that mean we would want to check any of the *store bytecodes?
>> 
>> (I know the spec would change for all bytecodes which might be cleaner - I suspect you are being extra cautious about making changes
>> to reduce risk of breaking something in the field).
>> 
>> thanks,
>> Karen
>> 
>> On Mar 18, 2015, at 2:18 PM, harold seigel wrote:
>> 
>>> Hi Karen,
>>> 
>>> The fix does not affect normal verifier type-state (stack map) checking that is done when looping through the bytecodes.  It only affects whether the incoming or outgoing type-state is used when comparing the type-state at a particular bytecode with the type-state at the start of its potential exception handlers.
>>> 
>>> In addition, (to paraphrase Keith's comment in the bug), it only affects instructions that set the type of a local slot (astore and friends) ... .  Instructions that affect the expression stack are not a problem since the type-state's stack is cleared when type checking an exception handler.
>>> 
>>> So, other bytecodes such as aload and friends, getstatic and getfield, etc.  are not an issue.
>>> 
>>> Thanks, Harold
>>> 
>>> On 3/16/2015 3:49 PM, Karen Kinnear wrote:
>>>> Harold,
>>>> 
>>>> Thanks for helping me walk through this in more detail.
>>>> 
>>>> The way I read this, the fix would apply to all bytecodes - except for
>>>> invokespecial <init> - which is handled I believe correctly inside the
>>>> verify_invoke_init.
>>>> 
>>>> So if you could possibly experiment with some additional instructions - I suspect
>>>> you can make a conditional check where you put the beginning check and remove
>>>> the check at the end.
>>>> 
>>>> thanks,
>>>> Karen
>>>> 
>>>> On Mar 15, 2015, at 8:58 PM, David Holmes wrote:
>>>> 
>>>>> Hi Harold,
>>>>> On 14/03/2015 4:06 AM, harold seigel wrote:
>>>>>> Hi,
>>>>>> 
>>>>>> Please review this fix for bug JDK-7127066.  The fix applies to astore*
>>>>>> bytecodes because, when inside an exception handler, they can reference
>>>>>> the thrown object and modify the number of stack locals, enabling the
>>>>>> incorrect stack match.
>>>>>> 
>>>>>> Open webrev: http://oklahoma.us.oracle.com/~hseigel/webrev/bug_7127066/
>>>>>> 
>>>>>> JBS bug: https://bugs.openjdk.java.net/browse/JDK-7127066
>>>>>> 
>>>>>> The fix was tested with JCK api, lang, and vm tests, jtreg hotspot,
>>>>>> java/lang, java/io, and java/util tests, and testbase quick and split
>>>>>> verifier tests, and with the test case provided in the bug.
>>>>> The new check looks okay, though I can't verify the exact placement of it.
>>>>> 
>>>>> Thanks,
>>>>> David
>>>>> 
>>>>>> Thanks! Harold
>