RFR(S+M) / 8022701 Accessibility checking: InvocationTargetException is thrown instead of IllegalAccessError

John Rose john.r.rose at oracle.com
Thu Sep 12 21:02:41 PDT 2013


On Sep 12, 2013, at 5:44 PM, David Chase <david.r.chase at oracle.com> wrote:

> Do these sibling bugs have numbers?

Yes, 8022701.  I just updated the bug to explain their common genesis.

> And I take it you would like to see
> 
> 1) a test enhanced with ASM to do all 3 variants of this

Yes, please.  The case of "no such field" does not have a direct cause from lambda expressions AFAIK, but it can occur with "raw" CONSTANT_MethodHandles.  (It would be reasonable for javac to emit CONSTANT_MH's for fields in some very limited circumstances, but I'll bet it doesn't.)

> 2) DO attach the underlying cause

Yes.  Read the javadoc for ExceptionInInitializerError for an example of why users want the underlying cause for linkage errors.

(Golly, I wonder what happens if resolution of a CONSTANT_MethodHandle tries to touch a class with a booby-trapped <clinit>.  I hope it's the case that the ExceptionInInitializerError just passes straight up the stack.  But if it were to get wrapped in a ROE of some sort, it would probably bubble up as an IAE.  Could be a charming exploration.  Actually, no, I don't want to go in there.  Getting all possible linkage errors correct is fine, but we have more important things to do.)

— John

> David
> 
> On 2013-09-12, at 5:35 PM, John Rose <john.r.rose at oracle.com> wrote:
> 
>> It looks good.  I have three requests.
>> 
>> Regarding this comment:
>> +     * See MethodSupplier.java to see how to produce these bytes.
>> +     * They encode that class, except that method m is private, not public.
>> 
>> The recipe is incomplete, since it does not say which bits to tweak to make m private.  Please add that step to the comments more explicitly, or if possible to the code (so bogusMethodSupplier is a clean copy of the od output).  I suppose you could ask sed to change the byte for you.  That will make this test a better example for future tests, and make it easier to maintain if javac output changes.  The high road to use would be asm, although for a single byte tweak od works OK.
>> 
>> Also, this bug has two twins.  The same thing will happen with NoSuchMethodE* and NoSuchFieldE*.  Can you please make fixes for those guys also?
>> 
>> FTR, see MemberName.makeAccessException() for logic which maps the other way, from *Error to *Exception.  I don't see a direct way to avoid the double mapping (Error=>Exception=>Error) when it occurs.
>> 
>> For the initCause bit we should do this:
>> 
>>       } catch (IllegalAccessException ex) {
>>           Error err = new IllegalAccessError(ex.getMessage());
>>           err.initCause(ex.getCause());  // reuse underlying cause of ex
>>           throw err;
>>       } ... and for NSME and NSFE...
>> 
>> That way users can avoid the duplicate exception but can see any underlying causes that the JVM may include.
>> 
>> Thanks for untangling this!
>> 
>> — John
>> 
>> On Sep 12, 2013, at 12:17 PM, David Chase <david.r.chase at oracle.com> wrote:
>> 
>>> The test is adapted from the test in the bug report.
>>> The headline on the bug report is wrong -- because it uses reflection in the test to do the invocation,
>>> the thrown exception is wrapped with InvocationTargetException, which is completely correct.
>>> HOWEVER, the exception inside the wrapper is the wrong one.
>>> 
>>> The new test checks the exception in both the reflective-invocation and plain-invocation cases,
>>> and also checks both a method handle call (which threw the wrong exception) and a plain
>>> call (which threw, and throws, the right exception).
>>> 
>>> The test also uses a tweaked classloader to substitute the borked bytecodes necessary to get
>>> the error, so it is not only shell-free, it can also be run outside jtreg.
>>> 
>>> On 2013-09-12, at 2:34 PM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
>>> 
>>>> 
>>>> On Sep 12, 2013, at 11:28 AM, David Chase <david.r.chase at oracle.com> wrote:
>>>> 
>>>>> New webrev, commented line removed:
>>>>> http://cr.openjdk.java.net/~drchase/8022701/webrev.03/
>>>> 
>>>> I think the change is good given that the tests work now.  Is your test derived from the test in the bug report?
>>>> 
>>>> And it would be good if John could also have a quick look (just be to be sure).
>>>> 
>>>> -- Chris
>>>> 
>>>>> 
>>>>> On 2013-09-12, at 1:53 PM, David Chase <david.r.chase at oracle.com> wrote:
>>>>> 
>>>>>> I believe it produced extraneous output -- it was not clear to me that it was either necessary or useful to fully describe all the converted exceptions that lead to the defined exception being thrown.  The commented line should have just been removed (I think).
>>>>>> 
>>>>>> On 2013-09-12, at 1:24 PM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
>>>>>> 
>>>>>>> +             // err.initCause(ex);
>>>>>>> 
>>>>>>> Why is this commented?
>>>>>>> 
>>>>>>> -- Chris
>>>>> 
>>>> 
>>> 
>>> _______________________________________________
>>> mlvm-dev mailing list
>>> mlvm-dev at openjdk.java.net
>>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>> 
> 



More information about the mlvm-dev mailing list