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

[Gerald Thornbrugh]

Hi David,
> 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?
It is used to include the library in the build process that involves 
"liboverflow.c".
Yes, it is automatically associated with 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?
This feature is only supported on the M7 SPARC architecture.
>
>
> Can you add a comment to the final #endif please, showing which #if it 
> matches.
Yes.
>
> 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).
I will clean up the alignment.
>
> ---
>
> 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.
I will change this.
>
> nativesegv does not need to be public.
I will change this.
>
> 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.
I will change this.
>
> ---
>
> 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.
I will change this.
>
>   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).
I will change this.
>
>
>  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.
>
I will change this.

Thanks!

Jerry
>
> Thanks,
> David
>
>
>> Thanks,
>>
>> Jerry
>>