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

[coleen.phillimore at oracle.com]

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) {


I don't see why the caller of java_lang_StackTraceElement can't get the 
methodname itself, and save this one output parameter.

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