RFR (S + L test) : 8016839 : JSR292: NPE instead of IAE when calling a method
David Chase
david.r.chase at oracle.com
Mon Nov 18 12:26:46 PST 2013
Bug: https://bugs.openjdk.java.net/browse/JDK-8016839
Webrev(s):
http://cr.openjdk.java.net/~drchase/8016839/webrev-hotspot.00/
http://cr.openjdk.java.net/~drchase/8016839/webrev-jdk.00/
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 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
and Intel (ARM and PPC are coming, as soon as I can convince JPRT to create the necessary binaries).
Testing: final tests TBD, I have been hand-testing (jtreg, defmeth) with no problems.
Test case:
The test is an enhanced version of the old test for AME-not-NPE. It covers several additional cases
of inaccessibility (protected, packaged-protected in a different package) and also includes options
to write out the ASM-generated classes to jar files for offline inspection and modification (we might want
to put that somewhere that it's easy to reuse, if we like it). I write jar files to deal with the two problems
of not knowing much about the platform file system, and because the same class name is used several times
in the tests. The "write" mode writes out the files, the "read" mode use those same files instead of the
program-supplied bytes.
More information about the hotspot-compiler-dev
mailing list