RFR (S + L test) : 8016839 : JSR292: AME instead of IAE when calling a method

Karen Kinnear karen.kinnear at oracle.com
Fri Nov 22 19:56:57 UTC 2013


Thank you so much for finding a way to do this. We do think this is important to get in for 8.
And thank you for a way to check the hotspot changes in without waiting for the jdk changes.

Code looks good.

Couple of minor comments:
1. universe.cpp - when the hotspot change gets in and the jdk change isn't in yet, are
we going to see the tty->print_cr message?
2. style: Method* rather than Method  space *
3. Head's up that ASM is being updated for default method support
  - visitMethoInsn will take an additional argument at the end for "isinterface" to determine
if it needs a methodref or an interfacemethodref in the constant pool. I think that is in b117 (?)
so we don't have it yet - so this will need modifying later - don't let that stop you from pushing the

See Lois's test webrev 


On Nov 22, 2013, at 2:07 PM, David Chase wrote:

> Updated request.
> This is for a bug that it "deferred" by compilers,
> but runtime really wants it fixed because they
> are working in the same area and don't like it at all.
> In particular, they want it committed to hotspot-rt ASAP
> (don't want to wait for the multiweek turnaround)
> and thus the diffs and testing are against hotspot-rt.
> There are jdk and hotspot webrevs; the hotspot webrev
> is designed to allow it to run (with old behavior) in the
> absence of the jdk change.
> Bug: https://bugs.openjdk.java.net/browse/JDK-8016839
> Related test bug: https://bugs.openjdk.java.net/browse/JDK-8027733 (confidential, sorry)
> Webrev(s):
> http://cr.openjdk.java.net/~drchase/8016839/webrev-hotspot.01/
> http://cr.openjdk.java.net/~drchase/8016839/webrev-jdk.01/
> Testing:
> ute vm.quick.testlist A/B, no (new) failures
> jtreg compiler runtime sanity, no (new) failures
> defmeth, no new failures, fewer old failures
> New test, specifically tested on Sparc, Intel, embedded
> -Xint, -Xcomp, plain, and with jdb on embedded, and
> Netbeans on x86.  (this was specifically to address the main
> uncertainty, which was calling a zero-arg exception-throwing
> method instead of the expected perhaps-multi-arg inaccessible
> method; it is possible to devise calling conventions that will
> make that not work, but we appear not to have done this.  The test
> includes 0- and 11-arg instances of the not-legally-accessible
> methods).
> Problem: 
> Various patterns of inheritance and faulty overriding are supposed to
> throw IllegalAccessError (IAE). At the time the bug was filed, they
> instead threw NullPointerException (NPE) but after a related bug was
> fixed the wrong exception changed to AbstractMethodError (AME).
> The root cause of the error is that (1) by convention, if an itable
> entry is null, AME is thrown, and (2) in the case of inaccessible
> methods overriding other methods that implement interface methods, a
> null was entered into the itable (leading to throws of AME on code paths
> that use the itable).
> This is a long-standing problem dating back to at least JDK 6, but made
> much easier to observe by the introduction of method handles.  Before
> methodhandles this bug was more difficult to observe because resolution
> of invokeinterface would aggressively evaluate errors and leave the
> invoke unresolved if there was an error; thus repeated testing with
> error receivers would always throw the proper error.  If, however, a
> good receiver is used for the invokeinterface, then resolution succeeds,
> and subsequent invocations with bad receivers throws the wrong error
> (AME instead of NPE) because the resolved code goes directly to the
> itable and sees the null method.
> With Methodhandles, resolution and invocation are separate, and it
> always throws the wrong error.
> Fix:
> I considered (and discussed with John Rose and Coleen Phillimore and
> Karen Kinnear) use of a "second null value", where instead of using 0x0
> for the missing method, I would use 0x1, and then special case the null
> checks (in triplicate, across all platforms) to instead unsigned compare
> against a small integer and also special-case the trap handler for this
> value.
> This seemed like a big change, potentially confusing to C programmers,
> hacky, and somewhat hateful.
> I next considered creating a "bridge to nowhere" stub that would always
> throw IAE, and fill that Method * in instead of null.  Unfortunately,
> the need for this stub is not discovered until after constant pool
> rewriting has occurred, and adding new methods and CPC entries after
> the rewriting looked very hairy.
> The last choice was to define a helper method in sun.misc.Unsafe that
> would always throw IllegalAccessError when called, and use that Method *
> in the should-throw-IAE case.  The risk here is that the helper method
> is zero-arg, where the method about to be called (and whose parameters
> had been pushed on the stack) could have a different set of arguments,
> and perhaps there is some platform where this does not work cleanly.
> However, the existing code that checks for null and throws AME seems to
> not be special-cased for number of args (that is, it branches to a
> generic exception thrower with no additional cleanup).  In addition, I
> enhanced the test to try faulty invocations with both zero and 11 args,
> and to run under jtreg in several modes, and I added iteration to help
> force code to the compiled case as well, and it has performed properly
> on Sparc, Intel, and embedded.

More information about the core-libs-dev mailing list