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.r...@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.ch...@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.thalin...@oracle.com> wrote:
>> 
>>> 
>>> On Sep 12, 2013, at 11:28 AM, David Chase <david.r.ch...@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.ch...@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.thalin...@oracle.com> wrote:
>>>>> 
>>>>>> +             // err.initCause(ex);
>>>>>> 
>>>>>> Why is this commented?
>>>>>> 
>>>>>> -- Chris
>>>> 
>>> 
>> 
>> _______________________________________________
>> mlvm-dev mailing list
>> mlvm-dev@openjdk.java.net
>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
> 

_______________________________________________
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Reply via email to