[14] RFR(S) 8235438: [JVMCI] StackTraceElement::decode should use the original Method

[Vladimir Kozlov]

Thank you, Coleen

Vladimir

On 12/6/19 1:47 PM, coleen.phillimore at oracle.com wrote:
> 
> 
> On 12/6/19 4:42 PM, Vladimir Kozlov wrote:
>> Thank you, Coleen
>>
>> On 12/6/19 12:11 PM, coleen.phillimore at oracle.com wrote:
>>> This looks good.  I'm glad to see less places that have to call version_matches(), and this awkward handling for 
>>> redefinition.
>>>
>>> CDS should probably zero version after it loads the class out of the archive too, which I assume would have fixed 
>>> it.  But I like the new version of the code better than the old.
>>>
>>> Also, one small nit, which made reviewing this in frames a pain: Can you split these long lines?
>>>
>>> 2722 void java_lang_StackTraceElement::decode_file_and_line(Handle java_class, InstanceKlass* holder, int version,
>>> 2723 const methodHandle& method, int bci,
>>> 2724 Symbol*& source, oop& source_file, int& line_number, TRAPS) {
>>>
>>> 2746 void java_lang_StackTraceElement::decode(const methodHandle& method, int bci, Symbol*& methodname, Symbol*& 
>>> filename, int& line_number, TRAPS) {
>>
>> Done.
>>
>>>
>>> I don't see why the caller of java_lang_StackTraceElement can't get the methodname itself, and save this one output 
>>> parameter.
>>
>> Good suggestion.
>>
>> https://cr.openjdk.java.net/~kvn/8235438/webrev.01/
> 
> Yes, that makes is clear which functions want which output parameters.
> 
> Looks good - thanks,
> Coleen
>>
>> Thanks,
>> Vladimir
>>
>>>
>>> thanks,
>>> Coleen
>>>
>>>
>>> On 12/6/19 2:23 PM, Vladimir Kozlov wrote:
>>>> https://cr.openjdk.java.net/~kvn/8235438/webrev.00/
>>>> https://bugs.openjdk.java.net/browse/JDK-8235438
>>>>
>>>> This fix is prepared by Tom R.
>>>>
>>>> "The JDK14 version of StackTraceElement::decode is based on the JDK8 code which contains mixed usages of 
>>>> method->constants() and method->method_holder()->constants() assuming they point to the same thing. In the case of 
>>>> anonymous methods this isn't true. Usually this isn't a problem but if CDS is enabled the the version flag of 
>>>> method->method_holder()->constants() is non-zero but version of of method->constants() is 0 which causes the code to 
>>>> switch constants pools and it reads garbage. JDK-8140685 [1] refactored this code to remove this logic and the JVMCI 
>>>> version of this code should be converted to use the same scheme."
>>>>
>>>> I tested tier1-2 and tier3-4-graal. All clean.
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8140685
>>>
>