RFR: 8294053: Unneeded local variable in handle_safefetch()
Aleksey Shipilev
shade at openjdk.org
Thu Sep 22 14:49:24 UTC 2022
On Thu, 22 Sep 2022 12:38:51 GMT, Thomas Stuefe <stuefe 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.
> > `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.
-------------
PR: https://git.openjdk.org/jdk/pull/10373
More information about the hotspot-runtime-dev
mailing list