RFR(xs): 8147510: [windows] no text locations shown for register info in hs-err file

[Thomas Stüfe]

Hi Ioi,

Your suggestion makes sense, I changed the fix accordingly.

New webrev:
http://cr.openjdk.java.net/~stuefe/webrevs/8147510-windows-register-info-print-location/webrev.02/webrev/

Kind Regards, Thomas


On Fri, Jan 29, 2016 at 12:32 AM, Ioi Lam <ioi.lam at oracle.com> wrote:

> Hi Thomas,
>
> Thanks for taking my suggestion :-)
>
> To make the intention of the truncation check clearer, how about
> reorganizing the code to:
>
>   if (os::dll_address_to_library_name(addr, buf, sizeof(buf), &offset)) {
>     st->print(PTR_FORMAT " ", addr);
>     if (strlen(buf) < sizeof(buf)-1) {
>       char* p = strrchr(buf, '\\');
>       if (p) {
>         st->print("%s", p + 1);
>       } else {
>         st->print("%s", buf);
>       } else {
>         // The library name is probably truncated. Let's omit the library
> name.
>         // See also JDK-8147512.
>       }
>     }
>
> I'd say 'probably' because the name could be exactly 255 characters long
> ...
>
> Thanks
> - Ioi
>
>
>
> On 1/28/16 5:01 AM, Thomas Stüfe wrote:
>
> Hi Ioi,
>
> thanks a lot for the review!
>
> You are right, displaying the function name is more useful. I wanted to
> postpone this until I could start
> https://bugs.openjdk.java.net/browse/JDK-8147512, but now I took your
> suggestion and implemented this anyway.
>
> See here new webrev:
>
>
> http://cr.openjdk.java.net/~stuefe/webrevs/8147510-windows-register-info-print-location/webrev.01/webrev
>
> Notes:
> -I decided to cut off the path for libraries to make the printout easier
> to the eye; also, in the DLL-section we print out all DLLs including paths.
> -There is no clean way to handle truncation of long library paths, so I
> decided to at least detect truncation and if it happens to not print the
> library at all. I want to avoid large on-stack buffers, therefore I keep
> the result buffer relativly small (256 bytes). A better approach would be
> to - if os::find is used from error handling - hand down the error handling
> scratch buffer like we do in many other cases. But this is beyond the scope
> for this small fix.
>
> New printout looks like this:
>
>  35 Register to memory mapping:
>  36
>  37 RIP=0x000000006ddd6a43 jvm.dll::crash_with_segfault + 0x13
>  38 RAX=0xabc0000000000abc is an unknown value
>
> Kind Regards, Thomas
>
>
>
> On Thu, Jan 28, 2016 at 7:42 AM, Ioi Lam <ioi.lam at oracle.com> wrote:
>
>> Hi Thomas,
>>
>> 5269 bool os::find(address addr, outputStream* st) {
>> 5270   int offset = -1;
>> 5271   char buf[255];
>> 5272   if (os::dll_address_to_library_name(addr, buf, sizeof(buf),
>> &offset)) {
>> 5273     st->print_cr("%s + %x", buf, offset);
>> 5274     return true;
>> 5275   }
>> 5276   return false;
>> 5277 }
>>
>>
>> Have you tried calling os::dll_address_to_function_name as well so you
>> can get the function name?
>>
>> I noticed that the Linux version of os::find contains code for printing
>> out the function name, but couldn't find an hs_err file where that code is
>> actually used:
>>
>> bool os::find(address addr, outputStream* st) {
>>     ...
>>     if (dlinfo.dli_fname != NULL) {
>>       st->print(" in %s", dlinfo.dli_fname);
>>     }
>>
>>
>> Thanks
>> - Ioi
>>
>>
>> On 1/19/16 11:53 PM, Thomas Stüfe wrote:
>>
>>> May I please have a review and a sponsor?
>>>
>>> Thank you!
>>>
>>> On Sat, Jan 16, 2016 at 10:42 AM, Thomas Stüfe <thomas.stuefe at gmail.com>
>>> wrote:
>>>
>>> Hi,
>>>>
>>>> could I please have reviews and a sponsor for this small fix:
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8147510
>>>> Webrev:
>>>>
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8147510-windows-register-info-print-location/webrev.00/
>>>>
>>>> When we print register to memory info in hs-err file, text addresses are
>>>> not resolved on windows like on other platforms:
>>>>
>>>> "RCX=0x000000005efa0000 is an unknown value"
>>>>
>>>> This fix changes this (the same way it is implemented on other
>>>> platforms):
>>>>
>>>> "RCX=C:\d031900\openjdk\jdk9-hs-rt\output\images\jdk\bin\server\jvm.dll
>>>> +
>>>> 0"
>>>>
>>>> Please note that this printout and the code behind it could be improved
>>>> and cleaned up a lot. For instance, I know that using an on-stack
>>>> buffer in
>>>> os::find() is not the best implementation here. But I wanted a minimal
>>>> change. For the follow-up work, I opened a new bug:
>>>> https://bugs.openjdk.java.net/browse/JDK-8147512.
>>>>
>>>> Thanks & Regards, Thomas
>>>>
>>>>
>>>>
>>>>
>>>>
>>
>
>