RFR: 8294053: Unneeded local variable in handle_safefetch()

[Thomas Stuefe]

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

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

Makes sense.

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

We have gtests for SafeFetch (https://github.com/openjdk/jdk/blob/master/test/hotspot/gtest/runtime/test_safefetch.cpp) and jtreg tests for SafeFetch during signal handling (eg. when writing hs-err files) https://github.com/openjdk/jdk/blob/master/test/hotspot/jtreg/runtime/ErrorHandling/SafeFetchInErrorHandlingTest.java . If those are green, all is well.

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

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