Request for review (S): 6865265 JVM crashes with "missing exception handler" error

[Vladimir Kozlov]

Thank you, Volker

Your changes looks good to me. I will push it when we done with 7u2, may be next 
week. About your question:

Question: is it correct that we implicitly create 'catch_all' CatchProjNode in 
Parse::catch_call_exceptions() with an handler_bci which is not '-1'?

'catch_all_index' catch projection is only checked in extract projections code 
when Java call is replaced with inlined code or when Allocate nodes are 
expanded. So it is fine to create "catch_all_index" CatchProjNode with not '-1' 
bci in catch_call_exceptions() since it is called only after runtime call to 
rethrow_Java() where all projections are treated equally (except fall_through 
which is dead).

Thanks,
Vladimir

Volker Simonis wrote:
> The following request for review somehow got lost in the summer holidays.
> 
> I would need one additional reviewer and somebody who would be so kind
> to actually submit the change once it was reviewed.
> 
> Thank you and best regards,
> Volker
> 
> 
> On Tue, Jul 26, 2011 at 7:20 PM, Volker Simonis
> <volker.simonis at gmail.com> wrote:
>> Hi,
>>
>> I've made "original_exception" a Handle as suggested by Keith. Here's
>> the new webrev:
>>
>> http://www.progdoc.de/webrev/6865265.v2
>>
>> Regarding Keith' comments about the verification process: he's right,
>> 'VerificationType::is_reference_assignable_from()' loads both
>> references with the same class loader (the initiating one in this
>> case). Initially I though that verifying if an Exception class in the
>> class file's exception table is "java.lang.Throwable" only by name may
>> be not enough, because a custom system class loader could load a bogus
>> Throwable class which is different from the one loaded by the boot
>> class loader. But I've just verified that such a scenario is
>> prohibited by 'ClassLoader.defineClass()' which is final and rejects
>> the loading of classes from the 'java.' package.
>>
>> Regards,
>> Volker
>>
>> On Mon, Jul 25, 2011 at 8:19 PM, Vladimir Kozlov
>> <vladimir.kozlov at oracle.com> wrote:
>>> Resending to all.
>>>
>>> Keith McGuigan wrote:
>>>> I think the code looks ok, but why not use a Handle for the
>>>> "original_exception" in runtime.cpp -- then you don't need to worry about
>>>> the GC case either.
>>>>
>>>> As to the question about the verifier comparing by name, this is correct
>>>> (in that this is what the verifier spec requires, IIRC), but intuitively it
>>>> makes sense anyway because the class's current class loader is the
>>>> initiating loader for any referenced class that might need to be loaded.
>>>>  Thus two different references to classes with the same name will always
>>>> resolve to the same class implementation so a system dictionary lookup is
>>>> unnecessary.  The verifier actively attempts to NOT load or initialize
>>>> classes when it can, but in some cases it must, unfortunately.
>>>>
>>>> --
>>>> - Keith
>>>>
>>>>
>>>> On Jul 25, 2011, at 1:14 PM, Vladimir Kozlov wrote:
>>>>
>>>>> Forwarding to RT since runtime code is also involved.
>>>>>
>>>>> Vladimir
>>>>>
>>>>> -------- Original Message --------
>>>>> Subject: Request for review (S): 6865265 JVM crashes with "missing
>>>>> exception handler" error
>>>>> Date: Mon, 25 Jul 2011 18:58:58 +0200
>>>>> From: Volker Simonis <volker.simonis at gmail.com>
>>>>> To: hotspot compiler <hotspot-compiler-dev at openjdk.java.net>
>>>>>
>>>>> Although I've found a tiny test case for 6865265 and a small fix for
>>>>> the problem, I'm still not sure if my fix is complete.
>>>>>
>>>>> I would appreciate it very much if somebody could review my (somewhat
>>>>> lengthy) explanation for the fix and answer the two questions I
>>>>> encountered.
>>>>> Both, the explanation of the fix and the questions are in the webrev at:
>>>>>
>>>>> http://www.progdoc.de/webrev/6865265/
>>>>>
>>>>> Regards,
>>>>> Volker