Recommended changes made:
http://cr.openjdk.java.net/~drchase/8022701/webrev.04/
Test with jtreg (for pass and for induced failure) on MacOS,
not sure what additional other testing is desired since it is entirely in the
libraries.
I included code to handle the case of a broken field-referencing methodhandle,
but did not test it because there's no syntax for these in Java, nor is their
creation
well-documented.
David
On 2013-09-13, at 12:02 AM, John Rose <john.r.r...@oracle.com> wrote:
> On Sep 12, 2013, at 5:44 PM, David Chase <david.r.ch...@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.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-...@openjdk.java.net
>>>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>>>
>>
>
