RFR: JDK-8257828: SafeFetch may crash if invoked in non-JavaThreads [v2]
Thomas Stuefe
stuefe at openjdk.java.net
Sat Dec 12 07:17:14 UTC 2020
> In our primary hotspot signal handlers, SafeFetch handling is limited to JavaThread objects:
>
> JavaThread* thread = NULL;
> ...
> if(t->is_Java_thread()) {
> thread = (JavaThread*)t;
> }
> ...
> if (info != NULL && uc != NULL && thread != NULL) {
> pc = (address) os::Linux::ucontext_get_pc(uc);
> if (StubRoutines::is_safefetch_fault(pc)) {
>
> As a result of this, using SafeFetch may crash non-JavaThreads if the location is invalid. E.g. using SafeFetch inside a VMOperation may crash the VM.
>
> This is unfortunate since SafeFetch is used for os::is_readable_pointer() which explicitly promises to not crash. It is used e.g. in os::print_hex_dump(). There is also no reason why SafeFetch would not work for non-JavaThreads. In fact, SafeFetch handling for the secondary signal handler works just fine for all threads.
>
> ----
>
> The patch makes handling of SafeFetch faults independent on whether the crashing thread is a JavaThread (indeed, whether we have a current Thread at all). This had been the case for AIX and Linux ppc, s390 before, since we already fixed this issue for our platform, so we know this works.
>
> I also hauled the SafeFetch handling out of the platform dependent part of the signal handler into the generic signal handler. This removes some duplicate coding.
>
> To be consistent, I moved the SafeFetch handling for Zero up into the generic signal handler too. Zero did not have a problem, but this reduces code.
>
> I added a gtest which reproduces the issue and used that to check that the patch works.
>
> Thanks, Thomas
Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
Make SafeFetch work on Windows + AIX fix
-------------
Changes:
- all: https://git.openjdk.java.net/jdk/pull/1695/files
- new: https://git.openjdk.java.net/jdk/pull/1695/files/a7a94321..eb0efc1e
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1695&range=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1695&range=00-01
Stats: 26 lines in 2 files changed: 7 ins; 9 del; 10 mod
Patch: https://git.openjdk.java.net/jdk/pull/1695.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/1695/head:pull/1695
PR: https://git.openjdk.java.net/jdk/pull/1695
More information about the hotspot-runtime-dev
mailing list