Skip to content

[tsan] Fix deadlock with dyld during symbolization on darwin platforms #113661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions compiler-rt/lib/tsan/go/tsan_go.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,26 @@ ReportLocation *SymbolizeData(uptr addr) {
}
}

bool SymbolizeData(uptr addr, DataInfo *info) {
SymbolizeDataContext cbctx;
internal_memset(&cbctx, 0, sizeof(cbctx));
cbctx.addr = addr;
go_runtime_cb(CallbackSymbolizeData, &cbctx);
if (!cbctx.res)
return false;
if (cbctx.heap) {
return false;
} else {
info->Clear();
info->name = internal_strdup(cbctx.name ? cbctx.name : "??");
info->file = internal_strdup(cbctx.file ? cbctx.file : "??");
info->line = cbctx.line;
info->start = cbctx.start;
info->size = cbctx.size;
return true;
}
}

static ThreadState *main_thr;
static bool inited;

Expand Down
12 changes: 8 additions & 4 deletions compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2064,13 +2064,17 @@ static void ReportErrnoSpoiling(ThreadState *thr, uptr pc, int sig) {
// StackTrace::GetNestInstructionPc(pc) is used because return address is
// expected, OutputReport() will undo this.
ObtainCurrentStack(thr, StackTrace::GetNextInstructionPc(pc), &stack);
ThreadRegistryLock l(&ctx->thread_registry);
if (IsFiredSuppression(ctx, ReportTypeErrnoInSignal, stack)) {
return;
}
ScopedReport rep(ReportTypeErrnoInSignal);
rep.SetSigNum(sig);
if (!IsFiredSuppression(ctx, ReportTypeErrnoInSignal, stack)) {
{
// ThreadRegistryLock l(&ctx->thread_registry);
rep.SetSigNum(sig);
rep.AddStack(stack, true);
OutputReport(thr, rep);
}
rep.SymbolizeReport();
OutputReport(thr, rep);
}

static void CallUserSignalHandler(ThreadState *thr, bool sync, bool acquire,
Expand Down
17 changes: 10 additions & 7 deletions compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -437,15 +437,18 @@ void __tsan_mutex_post_divert(void *addr, unsigned flagz) {
}

static void ReportMutexHeldWrongContext(ThreadState *thr, uptr pc) {
ThreadRegistryLock l(&ctx->thread_registry);
ScopedReport rep(ReportTypeMutexHeldWrongContext);
for (uptr i = 0; i < thr->mset.Size(); ++i) {
MutexSet::Desc desc = thr->mset.Get(i);
rep.AddMutex(desc.addr, desc.stack_id);
{
ThreadRegistryLock l(&ctx->thread_registry);
for (uptr i = 0; i < thr->mset.Size(); ++i) {
MutexSet::Desc desc = thr->mset.Get(i);
rep.AddMutex(desc.addr, desc.stack_id);
}
VarSizeStackTrace trace;
ObtainCurrentStack(thr, pc, &trace);
rep.AddStack(trace, true);
}
VarSizeStackTrace trace;
ObtainCurrentStack(thr, pc, &trace);
rep.AddStack(trace, true);
rep.SymbolizeReport();
OutputReport(thr, rep);
}

Expand Down
7 changes: 5 additions & 2 deletions compiler-rt/lib/tsan/rtl/tsan_mman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,12 @@ static void SignalUnsafeCall(ThreadState *thr, uptr pc) {
ObtainCurrentStack(thr, pc, &stack);
if (IsFiredSuppression(ctx, ReportTypeSignalUnsafe, stack))
return;
ThreadRegistryLock l(&ctx->thread_registry);
ScopedReport rep(ReportTypeSignalUnsafe);
rep.AddStack(stack, true);
{
// ThreadRegistryLock l(&ctx->thread_registry);
rep.AddStack(stack, true);
}
rep.SymbolizeReport();
OutputReport(thr, rep);
}

Expand Down
2 changes: 2 additions & 0 deletions compiler-rt/lib/tsan/rtl/tsan_report.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "sanitizer_common/sanitizer_thread_registry.h"
#include "sanitizer_common/sanitizer_vector.h"
#include "tsan_defs.h"
#include "tsan_stack_trace.h"

namespace __tsan {

Expand All @@ -39,6 +40,7 @@ enum ReportType {
};

struct ReportStack {
VarSizeStackTrace raw;
SymbolizedStack *frames = nullptr;
bool suppressable = false;
};
Expand Down
10 changes: 4 additions & 6 deletions compiler-rt/lib/tsan/rtl/tsan_rtl.h
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,7 @@ uptr TagFromShadowStackFrame(uptr pc);

class ScopedReportBase {
public:
void SetKindAndTag(ReportType typ, uptr tag);
void AddMemoryAccess(uptr addr, uptr external_tag, Shadow s, Tid tid,
StackTrace stack, const MutexSet *mset);
void AddStack(StackTrace stack, bool suppressable = false);
Expand All @@ -418,29 +419,26 @@ class ScopedReportBase {
void SetCount(int count);
void SetSigNum(int sig);

void SymbolizeReport();
const ReportDesc *GetReport() const;

protected:
ScopedReportBase();
ScopedReportBase(ReportType typ, uptr tag);
~ScopedReportBase();

private:
ReportDesc *rep_;
// Symbolizer makes lots of intercepted calls. If we try to process them,
// at best it will cause deadlocks on internal mutexes.
ScopedIgnoreInterceptors ignore_interceptors_;

ScopedReportBase(const ScopedReportBase &) = delete;
void operator=(const ScopedReportBase &) = delete;
};

class ScopedReport : public ScopedReportBase {
public:
explicit ScopedReport();
explicit ScopedReport(ReportType typ, uptr tag = kExternalTagNone);
~ScopedReport();

private:
ScopedErrorReportLock lock_;
};

bool ShouldReport(ThreadState *thr, ReportType typ);
Expand Down
89 changes: 49 additions & 40 deletions compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,16 @@ static void ReportMutexMisuse(ThreadState *thr, uptr pc, ReportType typ,
return;
if (!ShouldReport(thr, typ))
return;
ThreadRegistryLock l(&ctx->thread_registry);
ScopedReport rep(typ);
rep.AddMutex(addr, creation_stack_id);
VarSizeStackTrace trace;
ObtainCurrentStack(thr, pc, &trace);
rep.AddStack(trace, true);
rep.AddLocation(addr, 1);
{
ThreadRegistryLock l(&ctx->thread_registry);
rep.AddMutex(addr, creation_stack_id);
VarSizeStackTrace trace;
ObtainCurrentStack(thr, pc, &trace);
rep.AddStack(trace, true);
rep.AddLocation(addr, 1);
}
rep.SymbolizeReport();
OutputReport(thr, rep);
}

Expand Down Expand Up @@ -528,50 +531,56 @@ void AfterSleep(ThreadState *thr, uptr pc) {
void ReportDeadlock(ThreadState *thr, uptr pc, DDReport *r) {
if (r == 0 || !ShouldReport(thr, ReportTypeDeadlock))
return;
ThreadRegistryLock l(&ctx->thread_registry);
ScopedReport rep(ReportTypeDeadlock);
for (int i = 0; i < r->n; i++) {
rep.AddMutex(r->loop[i].mtx_ctx0, r->loop[i].stk[0]);
rep.AddUniqueTid((int)r->loop[i].thr_ctx);
rep.AddThread((int)r->loop[i].thr_ctx);
}
uptr dummy_pc = 0x42;
for (int i = 0; i < r->n; i++) {
for (int j = 0; j < (flags()->second_deadlock_stack ? 2 : 1); j++) {
u32 stk = r->loop[i].stk[j];
if (stk && stk != kInvalidStackID) {
rep.AddStack(StackDepotGet(stk), true);
} else {
// Sometimes we fail to extract the stack trace (FIXME: investigate),
// but we should still produce some stack trace in the report.
rep.AddStack(StackTrace(&dummy_pc, 1), true);
{
ThreadRegistryLock l(&ctx->thread_registry);
for (int i = 0; i < r->n; i++) {
rep.AddMutex(r->loop[i].mtx_ctx0, r->loop[i].stk[0]);
rep.AddUniqueTid((int)r->loop[i].thr_ctx);
rep.AddThread((int)r->loop[i].thr_ctx);
}
uptr dummy_pc = 0x42;
for (int i = 0; i < r->n; i++) {
for (int j = 0; j < (flags()->second_deadlock_stack ? 2 : 1); j++) {
u32 stk = r->loop[i].stk[j];
if (stk && stk != kInvalidStackID) {
rep.AddStack(StackDepotGet(stk), true);
} else {
// Sometimes we fail to extract the stack trace (FIXME: investigate),
// but we should still produce some stack trace in the report.
rep.AddStack(StackTrace(&dummy_pc, 1), true);
}
}
}
}
rep.SymbolizeReport();
OutputReport(thr, rep);
}

void ReportDestroyLocked(ThreadState *thr, uptr pc, uptr addr,
FastState last_lock, StackID creation_stack_id) {
// We need to lock the slot during RestoreStack because it protects
// the slot journal.
Lock slot_lock(&ctx->slots[static_cast<uptr>(last_lock.sid())].mtx);
ThreadRegistryLock l0(&ctx->thread_registry);
Lock slots_lock(&ctx->slot_mtx);
ScopedReport rep(ReportTypeMutexDestroyLocked);
rep.AddMutex(addr, creation_stack_id);
VarSizeStackTrace trace;
ObtainCurrentStack(thr, pc, &trace);
rep.AddStack(trace, true);

Tid tid;
DynamicMutexSet mset;
uptr tag;
if (!RestoreStack(EventType::kLock, last_lock.sid(), last_lock.epoch(), addr,
0, kAccessWrite, &tid, &trace, mset, &tag))
return;
rep.AddStack(trace, true);
rep.AddLocation(addr, 1);
{
// We need to lock the slot during RestoreStack because it protects
// the slot journal.
Lock slot_lock(&ctx->slots[static_cast<uptr>(last_lock.sid())].mtx);
ThreadRegistryLock l0(&ctx->thread_registry);
Lock slots_lock(&ctx->slot_mtx);
rep.AddMutex(addr, creation_stack_id);
VarSizeStackTrace trace;
ObtainCurrentStack(thr, pc, &trace);
rep.AddStack(trace, true);

Tid tid;
DynamicMutexSet mset;
uptr tag;
if (!RestoreStack(EventType::kLock, last_lock.sid(), last_lock.epoch(),
addr, 0, kAccessWrite, &tid, &trace, mset, &tag))
return;
rep.AddStack(trace, true);
rep.AddLocation(addr, 1);
}
rep.SymbolizeReport();
OutputReport(thr, rep);
}

Expand Down
Loading
Loading