Parse stubs/trampoline for more accurate stackwalking #1416
Conversation
In which situations do you foresee the stack will still exist after this PR? |
@fandreuz The issue is macOS specific & sadly due to the skipping of frames which is a separate issue as mentioned in the description is quite hard to write a clean test that wouldn't confuse most people on why certain checks are being performed |
src/symbols_macos.cpp
Outdated
| } | ||
| } | ||
|
|
||
| const section_64* findStubSection(const segment_command_64* sc) { |
There was a problem hiding this comment.
Better to make it a general purpose findSection function that accepts name as an argument.
This will be in line with ElfParser on Linux, and will allow reuse for other sections in the future.
There was a problem hiding this comment.
Handled here 86fcc22
src/symbols_macos.cpp
Outdated
| if ((isym[i] & (INDIRECT_SYMBOL_LOCAL | INDIRECT_SYMBOL_ABS)) == 0) { | ||
| const char* name = str_table + sym[isym[i]].n_un.n_strx; | ||
| if (name[0] == '_') name++; | ||
| _cc->add(base_addr + i * stubs_section->reserved2, 0, name); |
There was a problem hiding this comment.
Marking a trampoline with the target symbol's name can be confusing.
This is only a jump instruction to the target, not the actual implementation.
For example, there is a similar technique on Linux when an indirect call is made via PLT, and async-profiler adds synthetic symbols with @plt suffix to mimic gdb behavior.
Can you check what gdb/lldb print for trampolines on macOS?
There was a problem hiding this comment.
Also, these trampolines have known fixed size, we can pass this size instead of 0.
There was a problem hiding this comment.
in my debug sessions the lldb simply skips printing the trampolines completely
=> however if you look for the PC it will print the target symbol as the resolved function
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
* frame #0: 0x00000001002c755c libasyncProfiler.dylib`StackWalker::walkVM(ucontext=0x000000016fdadf48, frames=0x0000000128b40000, max_depth=2048, detail=VM_NORMAL, pc=0x000000010017865c, sp=6171583280, fp=6171583296) at stackWalker.cpp:387:25
frame #1: 0x00000001002beaa4 libasyncProfiler.dylib`StackWalker::walkVM(ucontext=0x000000016fdadf48, frames=0x0000000128b40000, max_depth=2048, detail=VM_NORMAL) at stackWalker.cpp:210:16
frame #2: 0x000000010029d450 libasyncProfiler.dylib`Profiler::recordSample(this=0x0000000118008000, ucontext=0x000000016fdadf48, counter=10000000, event_type=EXECUTION_SAMPLE, event=0x000000016fdade60) at profiler.cpp:655:27
frame #3: 0x00000001002cd980 libasyncProfiler.dylib`WallClock::signalHandler(signo=26, siginfo=0x000000016fdadee0, ucontext=0x000000016fdadf48) at wallClock.cpp:141:31
frame #4: 0x000000018b8ec624 libsystem_platform.dylib`_sigtramp + 56
frame #5: 0x0000000100178458 libjninativestacks.dylib`doCpuTask + 136
frame #6: 0x0000000100178568 libjninativestacks.dylib`Java_test_stackwalker_StackGenerator_largeFrame + 68
frame #7: 0x000000014753ca88
frame #8: 0x0000000147539130
frame #9: 0x0000000147534154
frame #10: 0x0000000101797abc libjvm.dylib`JavaCalls::call_helper(JavaValue*, methodHandle const&, JavaCallArguments*, JavaThread*) + 952
frame #11: 0x000000010180ccd8 libjvm.dylib`jni_invoke_static(JNIEnv_*, JavaValue*, _jobject*, JNICallType, _jmethodID*, JNI_ArgumentPusher*, JavaThread*) + 368
frame #12: 0x000000010180f36c libjvm.dylib`jni_CallStaticDoubleMethodV + 436
frame #13: 0x0000000100000bec main.o`JNIEnv_::CallStaticDoubleMethod(this=0x00000001248091b8, clazz=0x000000012460d360, methodID=0x0000600003829b70) at jni.h:1510:18
frame #14: 0x0000000100000a90 main.o`executeJvmTask() at main.cpp:110:32
frame #15: 0x0000000100000d40 main.o`main(argc=1, argv=0x000000016fdfef10) at main.cpp:132:5
frame #16: 0x000000018b512b98 dyld`start + 6076
(lldb) p pc
(const void *) 0x000000010017865c
(lldb) image lookup -a 0x000000010017865c
Address: libjninativestacks.dylib[0x000000000000065c] (libjninativestacks.dylib.__TEXT.__stubs + 16)
Summary: libjninativestacks.dylib`symbol stub for: pow + 4
(lldb) p depth
(volatile int) 0
I can flow the same standard as Linux of appending @plt, unless we want to skip these frames as lldb does,
There was a problem hiding this comment.
I've appended @plt at the end of the symbol here 86fcc22
Java_test_stackwalker_StackGenerator_largeFrame;doCpuTask 19
Java_test_stackwalker_StackGenerator_largeFrame;doCpuTask;powl 52
[unknown];main;startJvm;JNI_CreateJavaVM;Threads::create_vm;Management::initialize;NotificationThread::initialize;JavaThread::JavaThread;os::create_thread;Monitor::wait_without_safepoint_check;PlatformMonitor::wait;__psynch_cvwait 1
Java_test_stackwalker_StackGenerator_largeFrame;pow@plt 3
3 participants
Description
If async-profiler captures a sample inside a stub/Trampoline it will incorrectly resolve the PC to the wrong symbol/function as that data is missing from the CodeCache
This change fixes that aspect of the stack unwinding by adding the stubs/Trampolines to the CodeCache
Please note that the stack unwinding will still have issues of skipping frames
=> After the fix we're still seeing missing
doCpuTaskframe betweenpow&Java_test_stackwalker_StackGenerator_largeFrameHowever that would be a separate issue.
From my readings on the subject we will need to parse the
__unwind_info&eh_framesections to fix that aspect of the unwinding.Related issues
#1405
Motivation and context
Prevent obvious impossible frames from being generated on macOS
How has this been tested?
manual testing
=> Adding an assertion to check that
Java_test_stackwalker_StackGenerator_largeFrame;Java_test_stackwalker_StackGenerator_leafFramestack doesn't exist will create confusion for anyone finding it in the futureBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.