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

[Vladimir Kozlov]

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/

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
>