friss created this revision. friss added reviewers: jasonmolenda, JDevlieghere. friss requested review of this revision. Herald added a project: LLDB.
A couple of our Instrumentation runtimes were gathering backtraces, storing it in a StructuredData array and later creating a HistoryThread using this data. By deafult HistoryThread will consider the history PCs as return addresses and thus will substract 1 from them to go to the call address. This is usually correct, but it's also wasteful as when we gather the backtraces ourselves, we have much better information to decide how to backtrace and symbolicate. This patch uses the new GetFrameCodeAddressForSymbolication() to gather the PCs that should be used for symbolication and configures the HistoryThread to just use those PCs as-is. (The MTC plugin was actaully applying a -1 itself and then the HistoryThread would do it again, so this actaully fixes a bug there.) rdar://77027680 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D101094 Files: lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp Index: lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp =================================================================== --- lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp +++ lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp @@ -150,8 +150,8 @@ StructuredData::Array *trace = new StructuredData::Array(); auto trace_sp = StructuredData::ObjectSP(trace); for (unsigned I = 0; I < thread_sp->GetStackFrameCount(); ++I) { - const Address FCA = - thread_sp->GetStackFrameAtIndex(I)->GetFrameCodeAddress(); + const Address FCA = thread_sp->GetStackFrameAtIndex(I) + ->GetFrameCodeAddressForSymbolication(); if (FCA.GetModule() == runtime_module_sp) // Skip PCs from the runtime. continue; @@ -324,7 +324,11 @@ info->GetObjectForDotSeparatedPath("tid"); tid_t tid = thread_id_obj ? thread_id_obj->GetIntegerValue() : 0; - HistoryThread *history_thread = new HistoryThread(*process_sp, tid, PCs); + // We gather symbolication addresses above, so no need for HistoryThread to + // try to infer the call addresses. + bool pcs_are_call_addresses = true; + HistoryThread *history_thread = + new HistoryThread(*process_sp, tid, PCs, pcs_are_call_addresses); ThreadSP new_thread_sp(history_thread); std::string stop_reason_description = GetStopReasonDescription(info); new_thread_sp->SetName(stop_reason_description.c_str()); Index: lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp =================================================================== --- lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp +++ lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp @@ -127,7 +127,7 @@ StackFrameSP responsible_frame; for (unsigned I = 0; I < thread_sp->GetStackFrameCount(); ++I) { StackFrameSP frame = thread_sp->GetStackFrameAtIndex(I); - Address addr = frame->GetFrameCodeAddress(); + Address addr = frame->GetFrameCodeAddressForSymbolication(); if (addr.GetModule() == runtime_module_sp) // Skip PCs from the runtime. continue; @@ -135,11 +135,6 @@ if (!responsible_frame) responsible_frame = frame; - // First frame in stacktrace should point to a real PC, not return address. - if (I != 0 && trace->GetSize() == 0) { - addr.Slide(-1); - } - lldb::addr_t PC = addr.GetLoadAddress(&target); trace->AddItem(StructuredData::ObjectSP(new StructuredData::Integer(PC))); } @@ -271,7 +266,11 @@ info->GetObjectForDotSeparatedPath("tid"); tid_t tid = thread_id_obj ? thread_id_obj->GetIntegerValue() : 0; - HistoryThread *history_thread = new HistoryThread(*process_sp, tid, PCs); + // We gather symbolication addresses above, so no need for HistoryThread to + // try to infer the call addresses. + bool pcs_are_call_addresses = true; + HistoryThread *history_thread = + new HistoryThread(*process_sp, tid, PCs, pcs_are_call_addresses); ThreadSP new_thread_sp(history_thread); // Save this in the Process' ExtendedThreadList so a strong pointer retains
Index: lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp =================================================================== --- lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp +++ lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp @@ -150,8 +150,8 @@ StructuredData::Array *trace = new StructuredData::Array(); auto trace_sp = StructuredData::ObjectSP(trace); for (unsigned I = 0; I < thread_sp->GetStackFrameCount(); ++I) { - const Address FCA = - thread_sp->GetStackFrameAtIndex(I)->GetFrameCodeAddress(); + const Address FCA = thread_sp->GetStackFrameAtIndex(I) + ->GetFrameCodeAddressForSymbolication(); if (FCA.GetModule() == runtime_module_sp) // Skip PCs from the runtime. continue; @@ -324,7 +324,11 @@ info->GetObjectForDotSeparatedPath("tid"); tid_t tid = thread_id_obj ? thread_id_obj->GetIntegerValue() : 0; - HistoryThread *history_thread = new HistoryThread(*process_sp, tid, PCs); + // We gather symbolication addresses above, so no need for HistoryThread to + // try to infer the call addresses. + bool pcs_are_call_addresses = true; + HistoryThread *history_thread = + new HistoryThread(*process_sp, tid, PCs, pcs_are_call_addresses); ThreadSP new_thread_sp(history_thread); std::string stop_reason_description = GetStopReasonDescription(info); new_thread_sp->SetName(stop_reason_description.c_str()); Index: lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp =================================================================== --- lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp +++ lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp @@ -127,7 +127,7 @@ StackFrameSP responsible_frame; for (unsigned I = 0; I < thread_sp->GetStackFrameCount(); ++I) { StackFrameSP frame = thread_sp->GetStackFrameAtIndex(I); - Address addr = frame->GetFrameCodeAddress(); + Address addr = frame->GetFrameCodeAddressForSymbolication(); if (addr.GetModule() == runtime_module_sp) // Skip PCs from the runtime. continue; @@ -135,11 +135,6 @@ if (!responsible_frame) responsible_frame = frame; - // First frame in stacktrace should point to a real PC, not return address. - if (I != 0 && trace->GetSize() == 0) { - addr.Slide(-1); - } - lldb::addr_t PC = addr.GetLoadAddress(&target); trace->AddItem(StructuredData::ObjectSP(new StructuredData::Integer(PC))); } @@ -271,7 +266,11 @@ info->GetObjectForDotSeparatedPath("tid"); tid_t tid = thread_id_obj ? thread_id_obj->GetIntegerValue() : 0; - HistoryThread *history_thread = new HistoryThread(*process_sp, tid, PCs); + // We gather symbolication addresses above, so no need for HistoryThread to + // try to infer the call addresses. + bool pcs_are_call_addresses = true; + HistoryThread *history_thread = + new HistoryThread(*process_sp, tid, PCs, pcs_are_call_addresses); ThreadSP new_thread_sp(history_thread); // Save this in the Process' ExtendedThreadList so a strong pointer retains
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits