RFR: 8141445: Use of Solaris/SPARC M7 libadimalloc.so can generate unknown signal in hs_err file

[David Holmes]

Hi Jerry,

On 5/11/2015 7:15 AM, Gerald Thornbrugh wrote:
> Hi Everyone,
>
> Please review my fix for JDK-8141445.
>
> JDK-8141445 describes an issue seen only on Solaris/SPARC M7 hardware running Solaris 11.3 or later when the libadimalloc.so library is
> preloaded before running the JVM and the JVM crashes due to a memory allocation fault triggered by libadimalloc.so.  Currently when
> libadimalloc.so triggers a memory allocation fault the SIGSEGV si_code is displayed as “unknown" in the hs_err file.  This change adds
> additional si_code defines so the memory allocation faults that trigger the JVM to core are displayed correctly in the hs_err file.
> The changes also include a test that preloads the libadimalloc.so library, generates a libadimalloc.so memory allocation fault that crashes
> the JVM and then verifies that the correct si_code information is displayed in the hs_err file.
>
> Webrev: http://cr.openjdk.java.net/~gthornbr/8141445/webrev.00/ <http://cr.openjdk.java.net/~gthornbr/8141445/webrev.00/>
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8141445 <https://bugs.openjdk.java.net/browse/JDK-8141445>
>
> Please let me know if you have any questions or concerns.

make/test/JtregNative.gmk

+ ifeq ($(TOOLCHAIN_TYPE), solstudio)
+   BUILD_HOTSPOT_JTREG_LIBRARIES_LDFLAGS_liboverflow := -lc
+ endif

This doesn't seem to be used anywhere? Is it automatically associated 
with the filename of the .c file?

---

src/os/posix/vm/os_posix.cpp

Why are these only defined for sparc? I would have expected this to be a 
solaris feature not a solaris-on-sparc feature?

Can you add a comment to the final #endif please, showing which #if it 
matches.

Lines 845 and 849 are slightly out of alignment compared to preceding 
lines and following lines (though in following lines CLD_CONTINUED 
messes up alignment anyway).

---

test/runtime/ErrorHandling/segv_overflow.java

Java style uses camel-case with initial capital for class names and 
therefor the file names. So this should be SegvOverflow.java or even 
SEGVOverflow.java (given SEGV is part of the signal name.

nativesegv does not need to be public.

print can be static then you don't need to create an instance. In fact 
given it is so short just put all the code in main.

---

test/runtime/ErrorHandling/Testlibadimalloc.java

  43       throws InterruptedException,IOException,Exception

"throws Throwable" is more succinct to avoid the checked-exceptions 
problem. Otherwise space after commas please.

   45       ProcessBuilder builder;
   46       Process process;
   47       long pid;
   48       int retval;
   49       boolean found = false;
   50       Path path;

Java does not require (nor does Java style encourage) all variables to 
be declared up front (nor does C/C++ any more :) ). Declare the 
variables on first use (or as close to if scoping is an issue).


  99       // If SEGV_ADIERR was not found in the hs_err file fail the test
  100       if (!found) {
  101         System.out.println("FAIL: forced failure");
  102         throw new RuntimeException("FAIL: forced failure");

You should report that SEGV_ADIERR was not found in the hs_err file.

Thanks,
David


> Thanks,
>
> Jerry
>