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

[Gerald Thornbrugh]

Hi David,
> 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)?
Yes, I tested this on an M7 machine and via JPRT (although it will not 
do anything until a M7 Solaris 11.3 machine
is added to the pool).

Thanks!

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