Skip to content

Commit 122f5a9

Browse files
committed
lldb/Instrumentation: NFC-ish use GetFrameCodeAddressForSymbolication()
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 Differential Revision: https://reviews.llvm.org/D101094 (cherry picked from commit 91e90cf)
1 parent d24c1c2 commit 122f5a9

File tree

2 files changed

+13
-12
lines changed

2 files changed

+13
-12
lines changed

lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -210,19 +210,14 @@ InstrumentationRuntimeMainThreadChecker::RetrieveReportData(
210210
StackFrameSP responsible_frame;
211211
for (unsigned I = 0; I < thread_sp->GetStackFrameCount(); ++I) {
212212
StackFrameSP frame = thread_sp->GetStackFrameAtIndex(I);
213-
Address addr = frame->GetFrameCodeAddress();
213+
Address addr = frame->GetFrameCodeAddressForSymbolication();
214214
if (addr.GetModule() == runtime_module_sp) // Skip PCs from the runtime.
215215
continue;
216216

217217
// The first non-runtime frame is responsible for the bug.
218218
if (!responsible_frame)
219219
responsible_frame = frame;
220220

221-
// First frame in stacktrace should point to a real PC, not return address.
222-
if (I != 0 && trace->GetSize() == 0) {
223-
addr.Slide(-1);
224-
}
225-
226221
lldb::addr_t PC = addr.GetLoadAddress(&target);
227222
trace->AddItem(StructuredData::ObjectSP(new StructuredData::Integer(PC)));
228223
}
@@ -365,8 +360,11 @@ InstrumentationRuntimeMainThreadChecker::GetBacktracesFromExtendedStopInfo(
365360
info->GetObjectForDotSeparatedPath("tid");
366361
tid_t tid = thread_id_obj ? thread_id_obj->GetIntegerValue() : 0;
367362

368-
HistoryThread *history_thread = new HistoryThread(*process_sp, tid, PCs);
369-
ThreadSP new_thread_sp(history_thread);
363+
// We gather symbolication addresses above, so no need for HistoryThread to
364+
// try to infer the call addresses.
365+
bool pcs_are_call_addresses = true;
366+
ThreadSP new_thread_sp = std::make_shared<HistoryThread>(
367+
*process_sp, tid, PCs, pcs_are_call_addresses);
370368

371369
// Save this in the Process' ExtendedThreadList so a strong pointer retains
372370
// the object

lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,8 @@ StructuredData::ObjectSP InstrumentationRuntimeUBSan::RetrieveReportData(
150150
StructuredData::Array *trace = new StructuredData::Array();
151151
auto trace_sp = StructuredData::ObjectSP(trace);
152152
for (unsigned I = 0; I < thread_sp->GetStackFrameCount(); ++I) {
153-
const Address FCA =
154-
thread_sp->GetStackFrameAtIndex(I)->GetFrameCodeAddress();
153+
const Address FCA = thread_sp->GetStackFrameAtIndex(I)
154+
->GetFrameCodeAddressForSymbolication();
155155
if (FCA.GetModule() == runtime_module_sp) // Skip PCs from the runtime.
156156
continue;
157157

@@ -324,8 +324,11 @@ InstrumentationRuntimeUBSan::GetBacktracesFromExtendedStopInfo(
324324
info->GetObjectForDotSeparatedPath("tid");
325325
tid_t tid = thread_id_obj ? thread_id_obj->GetIntegerValue() : 0;
326326

327-
HistoryThread *history_thread = new HistoryThread(*process_sp, tid, PCs);
328-
ThreadSP new_thread_sp(history_thread);
327+
// We gather symbolication addresses above, so no need for HistoryThread to
328+
// try to infer the call addresses.
329+
bool pcs_are_call_addresses = true;
330+
ThreadSP new_thread_sp = std::make_shared<HistoryThread>(
331+
*process_sp, tid, PCs, pcs_are_call_addresses);
329332
std::string stop_reason_description = GetStopReasonDescription(info);
330333
new_thread_sp->SetName(stop_reason_description.c_str());
331334

0 commit comments

Comments
 (0)