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

[David Holmes]

On 5/11/2015 1:28 PM, Gerald Thornbrugh wrote:
> 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.

Okay - I should have known that :)

I assume this has been tested on a native build where the new constants 
are defined in the system headers (as they are not in our devkits and so 
won't be fully tested via JPRT builds)?

Thanks,
David

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