Why no hs-err file on CheckJNI?

[Thomas Stüfe]

On Wed, Aug 25, 2021 at 9:28 AM David Holmes <david.holmes at oracle.com>
wrote:

> On 25/08/2021 4:04 pm, Thomas Stüfe wrote:
> > Hi David,
> >
> > thank you for looking at this. Answers below.
> >
> > On Tue, Aug 24, 2021 at 9:38 AM David Holmes <david.holmes at oracle.com
> > <mailto:david.holmes at oracle.com>> wrote:
> >
> >     Hi Thomas,
> >
> >     On 24/08/2021 12:27 am, Thomas Stüfe wrote:
> >      > Hi,
> >      >
> >      > when we specify CheckJNI or CheckJNICalls and we catch an error
> >     (e.g. a
> >      > memory overwriter), we write a short report, then abort. See:
> >      >
> >      >
> >
> https://github.com/openjdk/jdk/blob/594e5161b48382d61509b4969bc8f52c3c076452/src/hotspot/share/prims/jniCheck.hpp#L36-L41
> >     <
> https://github.com/openjdk/jdk/blob/594e5161b48382d61509b4969bc8f52c3c076452/src/hotspot/share/prims/jniCheck.hpp#L36-L41
> >
> >      >
> >      > This has been introduced in 2008 with JDK-6739363 "Xcheck jni
> >     doesn't check
> >      > native function arguments". I could find no discussion about this
> on
> >      > mailing list archives.
> >
> >     There have been a number of updates to Xcheck:jni since then and in
> >     17 I
> >     documented the different kinds of checks and their behaviour in more
> >     detail (JDK-8260194):
> >
> >
> https://htmlpreview.github.io/?https://github.com/openjdk/jdk17/blob/4f336dd3985b654dc3fbacabdcfccf590ea918e5/java.html
> >     <
> https://htmlpreview.github.io/?https://github.com/openjdk/jdk17/blob/4f336dd3985b654dc3fbacabdcfccf590ea918e5/java.html
> >
> >
> >
> > Nice and interesting. Does not mention buffer overruns though.
>
> Do we detect buffer overruns? I looked at all the jniCheck functions to
> see what things we checked for and thought I had found them all. :(
>
> >      > Does anyone know why we don't write a normal hs-err file in this
> >     case?
> >
> >     Because the intent is to mimic throwing an exception and exiting and
> it
> >     is not a "hotspot error" it is an application error.
> >
> >      > Would anyone care if we did? We do so in similar cases, e.g. if
> >     os::free()
> >      > catches an overwrite.
> >
> >     os::free() is capturing an internal hotspot programming error, not an
> >     error in user code.
> >
> >
> > Is this mainly a support issue for you? Meaning, the existence of an
> > hs-err file would indicate a hotspot error and third-party JNI errors
> > erroneously assigned to the hotspot group's support queue? If so, I can
> > understand that, though that separation has a lot of holes in practice
> > (it's very easy to make the hotspot crash from third-party code).
> >
> > Technically, a hs-err file would be useful even if most of the hotspot
> > internals are irrelevant for a JNI bug. The file contains a lot of
> > valuable context.
>
> I just don't think a "hotspot error file" is a reasonable or necessary
> response to detecting a JNI error in application code. A stacktrace
> should suffice for the vast majority of errors detected.
>
> >     You would need to rework the header error messages etc and remove the
> >     bug reporting stuff so that the user doesn't think it is an error in
> >     the
> >     VM itself. Overall I don't see the need to do it as the main thing is
> >     the stacktrace to see where the bad JNI usage occurred - and as I
> said
> >     this isn't a VM error.
> >
> >     It might also introduce compatibility issues for anyone who runs
> >     testing
> >     wiith -Xcheck:jni and doesn't expect to get the hs_err file - though
> if
> >     you keep the current output but also produce a modified hs_err file
> >     that
> >     may be okay. But I still question why you would need this?
> >
> >
> > I am currently investigating a buffer overrun at a client caused in
> > ReleaseByteArrayElements. A hs-err file would have been definitely
> useful.
>
> I need more info on this case. If the overrun was detected when it
> happened then I would hope a stacktrace would suffice to show the errant
> code. And I'm not clear how a hs_err file would help. ??
>
>
A hs-err file would give you context information beyond the stack, e.g. VM
and program arguments, runtime, memory content at register/stack addresses
(may contain the broken block) etc.

A hs-err file is also a clear sign for a fatal error, while output to
stderr may be accidentally ignored. It depends on how well versed your
first level support and your customers are.

But I don't like pressing this. I understand your reasons for not writing
an hs-err file. We may change this downstream only, the patch would be
really trivial.

Thank you,

Thomas


> Cheers,
> David
> -----
>
> > Thank you!
> >
> > ..Thomas
> >
> >     Cheers,
> >     David
> >
> >      > Thanks, Thomas
> >      >
> >
>