HSX-25: code review (round 0) for VMError::report_and_die () dies on bad PC (8015884)

[Daniel D. Daugherty]

Dmitry,

I changed my mind about changing the assert() into a guarantee() in
the changeset for 8015884. 8015884 is being backported to HSX-24
and I don't want introduce a new guarantee() there. I have filed
the following bug:

     JDK-8020090 os::address_is_in_vm() should guarantee() the base
                 address for libjvm
     https://jbs.oracle.com/bugs/browse/JDK-8020090

and I'll take care of pushing the fix for JDK-8020090 to HSX-25.

Dan


On 6/22/13 11:51 AM, Dmitry Samersoff wrote:
> Dan,
>
> os_linux.cpp:
>
> 1685:
>    It might be better to just turn assert to guarantee. Something bad is
> happening if we can't obtain JVM base address.
>
> 1795,1803:
> else is redundant
>
> -Dmitry
>
>
> On 2013-06-22 02:44, Daniel D. Daugherty wrote:
>> Greetings,
>>
>> I have have a proposed fix for the following bug:
>>
>>      8015884 runThese crashed with SIGSEGV, hs_err has an error instead
>>              of stacktrace
>>      http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8015884
>>      https://jbs.oracle.com/bugs/browse/JDK-8015884
>>
>> Here are the HSX-25 webrev URLs:
>>
>> OpenJDK: http://cr.openjdk.java.net/~dcubed/8015884-webrev/0-hsx25/
>> Internal: http://javaweb.us.oracle.com/~ddaugher/8015884-webrev/0-hsx25/
>>
>> Testing:
>> -  test/runtime/6888954/vmerrors.sh has been updated to include a
>>     test for a bad function pointer
>> - vmerrors.sh has been used on MacOS X and Solaris X86 to verify
>>    that bad function pointers get better trouble shooting info
>> - (running, @ 24 hours and not yet done) JPRT build and test job
>> - (TBD) JPRT build and new test only job
>> - (TBD) Aurora Adhoc vm.quick batch for all platforms in the following
>>    configs: {Client VM, Server VM} x {fastdebug} x {-Xmixed}
>>
>> Just to be clear: This fix solves the problem of not getting a
>> stacktrace when we get a bad PC; it does not solve the underlying
>> reason why libpthread sent the VM into the weeds.
>>
>> Gory details are below. As always, comments, questions and
>> suggestions are welome.
>>
>> Dan
>>
>>
>> Gory Details:
>>
>> The primary bug fixes are:
>>
>> - the Dl_info struct should only be used if dladdr() has
>>    returned non-zero (no errors) and always check the dladdr()
>>    return value
>> - the Dl_info.dli_sname and Dl_info.dli_saddr fields should
>>    only be used if non-NULL
>> - the first two fixes are based on the Solaris dladdr() man page
>> - make logic consistent between MacOS X, Linux and Solaris:
>>    - offset is only set when a matching symbol is found (and
>>      the pointer is non-NULL)
>>
>> Cleanup bug fixes are:
>>
>> - add assert()'s to verify os::dll_address_to_function_name() and
>>    os::dll_address_to_library_name() contract for 'buf' parameter
>>
>> - HotSpot style: explicitly check int function return values
>>    rather that assume conversion to boolean, e.g.
>>
>>      if (dladdr(...)) {  =>  if (dladdr(...) != 0) {
>>
>> - consistently compare Dl_info fields against NULL instead of a mix of
>>    '0', NULL and implied boolean
>>
>> - the Dl_info.dli_fname and Dl_info.dli_fbase fields are only used if
>>    non-NULL; this was done for consistency (and paranoia); the Solaris
>>    man page does not state whether or not NULL is a permitted value for
>>    these fields
>>
>> Test changes:
>> - add case "13" for bad function pointers
>> - change VM-side of the test code so that case "12", the
>>    os::signal_raise(SIGSEGV) case, does not have to be last;
>>    this puts a ShouldNotReachHere() call after the test_num
>>    case statement so this test function will always crash
>>    the VM in non-product bits
>> - change VM-side of the test code to issue an ERROR message
>>    when an unexpected test_num value is passed
>>
>