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

[coleen.phillimore at oracle.com]

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