RFR 8028347: Rewriter::scan_method asserts with array oob in RT_Baseline

Coleen Phillimore coleen.phillimore at oracle.com
Thu Nov 14 12:53:52 PST 2013


On 11/14/2013 03:22 PM, John Rose wrote:
> Rather than messing with the exceptions more, it would be better to have rewrite_invokespecial return a boolean for success, and have the caller (in the forward direction only) throw the exception.

The caller in the forward direction is the only one that can throw the 
exception.
>
> The CATCH macro use makes me especially nervous.  It's almost always the wrong answer.

Yes, it should make you nervous.
>
> Also, exceptions are wrong here (in hindsight) because they cause rewrite_invokespecial to fail out halfway, and then it's probably wrong to reverse all the way.  I.e., suppose you have three invokespecials, of which the second fails:  You will then *not* rewrite the third, and when attempting to reverse all three of them (during failure processing) you will probably do something wrong trying to reverse the third which was *not* rewritten.

If any of the invokespecials fail, this exception will propagate to the 
caller and the exception is stored for the entire class and the class is 
in error.   None of the reversing will happen in this case.   The case 
where we were reversing the rewrite is for OOM where we can reverse and 
try the rewriting again.

The simplest thing would be to crash the VM.   Returning an exit code 
has the same effect as throwing the exception.   In this case, you can't 
unrewrite.  The index has overflowed the field it needs to be stored 
in.   It's really more complicated to store a result than propagate the 
exception.

>
> So (in hindsight) exceptions are the wrong tool for the job here; accumulate a "failing" boolean, finish the rewrite pass, and then un-rewrite it all cleanly before throwing the exception.
>
> Sorry I didn't catch this.  When I have worked on rewriter.cpp, I have stayed away from exceptions for these sorts of reasons, and I should have called it out on first review.

Let's get rid of the rewriter for jdk9 instead, okay?  This regression 
is going to cause headaches for the promotion to jdk8 that we had to get 
some default methods changes in for.

thanks,
Coleen

>
> — John
>
> On Nov 14, 2013, at 12:01 PM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:
>
>> Summary: Fix reversing rewriting for invokespecial (which I broke with last checkin)
>>
>> Ran runtime jtreg tests (including failing tests).   Rerunning vm.mlvm.testlist.
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/8028347/
>> bug link https://bugs.openjdk.java.net/browse/JDK-8028347
>>
>> Thanks,
>> Coleen
>>
>>



More information about the hotspot-runtime-dev mailing list