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

[Ioi Lam]

Thanks Thomas, looks good!

- Ioi

On 1/29/16 12:53 AM, Thomas Stüfe wrote:
> 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/ 
> <http://cr.openjdk.java.net/%7Estuefe/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 
> <mailto: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
>>     <http://cr.openjdk.java.net/%7Estuefe/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
>>     <mailto: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 <mailto: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/
>>                 <http://cr.openjdk.java.net/%7Estuefe/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
>>
>>
>>
>>
>>
>>
>
>