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

[harold seigel]

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