Why no hs-err file on CheckJNI?

[David Holmes]

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. ??

Cheers,
David
-----

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