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

[David Chase]

Do these sibling bugs have numbers?

And I take it you would like to see

1) a test enhanced with ASM to do all 3 variants of this
2) DO attach the underlying cause

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
>