[11u] RFR: 8257828 SafeFetch may crash if invoked in non-JavaThreads

[Thomas Stüfe]

Thanks, Sergey!

..Thomas

On Wed, May 19, 2021 at 8:41 PM Sergey Nazarkin <snazarkin at azul.com> wrote:

> Checked on arm32: new test fails without patch and passes with the patch
>
>
>
>
>
> > On May 11, 2021, at 09:07, Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
> >
> > Hi,
> >
> > I'd like reviews for this downport, please.
> >
> > Issue: https://bugs.openjdk.java.net/browse/JDK-8257828
> > Original patch: https://git.openjdk.java.net/jdk/commit/3ab1dfeb
> > Original Review: https://git.openjdk.java.net/jdk/pull/1695
> >
> > This patch does two things:
> >
> > 1) On Posix platforms it fixes SafeFetch to work in all threads, not only
> > in JavaThreads. In non-JavaThreads before this patch it would crash.
> >
> > The patch is totally trivial. In the signal handler, when handling
> > SafeFetch, there was a condition that Thread != NULL and be a JavaThread.
> > This condition was unnecessary. All SafeFetch handling needs is a valid
> > context. The Patch moves SafeFetch handling out of the is-JavaThread
> > condition.
> >
> > 2) On Windows we did not have the problem; however there is the
> > `Handle_Exception` function handling SafeFetch among other things, which
> > may be called with a non-java thread, but then forcibly casts the Thread*
> > to JavaThread.
> >
> > 11u patch:
> >
> http://cr.openjdk.java.net/~stuefe/webrevs/backports/8257828-safefetch-non-java-threads
> >
> > The original patch does not apply, at all. The reason for this is that we
> > consolidated common functionality of the many cpu+arch signal handlers to
> > one function, in os_posix.cpp. This happened in JDK16 and I do not want
> to
> > backport this cleanup change to 11. For this patch, it means that the
> > posix-related fixes practically had to be rewritten for each and every
> > signal handler. Each fix is primitive however, since it just moves
> > SafeFetch handling around, but every signal handler is a tiny bit
> different.
> >
> > I tested this patch for several weeks in our systems on Linux
> > ppc/ppcle/x64/s390/aarch64, AIX, Solaris and Mac, no problems surfaced.
> >
> > I did not test on 32bit arm.
> >
> > Thanks, Thomas
>
>