[jdk17] RFR: 8268717: Upstream: 8268673: Stack walk across optimized entry frame on fresh native thread fails [v2]

David Holmes david.holmes at oracle.com
Thu Jun 17 12:29:09 UTC 2021


Hi Jorn,

On 17/06/2021 9:28 pm, Jorn Vernee wrote:
> On Thu, 17 Jun 2021 00:23:19 GMT, David Holmes <dholmes at openjdk.org> wrote:
> 
>>> Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision:
>>>
>>>    Add comment about optimized entry frames only being generated on x86_64
>>
>> src/hotspot/share/runtime/frame.inline.hpp line 54:
>>
>>> 52: inline bool frame::is_first_frame() const {
>>> 53:   return (is_entry_frame() && entry_frame_is_first())
>>> 54:       || (is_optimized_entry_frame() && optimized_entry_frame_is_first());
>>
>> Given `optimized_entry_frame_is_first` is only defined on a couple of platforms, it is far from obvious that this call can never happen on the other platforms. A comment explaining this would be useful.
> 
> Thanks, I've added the following comment:
> 
> ```C++
> inline bool frame::is_first_frame() const {
>    return (is_entry_frame() && entry_frame_is_first())
>        // optimized_entry_frame_is_first is currently only implemented on x86_64.
>        // This is okay since optimized entry frames are only generated on x86_64
>        // as well (see ProgrammableUpcallHandler::generate_optimized_upcall_stub
>        // in universalUpcallHandler_x86_64.cpp), so is_optimized_entry_frame will
>        // always return false on platforms where optimized_entry_frame_is_first
>        // is not implemented.
>        || (is_optimized_entry_frame() && optimized_entry_frame_is_first());
> }

Now that you have explained it I think a much simpler comment will 
suffice :)

     return (is_entry_frame() && entry_frame_is_first()) ||
        // Optimized entry frames are only present on certain platforms
        (is_optimized_entry_frame() && optimized_entry_frame_is_first());

Cheers,
David


> -------------
> 
> PR: https://git.openjdk.java.net/jdk17/pull/76
> 



More information about the build-dev mailing list