RFR (XS) 8216977: ShowHiddenFrames use in java_lang_StackTraceElement::fill_in appears broken
David Holmes
david.holmes at oracle.com
Thu Aug 29 05:33:50 UTC 2019
Hi Coleen,
On 29/08/2019 1:19 pm, coleen.phillimore at oracle.com wrote:
> On 8/28/19 9:36 PM, David Holmes wrote:
>> Hi Coleen,
>>
>> On 29/08/2019 6:25 am, coleen.phillimore at oracle.com wrote:
>>> Summary: Return NULL source file and negative line number for hidden
>>> frames.
>>>
>>> See bug for more details. Ran hs tier1-3 on linux-x64-debug and
>>> tier1 on all platforms.
>>
>> This issue made my head spin. I think what you have done has taken
>> things back to a sane place where ShowHiddenFrames is not being used
>> to do whacky things unrelated to hidden frames.
>>
>>> open webrev at
>>> http://cr.openjdk.java.net/~coleenp/2019/8216977.01/webrev
>>> bug link https://bugs.openjdk.java.net/browse/JDK-8216977
>>
>> src/hotspot/share/classfile/javaClasses.inline.hpp
>>
>> Okay - this code was reporting a specially encoded "line number" that
>> actually contained the BCI, but only if ShowHiddenFrames was true.
>> Getting rid of this is good because it has nothing to do with hidden
>> frames and there wasn't even a check that the current frame was in
>> fact hidden.
>>
>
> Yes, it could have checked if method->is_hidden() give the special line
> number with bci encoded. Maybe tools can print something useful with
> that information. StackTraceElement.toString() doesn't though.
>
>> If we want to report the BCI when no line number table exists then
>> that should be a specific enhancement to the API.
>
> I agree.
>>
>> ---
>>
>> src/hotspot/share/classfile/javaClasses.cpp
>>
>> Okay - this code was changing a NULL source file to "unknown" but only
>> if ShowHiddenFrames is true. Again getting rid of this is good as it
>> again has nothing to do with hidden frames.
>>
>> If we want to report "unknown" rather than NULL, then we should make
>> that change independent of ShowHiddenFrames.
>>
>
> Calling it <Unknown> makes it inconsistent with the case where the
> source file is unknown, which java.lang.StackFrame prints.
>> ---
>>
>> test/hotspot/jtreg/runtime/StackTrace/HiddenFrameTest.java
>>
>> Can you add a comment explaining why String::concat meets the criteria
>> of "null source file" and "no line number table", as it is not at all
>> obvious to me what that should be the case. Also is that true if we
>> disable the String concatenation use of lambda? If not we should add
>> the necessary -XX:+ flag to the @run line.
>
> I don't know what that flag is. It might be a property. I don't think
> we'd run all the tests with such a flag though so I think this is safe.
Sorry was confusing different things. You're just using a method
reference to String::concat which will use lambda expressions under the
hood. I was thinking about the lambdification of string concatenation
which is completely different.
>>
>> For completeness can you check that the NPE only has one frame if
>> -XX:-ShowHiddenFrames is used.
>
> Sure:
>
> http://cr.openjdk.java.net/~coleenp/2019/8216977.02/webrev/test/hotspot/jtreg/runtime/StackTrace/HiddenFrameTest.java.html
Thanks! One nit:
String arg = (String)args[0];
No need for a cast there - args is an array of String.
No need for new webrev.
Thanks,
David
>
> Thanks,
> Coleen
>>
>> Thanks,
>> David
>> -----
>>
>>> Thanks,
>>> Coleen
>
More information about the hotspot-runtime-dev
mailing list