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