RFR(M): 8221077: No NullPointerException message if top frame is hidden.

Tue May 7 09:46:35 UTC 2019

Hi Remi,

> A Throwable not only print an exception but also reify the stack elements (if
> one calls getStackElements()) and applications may use those stack elements
> to take decision, so the only way i see to be selective is to have printStackTrace
> and getStackElements() to disagree on the content of the stacktrace. I don't
> think it's a good idea.
Given my implementation, everybody who get's the NPE into his
hands gets the same stack trace. It can only be inconsistent if 
a method with a hidden frame uses a StackWalker and compares
those frames to the frames of an NPE thrown in the same 
method.  But you can ask the StackWalker to show the hidden frames.

> Those adapter frames usually don't throw NPEs, by example for a method
> reference bound to an instance that may be null, the nullcheck is done upfront
> when creating the lambda and not when calling it to ensure that no NPE can be
> raised inside the adapter frame.
Well, in this case they do. Please look at the included test. So does the 
String concatenation need to be improved?

Best regards,
  Goetz.

> -----Original Message-----
> From: forax at univ-mlv.fr <forax at univ-mlv.fr>
> Sent: Dienstag, 7. Mai 2019 11:31
> To: David Holmes <david.holmes at oracle.com>
> Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-runtime-dev
> <hotspot-runtime-dev at openjdk.java.net>; Coleen Phillimore,
> coleen.phillimore at oracle.com <coleen.phillimore at oracle.com>
> Subject: Re: RFR(M): 8221077: No NullPointerException message if top frame is
> hidden.
> 
> ----- Mail original -----
> > De: "David Holmes" <david.holmes at oracle.com>
> > À: "Remi Forax" <forax at univ-mlv.fr>, "Goetz Lindenmaier"
> <goetz.lindenmaier at sap.com>
> > Cc: "hotspot-runtime-dev" <hotspot-runtime-dev at openjdk.java.net>,
> "Coleen Phillimore, coleen.phillimore at oracle.com"
> > <coleen.phillimore at oracle.com>
> > Envoyé: Mardi 7 Mai 2019 09:52:06
> > Objet: Re: RFR(M): 8221077: No NullPointerException message if top frame is
> hidden.
> 
> > Hi Remi,
> 
> Hi David,
> 
> >
> > On 7/05/2019 12:39 am, Remi Forax wrote:
> >> Hi Goetz,
> >> please don't !
> >>
> >> You have existing codes that behave differently depending on the caller
> class,
> >> part of the security model is based on that, but also other APIs like by
> >> example, many of the existing logging libraries.
> >>
> >> When a stack frame is hidden, it means that the stack frame is not visible
> for
> >> getCallerClass, the StackWalker API and the Throwable stacktrace,
> >> the VM/JDK offers one coherent view of the stack frames.
> >
> > I would argue that "throwable stacktrace" has different considerations
> > if the exception is actually thrown due to an error in the code in the
> > "hidden frame". Otherwise you will get messages about impossible
> exceptions.
> 
> It's exactly the current behavior, you can have exceptions with the "wrong"
> stacktrace.
> 
> The "hidden frame" feature is not named correctly, those frames are adapter
> frames, like the other adapter frames produced by the VM but those one are
> written in Java.
> Currently the method handle implementation, lambda proxies or var handles
> are the main source of these adapters, but in the future we will have more as a
> consequence of metropolis.
> Those adapter frames usually don't throw NPEs, by example for a method
> reference bound to an instance that may be null, the nullcheck is done upfront
> when creating the lambda and not when calling it to ensure that no NPE can be
> raised inside the adapter frame.
> 
> >
> >> If you suppress the hiding for one of these entry points, at best, applications
> >> will fail, at worst you have introduced a security issue.
> >
> > The suppression would need to be selective - unless you're claiming that
> > anything that shows these hidden frames exist could be considered a
> > security issue ?
> 
> A Throwable not only print an exception but also reify the stack elements (if
> one calls getStackElements()) and applications may use those stack elements
> to take decision, so the only way i see to be selective is to have printStackTrace
> and getStackElements() to disagree on the content of the stacktrace. I don't
> think it's a good idea.
> 
> >
> > Cheers,
> > David
> > -----
> 
> Rémi
> 
> >
> >> regards,
> >> Rémi
> >>
> >> ----- Mail original -----
> >>> De: "Goetz Lindenmaier" <goetz.lindenmaier at sap.com>
> >>> À: "David Holmes" <david.holmes at oracle.com>, "hotspot-runtime-dev"
> >>> <hotspot-runtime-dev at openjdk.java.net>, "Coleen
> >>> Phillimore (coleen.phillimore at oracle.com)"
> <coleen.phillimore at oracle.com>
> >>> Envoyé: Lundi 6 Mai 2019 14:36:36
> >>> Objet: RE: RFR(M): 8221077: No NullPointerException message if top
> frame is
> >>> hidden.
> >>
> >>> Hi David,
> >>>
> >>> I'm not really happy with the design of hiding the frames,
> >>> either.
> >>> For 'inner' frames this seems ok, but for the top frames
> >>> up to the first not-hidden frame it is questionable to
> >>> hide them.  As you say, there might be exceptions where
> >>> this is desirable, for NPE it seems wrong.
> >>>
> >>> I’ll have a look how it could be implemented to
> >>> suppress the hiding in case of NPE.
> >>>
> >>> Best regards,
> >>>   Goetz.
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: David Holmes <david.holmes at oracle.com>
> >>>> Sent: Montag, 6. Mai 2019 13:50
> >>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-runtime-
> >>>> dev at openjdk.java.net; Coleen Phillimore (coleen.phillimore at oracle.com)
> >>>> <coleen.phillimore at oracle.com>
> >>>> Subject: Re: RFR(M): 8221077: No NullPointerException message if top
> frame is
> >>>> hidden.
> >>>>
> >>>> Hi Goetz,
> >>>>
> >>>> Just commenting on the situation ...
> >>>>
> >>>> On 6/05/2019 8:22 pm, Lindenmaier, Goetz wrote:
> >>>>> Hi,
> >>>>>
> >>>>> Exceptions can print the stack trace of when the exception occurred.
> >>>>> There can be methods on the stack that have been generated by
> >>>>> the runtime. To not confuse the reader of the stackTrace,
> >>>>> these are omitted (if -XX:-ShowHiddenFrames, which is default.)
> >>>>>
> >>>>> If the top frame is omitted/hidden,
> >>>>> it is inconsistent to print a message telling where
> >>>>> exactly in the hidden method the error occurred.
> >>>>>
> >>>>> Further, if the top frame is hidden, the information
> >>>>> needed to compute the message is missing. A wrong
> >>>>> message will be printed.
> >>>>>
> >>>>> This change suppresses the NPE message if the NPE
> >>>>> occurred in a hidden top frame.
> >>>>
> >>>> This sounds to me like "hidden frames" are being applied too broadly.
> >>>> Frames should only be hidden with respect to throwing specific
> >>>> exceptions - those for which the frame is  part of the "throwing
> >>>> mechanics". If that code hits a real exception then the frame should not
> >>>> be treated as hidden IMO. But I don't know whether we can clearly
> >>>> identify which kinds of exceptions should really lead to a frame being
> >>>> hidden.
> >>>>
> >>>> David
> >>>> -----
> >>>>
> >>>>> It adds a java.lang.Boolean(true) to the backtrace
> >>>>> in case the real top frame is a hidden one and is dropped.
> >>>>>
> >>>>> When the NullPointerException message text is generated, this is
> >>>>> checked and the message is skipped if the proper frame is not
> >>>>> available.
> >>>>>
> >>>>> I handle this in a bug itself to keep the discussion of
> >>>>> the basic feature separated from this technical issue.
> >>>>>
> >>>>> Please review:
> >>>>> https://bugs.openjdk.java.net/browse/JDK-8221077
> >>>>> http://cr.openjdk.java.net/~goetz/wr19/8221077-
> hidden_top_frame/02/
> >>>>> This change must be applied on top of
> >>>>> http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/08/
> >>>>>
> >>>>> Best regards,
> > >>>>     Goetz.