RFR: JDK-8214114: Switch expressions with try-catch statements

[Jan Lahoda]

On 7.12.2018 00:42, Maurizio Cimadamore wrote:
>
> On 06/12/2018 23:24, Jan Lahoda wrote:
>> On 6.12.2018 23:04, Maurizio Cimadamore wrote:
>>> While I don't think the stack should be as what Vicente says (after all,
>>> there's a Println object somewhere that should be tracked), I tend to
>>> agree with Vicente that there's something fishy here - I looked at the
>>> bug report and also run the example and the verifier crashes _before_
>>> entering the exception handler - it crashes on the 'new' when the
>>> exception is allocated. That doesn't seem to match what you said in the
>>> RFR. Now, as I pointed out in my other message, I believe some stashing
>>> is unavoidable as the VM will wipe out the stack, but it seems like
>>> there could be more issues piled up here?
>>
>> Sorry, I should have been more precise. What I meant is that the
>> StackMap for catch handler must not have anything on stack (except for
>> the Exception) (because VM will wipe the stack, that is, I think,
>> another angle to look at this). That is not related only to athrow -
>> the exception may be coming from a method invocation, for example. The
>> reason the verifier barks on "new" is, I think, because the
>> instruction may throw an exception as well. So it checks the StackMap
>> for the exception handler, and finds out there's something on the
>> stack (in the StackMap) that should not be there, and fails (or, from
>> another angle, when checking the exception handler, the verifier finds
>> out the handler expects something on the stack, besides the exception,
>> which can't be there).
>
> And sorry for the confusion - I realized I misread the verifier error -

I apologize to all - I should have been more precise/detailed in the 
original e-mail.

> it's actually pointing at the handler entry (as javac generated both
> [PrintStream, Exception], while the VM expects just [Exception] - it
> being an handler). So what you did is correct and I think just fixing
> the stackmap is not a real solution as then we need a way to recover the
> values before leaving the switch.

Yes, exactly.

Thanks,
     Jan

>
> Maurizio
>
>>
>> Jan
>>
>>>
>>> Maurizio
>>>
>>> On 06/12/2018 21:50, Vicente Romero wrote:
>>>> Hi Jan,
>>>>
>>>> Probably I'm missing something but shouldn't it be enough to generate
>>>> the right stackframe for the exception handler? Which in this case
>>>> should only contain:
>>>>
>>>> stack = [ class java/lang/Exception ]
>>>>
>>>> Thanks,
>>>> Vicente
>>>>
>>>> On 12/6/18 11:51 AM, Jan Lahoda wrote:
>>>>> Hi,
>>>>>
>>>>> Switch expressions that contain try-catch statements don't work
>>>>> currently, as the catch handlers require that the stack is empty when
>>>>> the handler is invoked, but this may not be true when the switch
>>>>> expression is inside an expression evaluation (and so some values are
>>>>> already on the stack).
>>>>>
>>>>> The proposed solution is to stash the stack content to local
>>>>> variables before "running" the switch expression, and fill the stack
>>>>> content back when breaking out of the switch expression. This is done
>>>>> only if the try-catch statement is present in the switch expression,
>>>>> as it produces longer bytecode.
>>>>>
>>>>> Also, it turned out variables inside switch expressions in field
>>>>> initializers don't work properly (as their owner is the field, not a
>>>>> method), attempted to fix that as well.
>>>>>
>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8214114
>>>>> Webrev: http://cr.openjdk.java.net/~jlahoda/8214114/webrev.00/
>>>>>
>>>>> How does this look?
>>>>>
>>>>> Thanks,
>>>>>     Jan
>>>>