JDK-8073432 : Object.getClass() throws stackless NPE, due to C2 intrinsic
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 9
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2015-02-18
  • Updated: 2015-06-04
  • Resolved: 2015-03-13
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 9
9 teamFixed
Related Reports
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Description
C2 intrinsifies Object.getClass(), and it puts the fastthrow-ed NPE exception on failure path.
This means users that (blindly) use obj.getClass() as weird make-shift NPE check will receive
the stackless NPE, which will confuse them and/or complicate debugging.

Sample program that prints the number of lines in the (exception + stacktrace) dump:

import java.io.PrintWriter;
import java.io.StringWriter;

public class Test {
        public static void main(String... args) {
                for (int c = 0; c < 1000000; c++) {
                        try {
                                callWith(null);
                        } catch (NullPointerException ex) {
                                StringWriter sw = new StringWriter();
                                PrintWriter pw = new PrintWriter(sw);
                                ex.printStackTrace(pw);
                                pw.close();
                                System.out.println(sw.toString().split("\n").length);
                        }
                }
        }

        public static void callWith(Object obj) {
                obj.getClass();
        }
}

Yields:

$ java Test | uniq -c 
   5772 3
  55684 1
  48128 3
 890416 1

I interpret this as follows: 5772 invocations happened in interpreter mode and got the full stack trace, then C2 kicked in and 55684 invocations ran with stackless NPEs, then deopt happened and we bailed to interpreter again, and finally we reached C2 where we have all the stackless NPEs. Enabling the stacktraces in fastthrow expectedly gives:

$ java -XX:-OmitStackTraceInFastThrow Test | uniq -c 
1000000 3

From my brief read of C2 code, fastthrow-ed built-in exceptions seem to serve the deoptimization goals, and I am not sure they should leak to the users.
Comments
Unless someone fixes the bad changeset that used the wrong CR number this CR will keep getting closed and/or have backports created for it incorrectly!
16-03-2015

No, hgupdater, bad boy. Sit! Roll! Bark! Stop auto-closing the issue. Re-opening.
13-03-2015

(fighting the hgupdater) Reopening again.
12-03-2015

This was mistakenly closed as part of the langtools push with same bug ID. Reopening.
06-03-2015

Sorry for the mishap - sent an email to Iris about this; my take is that if we reopen this and then close the right javac issue, the situation won't be any worse than with any typical bug that spans across multiple repos, where there could be two or more repos with a changeset mentioning same bug ID.
05-03-2015

Hey, I think Maurizio misreferenced this issue in his commit. He should have instead referenced JDK-8074306! Maurizio, please fix (if possible) and reopen.
05-03-2015

Writing self-consistent javadoc has unfortunately not been a strong point in the JDK. While the main text for Throwable doesn't mention the optionality of backtraces, nor the printStacktrace method, it is recognized in the spec for getStackTrace(): * <p>Some virtual machines may, under some circumstances, omit one * or more stack frames from the stack trace. In the extreme case, * a virtual machine that has no stack trace information concerning * this throwable is permitted to return a zero-length array from this * method. fillInStackTrace also hints at this: * <p>If the stack trace of this {@code Throwable} {@linkplain * Throwable#Throwable(String, Throwable, boolean, boolean) is not * writable}, calling this method has no effect. but never says under what circumstances a stacktrace would not be writable. And of course we utilize stackless throwables in other places eg for out-of-memory we try to create and throw an exception with a stacktrace but if we don't have the memory required to do that then we throw a pre-allocated one that has no stacktrace.
25-02-2015

There's no breach of the specification. It is written with care to sanction this optimization. The javadoc for Throwable seems to promise a lot, but realize that throwables can be created in contexts where the thread hasn't done anything yet. That can happen at JVM startup, or in native code. Also, there are well-known ways to explicitly suppress stack traces in throwable. I admit there is an unpleasant surprise; hence it could be an RFE or at most a QOS bug. JVMS 6.5 invokevirtual: "Otherwise, if objectref is null, the invokevirtual instruction throws a NullPointerException." JLS 15.12.4.4 Locate Method to Invoke: "If the target reference is null, a NullPointerException is thrown at this point." See also 15.6 Normal and Abrupt Completion of Evaluation and 11.1.2 The Causes of Exceptions. There is no guarantee, anywhere, that such an exception is newly created by the JVM. See also the non-normative comment in JLS Example 14.20.2-1. Handling An Uncaught Exception With finally "A backtrace is not required by this specification."
25-02-2015

I agree with pushing off from Object.getClass to saner NPE strategies, but as I said, the same thing applies to the "legal" getClass() calls that do actually use the class for e.g. reflecting on it. We will see if OmitStackTraceInFastThrow still has an impact on performance. Throwable javadoc says: "A throwable contains a snapshot of the execution stack of its thread at the time it was created." Notice there is no "may", or "could", or anything like that. It would seem the normative spec requires exceptions to have stack trace information. While users can produce stackless exceptions by overriding fillInStackTrace(), a compiler that observably strips the stack traces from the exception without user intervention (e.g. without -XX:-StackTraceInThrowable) is in the breach of the generic contract.
24-02-2015

Stackless exceptions are within spec, therefore this is an RFE, not a bug. Behavior changes after compilation are not bugs if they are within spec. Adding exception stack information has (or once had) a performance cost, which is why the compiler sometimes drops them for low-level errors. I agree that we have better null-check idioms available, and suggest a JDK cleanup to use Objects.requireNonNull instead of Object.getClass for null checks. If we do that, we can improve the quality of service for API-mandated null checks and keep the performance of Object.getClass. Independently of that, we can check if it is still the case that OmitStackTraceInFastThrow has an effect on code quality. If we believe there is no effect, we can flip the default, or perhaps even retire the flag. We can keep this bug open, but we need more information on the performance effect of OmitStackTraceInFastThrow before we take any action on the compiler's behavior.
21-02-2015

ILW=behavioral difference;c2,NPE from object.getClass;-XX:-OmitStackTraceInFastThrow=MMM=>P3
20-02-2015

First, it *is* the behavioral difference between interpreted and compiled code, so it is by definition the compiler bug. Compiler might change the performance behaviors, or reduce the number of observed functional behaviors, but it cannot introduce new functional behaviors. Second, while the prevailing use case seems to be null check, there are *legitimate* uses for obj.getClass() -- to get, well, object's class to act upon. Therefore, the issue is tangential to null-checks. I did use search before for "getClass NPE" and found nothing interesting. But re-searching now with "OmitStackTraceInFastThrow" yields https://bugs.openjdk.java.net/browse/JDK-4292742, which seems to argue this *is* the maitaintability issue for customers who cannot modify their VM invocation lines in production. So maybe the answer would be to flip OmitStackTraceInFastThrow to "false" by default.
19-02-2015

First, this is hardly a bug; more an Enhancement. We've got similar reports in the past (use search) and as far as I can remember we closed them because this is really an optimization. OmitStackTraceInFastThrow is a product flag and can be used to avoid this issue, as you state yourself. Maybe the actual issue to report here is the fact that we shouldn't be using obj.getClass() in core libraries to do null checks.
19-02-2015