RFR: 8294053: Unneeded local variable in handle_safefetch()

[Robbin Ehn]

On Thu, 22 Sep 2022 14:45:42 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Kim marked this and other issues in https://bugs.openjdk.org/browse/JDK-8292166. If this RFE gets pushed, https://bugs.openjdk.org/browse/JDK-8292166 should get adapted.
>> 
>>> On Zero, `os::Posix::ucontext_get_pc(uc)` would fail with `ShouldNotReachHere()`, so this patch does not make it worse. 
>> 
>> Nothing prevents us from reading the PC from the context apart from aesthetics (CPU-specific sections inside os_linux_zero), right? 
>> 
>>> `JVM_HANDLE_XXX_SIGNAL` makes a special provision for Zero by always taking `pc = NULL` route. Zero signal handling fell victim to last years POSIX signal refactorings, and I still have it in my TODO to try and revive it. In fact, I am not even sure safefetch ever worked with Zero.
>> 
>> Safefetch worked, we made it so (see https://bugs.openjdk.org/browse/JDK-8076185). It is also covered by tests, or at least should be unless someone deactivated them.
>
>> 
>> > On Zero, `os::Posix::ucontext_get_pc(uc)` would fail with `ShouldNotReachHere()`, so this patch does not make it worse.
>> 
>> Nothing prevents us from reading the PC from the context apart from aesthetics (CPU-specific sections inside os_linux_zero), right?
> 
> Quite right, I actually started doing that today in https://bugs.openjdk.org/browse/JDK-8294211. My point is that removing the `os::Posix::ucontext_get_pc(uc);` in this PR does nothing bad for Zero AFAICS, because current (shadowd) `pc` is `nullptr`, and calling `ucontext_get_pc(uc)` would just meet `ShouldNotReachHere()`. After JDK-8294211 is done, the `pc` would be from `ucontext_get_pc(uc)` anyway.
> 
>> > `JVM_HANDLE_XXX_SIGNAL` makes a special provision for Zero by always taking `pc = NULL` route. Zero signal handling fell victim to last years POSIX signal refactorings, and I still have it in my TODO to try and revive it. In fact, I am not even sure safefetch ever worked with Zero.
>> 
>> Safefetch worked, we made it so (see https://bugs.openjdk.org/browse/JDK-8076185). It is also covered by tests, or at least should be unless someone deactivated them.
> 
> Yes, so I discovered when working on JDK-8294211. My patch seems to work with safefetch, at least I can gdb my way trough safefetch_sigjmp code at Linux x86_65 Zero.

@shipilev and @tstuefe, if you are fine with this going in now can you please review it, otherwise advice @fbredber to close, thanks.

-------------

PR: https://git.openjdk.org/jdk/pull/10373