RFR (XS) 8144853: Print the names of callees in PrintAssembly/PrintInterprete

[Ioi Lam]

Hi David,

I have updated the patch according to your feedback:

http://cr.openjdk.java.net/~iklam/jdk9/8144853-print-interpreter-callee-names.v3/

The buf[] variable is now allocated using NEW_RESOURCE_ARRAY, and the 
useless 'found' variable has been removed. The comment is also fixed.

Thanks
- Ioi

On 12/8/15 8:47 PM, David Holmes wrote:
> Hi Ioi,
>
> src/share/vm/code/nmethod.cpp
>
> Your use of the static buffer seems not thread-safe.
>
> Minor nit:
>
> 3057             bool found;
> 3058             found = os::dll_address_to_function_name(dest, buf, 
> sizeof(buf), &offset);
>
> could be:
>
> 3057             bool found = os::dll_address_to_function_name(dest, 
> buf, sizeof(buf), &offset);
>
>
> ---
>
> src/share/vm/compiler/disassembler.cpp
>
> Your use of the static buffer seems not thread-safe.
>
> 366   bool found;
>
> This variable seems unused.
>
> Typo:
>
> 369     // Don't do this for native methods, as the function name will 
> be printed in
> 370     // by nmethod::reloc_string_for().
>
> delete 'in' or 'by'.
>
> Thanks,
> David
> -----
>
>
> On 8/12/2015 5:32 AM, Ioi Lam wrote:
>> Hi Vladimir,
>>
>> Thanks for the suggestion. I've changed the patch to handle the nmethods
>> differently than PrintInterpreter:
>>
>> http://cr.openjdk.java.net/~iklam/jdk9/8144853-print-interpreter-callee-names.v2/ 
>>
>>
>>
>> The nmethod dump now looks like this:
>>
>>    0x00007f607940c657: callq    0x00007f607186bb60  ;
>> ImmutableOopMap{[0]=Oop }
>>                                                  ;*ifeq {reexecute=1
>> rethrow=0 return_oop=0}
>>                                                  ; -
>> java.lang.String::charAt at 4 (line 685)
>>                                                  ; {runtime_call
>> UncommonTrapBlob}
>>    0x00007f607940c65c: callq    0x00007f608b05eb00  ;*ifeq {reexecute=0
>> rethrow=0 return_oop=0}
>>                                                  ; -
>> java.lang.String::charAt at 4 (line 685)
>>                                                  ; {runtime_call
>> os::breakpoint()}
>>
>>
>> Thanks
>> - Ioi
>>
>>
>> On 12/7/15 10:58 AM, Vladimir Ivanov wrote:
>>> Ioi,
>>>
>>> Nice improvement!
>>>
>>> Why don't you print "{runtime_call os::breakpoint()}" for
>>> -XX:+PrintAssemble instead? It looks more natural w.r.t. current 
>>> format.
>>>
>>> Best regards,
>>> Vladimir Ivanov
>>>
>>> On 12/7/15 9:18 PM, Ioi Lam wrote:
>>>> Please review a very small fix:
>>>>
>>>> http://cr.openjdk.java.net/~iklam/jdk9/8144853-print-interpreter-callee-names/ 
>>>>
>>>>
>>>>
>>>>
>>>> Bug: Print the names of callees in PrintAssembly/PrintInterprete
>>>>
>>>>      https://bugs.openjdk.java.net/browse/JDK-8144853
>>>>
>>>> Summary of fix:
>>>>
>>>>      In -XX:+PrintAssembly and -XX:+PrintInterpreter, sometimes only
>>>>      the address of a callee is printed, and the name is missing.
>>>>
>>>>      The fix is to use os::dll_address_to_function_name() to find the
>>>>      names of such functions and print them out if possible.
>>>>
>>>>      EXAMPLES:
>>>>      -XX:+PrintInterpreter:
>>>>      0x00007f1b018447c3: callq 0x00007f1b19b9bba0 =
>>>> InterpreterRuntime::newarray(JavaThread*, BasicType, int)
>>>>
>>>>      -XX:+PrintAssembly
>>>>      <....>
>>>>      0x00007f75d87b9629: xchg %ax,%ax
>>>>      0x00007f75d87b962b: callq 0x00007f75d0c11b60 ;
>>>> ImmutableOopMap{rbp=Oop }
>>>>                                                    ;*iflt {reexecute=1
>>>> rethrow=0 return_oop=0}
>>>>                                                    ; -
>>>> java.lang.StringLatin1::charAt at 1 (line 43)
>>>>                                                    ; {runtime_call
>>>> UncommonTrapBlob}
>>>>      0x00007f75d87b9630: callq 0x00007f75e8e41370 = os::breakpoint()
>>>>                                                    ;*iflt {reexecute=0
>>>> rethrow=0 return_oop=0}
>>>>                                                    ; -
>>>> java.lang.StringLatin1::charAt at 1 (line 43)
>>>>                                                    ; {runtime_call}
>>>>
>>>>
>>>> TESTS:
>>>>      RBT hotspot/test/:hotspot_all (this includes tests cases with
>>>> -XX:+PrintAssembly)
>>>>
>>>> Thanks
>>>> - Ioi
>>