RFR(S): 8058345: Refactor native stack printing from vmError.cpp to debug.cpp to make it available in gdb as well

Tue Sep 16 19:21:58 UTC 2014

Hi Vladimir,

thanks for looking at the change.

'make_frame' is only intended to be used from within the debugger to
simplify the usage of the new 'pns()' (i.e. "print native stack")
helper. It can be used as follows:

(gdb) call pns(make_frame($sp, $rbp, $pc))

"Executing pns"
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
C  [libpthread.so.0+0xc0fe]  pthread_cond_timedwait+0x13e
V  [libjvm.so+0x96c4c1]  os::sleep(Thread*, long, bool)+0x1a1
V  [libjvm.so+0x75f442]  JVM_Sleep+0x312
j  java.lang.Thread.sleep(J)V+0
j  CrashNative.crashIt(Lsun/misc/Unsafe;I)V+10
j  CrashNative.doIt()V+45
v  ~StubRoutines::call_stub
V  [libjvm.so+0x71599f]
JavaCalls::call_helper(JavaValue*,methodHandle*, JavaCallArguments*,
Thread*)+0xf8f

What about the two fixesin in 'print_native_stack()' - do you think they are OK?
Should I move 'print_native_stack()' to vmError.cpp as suggested by David?

Thank you and best regards,
Volker

On Tue, Sep 16, 2014 at 8:11 PM, Vladimir Kozlov
<vladimir.kozlov at oracle.com> wrote:
> Thank you for fixing frame walk.
> I don't see where make_frame() is used.
>
> Thanks,
> Vladimir
>
>
> On 9/16/14 9:35 AM, Volker Simonis wrote:
>>
>> Hi,
>>
>> while testing my change, I found two other small problems with native
>> stack traces:
>>
>> 1. we can not walk native wrappers on (at least not on Linux/amd64)
>> because they are treated as native "C" frames. However, if the native
>> wrapper was called from a compiled frame which had no valid frame
>> pointer (i.e. %rbp) os::get_sender_for_C_frame(&fr) will produce a bad
>> frame. This can be easily fixed by treating native wrappers like java
>> frames.
>>
>> 2. the fix for "8035983: Fix "Native frames:" in crash report (hs_err
>> file)" introduced a similar problem. If we walk tha stack from a
>> native wrapper down to a compiled frame, we will have a frame with an
>> invalid frame pointer. In that case, the newly introduced check from
>> change 8035983 will fail, because fr.sender_sp() depends on a valid
>> fp. I'll propose to replace fr.sender_sp() by fr.real_fp() which
>> should do the same but also works for compiled frames with invalid fp.
>>
>> Here's the new webrev:
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/8058345.v1/
>>
>> What dou you think?
>>
>> Thank you and best regards,
>> Volker
>>
>>
>> On Tue, Sep 16, 2014 at 3:48 PM, Volker Simonis
>> <volker.simonis at gmail.com> wrote:
>>>
>>> 'print_native_stack()' must be visible in both vmError.cpp and
>>> debug.cpp. Initially I saw that vmError.cpp already included debug.hpp
>>> so I decided to declare it in debug.hpp. But now I realized that also
>>> debug.cpp includes vmError.hpp so I could just as well declare
>>> 'print_native_stack()' in vmError.hpp and leave the implementation in
>>> vmError.cpp. Do you want me to change that?
>>>
>>> Thank you and best regards,
>>> Volker
>>>
>>>
>>> On Tue, Sep 16, 2014 at 8:51 AM, David Holmes <david.holmes at oracle.com>
>>> wrote:
>>>>
>>>> Hi Volker,
>>>>
>>>> On 13/09/2014 5:15 AM, Volker Simonis wrote:
>>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>> could you please review and sponsor the following small change which
>>>>> should make debugging a little more comfortabel (at least on Linux for
>>>>> now):
>>>>>
>>>>> http://cr.openjdk.java.net/~simonis/webrevs/8058345/
>>>>> https://bugs.openjdk.java.net/browse/JDK-8058345
>>>>>
>>>>> In the hs_err files we have a nice mixed stack trace which contains
>>>>> both, Java and native frames.
>>>>> It would be nice if we could make this functionality available from
>>>>> within gdb during debugging sessions (until now we can only print the
>>>>> pure Java stack with the "ps()" helper function from debug.cpp).
>>>>>
>>>>> This new feature can be easily achieved by refactoring the
>>>>> corresponding stack printing code from VMError::report() in
>>>>> vmError.cpp into its own method in debug.cpp. This change extracts
>>>>> that code into the new function 'print_native_stack()' in debug.cpp
>>>>> without changing anything of the functionality.
>>>>
>>>>
>>>>
>>>> Why does it need to move to debug.cpp to allow this ?
>>>>
>>>> David
>>>> -----
>>>>
>>>>
>>>>> It also adds some helper functions which make it easy to call the new
>>>>> 'print_native_stack()' method from within gdb. There's the new helper
>>>>> function 'pns(frame f)'  which takes a frame argument and calls
>>>>> 'print_native_stack()'. We need the frame argument because gdb inserts
>>>>> a dummy frame for every call and we can't easily walk over this dummy
>>>>> frame from our stack printing routine.
>>>>>
>>>>> To simplify the creation of the frame object, I've added the helper
>>>>> functions:
>>>>>
>>>>> extern "C" frame make_frame(intptr_t* sp, intptr_t* fp, address pc) {
>>>>>     return frame(sp, fp, pc);
>>>>> }
>>>>>
>>>>> for x86 (in frame_x86.cpp) and
>>>>>
>>>>> extern "C" frame make_frame(intptr_t* sp, address pc) {
>>>>>     return frame(sp, pc);
>>>>> }
>>>>>
>>>>> for ppc64 in frame_ppc.cpp. With these helper functions we can now
>>>>> easily get a mixed stack trace of a Java thread in gdb (see below).
>>>>>
>>>>> All the helper functions are protected by '#ifndef PRODUCT'
>>>>>
>>>>> Thank you and best regards,
>>>>> Volker
>>>>>
>>>>>
>>>>> (gdb) call pns(make_frame($sp, $rbp, $pc))
>>>>>
>>>>> "Executing pns"
>>>>> Native frames: (J=compiled Java code, j=interpreted, Vv=VM code,
>>>>> C=native
>>>>> code)
>>>>> C  [libpthread.so.0+0xc0fe]  pthread_cond_timedwait+0x13e
>>>>> V  [libjvm.so+0x96c4c1]  os::sleep(Thread*, long, bool)+0x1a1
>>>>> V  [libjvm.so+0x75f442]  JVM_Sleep+0x312
>>>>> j  java.lang.Thread.sleep(J)V+0
>>>>> j  CrashNative.crashIt(Lsun/misc/Unsafe;I)V+10
>>>>> j  CrashNative.doIt()V+45
>>>>> v  ~StubRoutines::call_stub
>>>>> V  [libjvm.so+0x71599f]  JavaCalls::call_helper(JavaValue*,
>>>>> methodHandle*, JavaCallArguments*, Thread*)+0xf8f
>>>>> V  [libjvm.so+0x9eab75]  Reflection::invoke(instanceKlassHandle,
>>>>> methodHandle, Handle, bool, objArrayHandle, BasicType, objArrayHandle,
>>>>> bool, Thread*) [clone .constprop.218]+0xa25
>>>>> V  [libjvm.so+0x9eb838]  Reflection::invoke_method(oopDesc*, Handle,
>>>>> objArrayHandle, Thread*)+0x1c8
>>>>> V  [libjvm.so+0x7637ae]  JVM_InvokeMethod+0xfe
>>>>> j
>>>>>
>>>>> sun.reflect.NativeMethodAccessorImpl.invoke0(Ljava/lang/reflect/Method;Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;+0
>>>>> j
>>>>>
>>>>> sun.reflect.NativeMethodAccessorImpl.invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;+100
>>>>> j
>>>>>
>>>>> sun.reflect.DelegatingMethodAccessorImpl.invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;+6
>>>>> j
>>>>>
>>>>> java.lang.reflect.Method.invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;+56
>>>>> j  CrashNative.mainJava()V+32
>>>>> v  ~StubRoutines::call_stub
>>>>> V  [libjvm.so+0x71599f]  JavaCalls::call_helper(JavaValue*,
>>>>> methodHandle*, JavaCallArguments*, Thread*)+0xf8f
>>>>> V  [libjvm.so+0x7384f5]  jni_invoke_static(JNIEnv_*, JavaValue*,
>>>>> _jobject*, JNICallType, _jmethodID*, JNI_ArgumentPusher*, Thread*)
>>>>> [clone .isra.238] [clone .constprop.250]+0x385
>>>>> V  [libjvm.so+0x73b3d7]  jni_CallStaticVoidMethodV+0xe7
>>>>> C  [libCrashNative.so+0x9a9]  JNIEnv_::CallStaticVoidMethod(_jclass*,
>>>>> _jmethodID*, ...)+0xb9
>>>>> C  [libCrashNative.so+0xa10]  step3(JNIEnv_*, _jobject*)+0x65
>>>>> C  [libCrashNative.so+0xa69]  step2(JNIEnv_*, _jobject*)+0x57
>>>>> C  [libCrashNative.so+0xa37]  step2(JNIEnv_*, _jobject*)+0x25
>>>>> C  [libCrashNative.so+0xa37]  step2(JNIEnv_*, _jobject*)+0x25
>>>>> C  [libCrashNative.so+0xa37]  step2(JNIEnv_*, _jobject*)+0x25
>>>>> C  [libCrashNative.so+0xa37]  step2(JNIEnv_*, _jobject*)+0x25
>>>>> C  [libCrashNative.so+0xa37]  step2(JNIEnv_*, _jobject*)+0x25
>>>>> C  [libCrashNative.so+0xa8e]  step1(JNIEnv_*, _jobject*)+0x23
>>>>> C  [libCrashNative.so+0x87f]  Java_CrashNative_nativeMethod+0x23
>>>>> j  CrashNative.nativeMethod()V+0
>>>>> j  CrashNative.main([Ljava/lang/String;)V+9
>>>>> v  ~StubRoutines::call_stub
>>>>> V  [libjvm.so+0x71599f]  JavaCalls::call_helper(JavaValue*,
>>>>> methodHandle*, JavaCallArguments*, Thread*)+0xf8f
>>>>> V  [libjvm.so+0x7384f5]  jni_invoke_static(JNIEnv_*, JavaValue*,
>>>>> _jobject*, JNICallType, _jmethodID*, JNI_ArgumentPusher*, Thread*)
>>>>> [clone .isra.238] [clone .constprop.250]+0x385
>>>>> V  [libjvm.so+0x73b2b0]  jni_CallStaticVoidMethod+0x170
>>>>> C  [libjli.so+0x742a]  JavaMain+0x65a
>>>>> C  [libpthread.so.0+0x7e9a]  start_thread+0xda
>>>>>
>>>>
>