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

Tue May 7 13:49:32 UTC 2019

----- Mail original -----
> De: "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>
> Envoyé: Mardi 7 Mai 2019 15:44:13
> Objet: Re: RFR(M): 8221077: No NullPointerException message if top frame is hidden.

> ----- Mail original -----
>> De: "Goetz Lindenmaier" <goetz.lindenmaier at sap.com>
>> À: "Remi Forax" <forax at univ-mlv.fr>, "David Holmes" <david.holmes at oracle.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 11:46:35
>> Objet: RE: RFR(M): 8221077: No NullPointerException message if top frame is
>> hidden.
> 
>> 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 ?
> 
> I was talking about the case of a method ref bound to an instance:
>  String s = null;
>  Function<String, String> fun = s::concat   // get a NPE upfront
> 
> not the case of an unbounded method ref:
>  BiFunction<String, String, String> fun = String::concat;
>  fun.apply(null, ...) // get a NPE in the adapter
> 
> but, yes, i wonder if the java compiler should not insert a requireNonNull with
> a nice message in this case.

oops, i mean the JDK, the adapter is generated by the JDK, not by javac.

> 
>> 
>> Best regards,
>>  Goetz.
>> 
>> 

Rémi

> 
> Rémi
> 
>> 
>>> -----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.