tsan: remove unused pc arguments

Remove pc argument of ThreadIgnoreEnd, ThreadIgnoreSyncEnd
and AcquireGlobal functions. It's unused and in some places
we don't even have a pc and pass 0 anyway.
Don't confuse readers and don't pretend that pc is needed
and that passing 0 is somehow deficient.

Use simpler convention for ThreadIgnoreBegin and ThreadIgnoreSyncBegin:
accept only pc instread of pc+save_stack. 0 pc means "don't save stack".

Reviewed By: vitalybuka

Differential Revision: https://reviews.llvm.org/D106973
This commit is contained in:
Dmitry Vyukov 2021-07-28 18:33:18 +02:00
parent aad17c55a8
commit 5acdfb7eda
9 changed files with 46 additions and 53 deletions

View File

@ -256,9 +256,7 @@ void __tsan_release_merge(ThreadState *thr, void *addr) {
Release(thr, 0, (uptr)addr); Release(thr, 0, (uptr)addr);
} }
void __tsan_finalizer_goroutine(ThreadState *thr) { void __tsan_finalizer_goroutine(ThreadState *thr) { AcquireGlobal(thr); }
AcquireGlobal(thr, 0);
}
void __tsan_mutex_before_lock(ThreadState *thr, uptr addr, uptr write) { void __tsan_mutex_before_lock(ThreadState *thr, uptr addr, uptr write) {
if (write) if (write)
@ -285,9 +283,7 @@ void __tsan_go_ignore_sync_begin(ThreadState *thr) {
ThreadIgnoreSyncBegin(thr, 0); ThreadIgnoreSyncBegin(thr, 0);
} }
void __tsan_go_ignore_sync_end(ThreadState *thr) { void __tsan_go_ignore_sync_end(ThreadState *thr) { ThreadIgnoreSyncEnd(thr); }
ThreadIgnoreSyncEnd(thr, 0);
}
void __tsan_report_count(u64 *pn) { void __tsan_report_count(u64 *pn) {
Lock lock(&ctx->report_mtx); Lock lock(&ctx->report_mtx);

View File

@ -14,7 +14,6 @@ class ScopedInterceptor {
void EnableIgnores(); void EnableIgnores();
private: private:
ThreadState *const thr_; ThreadState *const thr_;
const uptr pc_;
bool in_ignored_lib_; bool in_ignored_lib_;
bool ignoring_; bool ignoring_;
}; };

View File

@ -248,7 +248,7 @@ static ThreadSignalContext *SigCtx(ThreadState *thr) {
ScopedInterceptor::ScopedInterceptor(ThreadState *thr, const char *fname, ScopedInterceptor::ScopedInterceptor(ThreadState *thr, const char *fname,
uptr pc) uptr pc)
: thr_(thr), pc_(pc), in_ignored_lib_(false), ignoring_(false) { : thr_(thr), in_ignored_lib_(false), ignoring_(false) {
Initialize(thr); Initialize(thr);
if (!thr_->is_inited) return; if (!thr_->is_inited) return;
if (!thr_->ignore_interceptors) FuncEntry(thr, pc); if (!thr_->ignore_interceptors) FuncEntry(thr, pc);
@ -271,7 +271,7 @@ ScopedInterceptor::~ScopedInterceptor() {
void ScopedInterceptor::EnableIgnores() { void ScopedInterceptor::EnableIgnores() {
if (ignoring_) { if (ignoring_) {
ThreadIgnoreBegin(thr_, pc_, /*save_stack=*/false); ThreadIgnoreBegin(thr_, 0);
if (flags()->ignore_noninstrumented_modules) thr_->suppress_reports++; if (flags()->ignore_noninstrumented_modules) thr_->suppress_reports++;
if (in_ignored_lib_) { if (in_ignored_lib_) {
DCHECK(!thr_->in_ignored_lib); DCHECK(!thr_->in_ignored_lib);
@ -282,7 +282,7 @@ void ScopedInterceptor::EnableIgnores() {
void ScopedInterceptor::DisableIgnores() { void ScopedInterceptor::DisableIgnores() {
if (ignoring_) { if (ignoring_) {
ThreadIgnoreEnd(thr_, pc_); ThreadIgnoreEnd(thr_);
if (flags()->ignore_noninstrumented_modules) thr_->suppress_reports--; if (flags()->ignore_noninstrumented_modules) thr_->suppress_reports--;
if (in_ignored_lib_) { if (in_ignored_lib_) {
DCHECK(thr_->in_ignored_lib); DCHECK(thr_->in_ignored_lib);
@ -444,7 +444,7 @@ static int setup_at_exit_wrapper(ThreadState *thr, uptr pc, void(*f)(),
} else { } else {
res = REAL(__cxa_atexit)(cxa_at_exit_wrapper, ctx, dso); res = REAL(__cxa_atexit)(cxa_at_exit_wrapper, ctx, dso);
} }
ThreadIgnoreEnd(thr, pc); ThreadIgnoreEnd(thr);
return res; return res;
} }
@ -470,7 +470,7 @@ TSAN_INTERCEPTOR(int, on_exit, void(*f)(int, void*), void *arg) {
// because we do not see synchronization around atexit callback list. // because we do not see synchronization around atexit callback list.
ThreadIgnoreBegin(thr, pc); ThreadIgnoreBegin(thr, pc);
int res = REAL(on_exit)(on_exit_wrapper, ctx); int res = REAL(on_exit)(on_exit_wrapper, ctx);
ThreadIgnoreEnd(thr, pc); ThreadIgnoreEnd(thr);
return res; return res;
} }
#define TSAN_MAYBE_INTERCEPT_ON_EXIT TSAN_INTERCEPT(on_exit) #define TSAN_MAYBE_INTERCEPT_ON_EXIT TSAN_INTERCEPT(on_exit)
@ -955,7 +955,7 @@ extern "C" void *__tsan_thread_start_func(void *arg) {
Printf("ThreadSanitizer: failed to set thread key\n"); Printf("ThreadSanitizer: failed to set thread key\n");
Die(); Die();
} }
ThreadIgnoreEnd(thr, 0); ThreadIgnoreEnd(thr);
#endif #endif
while ((tid = atomic_load(&p->tid, memory_order_acquire)) == 0) while ((tid = atomic_load(&p->tid, memory_order_acquire)) == 0)
internal_sched_yield(); internal_sched_yield();
@ -1008,7 +1008,7 @@ TSAN_INTERCEPTOR(int, pthread_create,
ScopedIgnoreInterceptors ignore; ScopedIgnoreInterceptors ignore;
ThreadIgnoreBegin(thr, pc); ThreadIgnoreBegin(thr, pc);
res = REAL(pthread_create)(th, attr, __tsan_thread_start_func, &p); res = REAL(pthread_create)(th, attr, __tsan_thread_start_func, &p);
ThreadIgnoreEnd(thr, pc); ThreadIgnoreEnd(thr);
} }
if (res == 0) { if (res == 0) {
int tid = ThreadCreate(thr, pc, *(uptr*)th, IsStateDetached(detached)); int tid = ThreadCreate(thr, pc, *(uptr*)th, IsStateDetached(detached));
@ -1034,7 +1034,7 @@ TSAN_INTERCEPTOR(int, pthread_join, void *th, void **ret) {
int tid = ThreadConsumeTid(thr, pc, (uptr)th); int tid = ThreadConsumeTid(thr, pc, (uptr)th);
ThreadIgnoreBegin(thr, pc); ThreadIgnoreBegin(thr, pc);
int res = BLOCK_REAL(pthread_join)(th, ret); int res = BLOCK_REAL(pthread_join)(th, ret);
ThreadIgnoreEnd(thr, pc); ThreadIgnoreEnd(thr);
if (res == 0) { if (res == 0) {
ThreadJoin(thr, pc, tid); ThreadJoin(thr, pc, tid);
} }
@ -1069,7 +1069,7 @@ TSAN_INTERCEPTOR(int, pthread_tryjoin_np, void *th, void **ret) {
int tid = ThreadConsumeTid(thr, pc, (uptr)th); int tid = ThreadConsumeTid(thr, pc, (uptr)th);
ThreadIgnoreBegin(thr, pc); ThreadIgnoreBegin(thr, pc);
int res = REAL(pthread_tryjoin_np)(th, ret); int res = REAL(pthread_tryjoin_np)(th, ret);
ThreadIgnoreEnd(thr, pc); ThreadIgnoreEnd(thr);
if (res == 0) if (res == 0)
ThreadJoin(thr, pc, tid); ThreadJoin(thr, pc, tid);
else else
@ -1083,7 +1083,7 @@ TSAN_INTERCEPTOR(int, pthread_timedjoin_np, void *th, void **ret,
int tid = ThreadConsumeTid(thr, pc, (uptr)th); int tid = ThreadConsumeTid(thr, pc, (uptr)th);
ThreadIgnoreBegin(thr, pc); ThreadIgnoreBegin(thr, pc);
int res = BLOCK_REAL(pthread_timedjoin_np)(th, ret, abstime); int res = BLOCK_REAL(pthread_timedjoin_np)(th, ret, abstime);
ThreadIgnoreEnd(thr, pc); ThreadIgnoreEnd(thr);
if (res == 0) if (res == 0)
ThreadJoin(thr, pc, tid); ThreadJoin(thr, pc, tid);
else else
@ -2143,7 +2143,7 @@ TSAN_INTERCEPTOR(int, getaddrinfo, void *node, void *service,
// inside of getaddrinfo. So ignore memory accesses. // inside of getaddrinfo. So ignore memory accesses.
ThreadIgnoreBegin(thr, pc); ThreadIgnoreBegin(thr, pc);
int res = REAL(getaddrinfo)(node, service, hints, rv); int res = REAL(getaddrinfo)(node, service, hints, rv);
ThreadIgnoreEnd(thr, pc); ThreadIgnoreEnd(thr);
return res; return res;
} }

View File

@ -378,7 +378,7 @@ void INTERFACE_ATTRIBUTE AnnotateIgnoreReadsBegin(char *f, int l) {
void INTERFACE_ATTRIBUTE AnnotateIgnoreReadsEnd(char *f, int l) { void INTERFACE_ATTRIBUTE AnnotateIgnoreReadsEnd(char *f, int l) {
SCOPED_ANNOTATION(AnnotateIgnoreReadsEnd); SCOPED_ANNOTATION(AnnotateIgnoreReadsEnd);
ThreadIgnoreEnd(thr, pc); ThreadIgnoreEnd(thr);
} }
void INTERFACE_ATTRIBUTE AnnotateIgnoreWritesBegin(char *f, int l) { void INTERFACE_ATTRIBUTE AnnotateIgnoreWritesBegin(char *f, int l) {
@ -388,7 +388,7 @@ void INTERFACE_ATTRIBUTE AnnotateIgnoreWritesBegin(char *f, int l) {
void INTERFACE_ATTRIBUTE AnnotateIgnoreWritesEnd(char *f, int l) { void INTERFACE_ATTRIBUTE AnnotateIgnoreWritesEnd(char *f, int l) {
SCOPED_ANNOTATION(AnnotateIgnoreWritesEnd); SCOPED_ANNOTATION(AnnotateIgnoreWritesEnd);
ThreadIgnoreEnd(thr, pc); ThreadIgnoreEnd(thr);
} }
void INTERFACE_ATTRIBUTE AnnotateIgnoreSyncBegin(char *f, int l) { void INTERFACE_ATTRIBUTE AnnotateIgnoreSyncBegin(char *f, int l) {
@ -398,7 +398,7 @@ void INTERFACE_ATTRIBUTE AnnotateIgnoreSyncBegin(char *f, int l) {
void INTERFACE_ATTRIBUTE AnnotateIgnoreSyncEnd(char *f, int l) { void INTERFACE_ATTRIBUTE AnnotateIgnoreSyncEnd(char *f, int l) {
SCOPED_ANNOTATION(AnnotateIgnoreSyncEnd); SCOPED_ANNOTATION(AnnotateIgnoreSyncEnd);
ThreadIgnoreSyncEnd(thr, pc); ThreadIgnoreSyncEnd(thr);
} }
void INTERFACE_ATTRIBUTE AnnotatePublishMemoryRange( void INTERFACE_ATTRIBUTE AnnotatePublishMemoryRange(
@ -477,15 +477,15 @@ void __tsan_mutex_pre_lock(void *m, unsigned flagz) {
else else
MutexPreLock(thr, pc, (uptr)m); MutexPreLock(thr, pc, (uptr)m);
} }
ThreadIgnoreBegin(thr, pc, /*save_stack=*/false); ThreadIgnoreBegin(thr, 0);
ThreadIgnoreSyncBegin(thr, pc, /*save_stack=*/false); ThreadIgnoreSyncBegin(thr, 0);
} }
INTERFACE_ATTRIBUTE INTERFACE_ATTRIBUTE
void __tsan_mutex_post_lock(void *m, unsigned flagz, int rec) { void __tsan_mutex_post_lock(void *m, unsigned flagz, int rec) {
SCOPED_ANNOTATION(__tsan_mutex_post_lock); SCOPED_ANNOTATION(__tsan_mutex_post_lock);
ThreadIgnoreSyncEnd(thr, pc); ThreadIgnoreSyncEnd(thr);
ThreadIgnoreEnd(thr, pc); ThreadIgnoreEnd(thr);
if (!(flagz & MutexFlagTryLockFailed)) { if (!(flagz & MutexFlagTryLockFailed)) {
if (flagz & MutexFlagReadLock) if (flagz & MutexFlagReadLock)
MutexPostReadLock(thr, pc, (uptr)m, flagz); MutexPostReadLock(thr, pc, (uptr)m, flagz);
@ -504,44 +504,44 @@ int __tsan_mutex_pre_unlock(void *m, unsigned flagz) {
} else { } else {
ret = MutexUnlock(thr, pc, (uptr)m, flagz); ret = MutexUnlock(thr, pc, (uptr)m, flagz);
} }
ThreadIgnoreBegin(thr, pc, /*save_stack=*/false); ThreadIgnoreBegin(thr, 0);
ThreadIgnoreSyncBegin(thr, pc, /*save_stack=*/false); ThreadIgnoreSyncBegin(thr, 0);
return ret; return ret;
} }
INTERFACE_ATTRIBUTE INTERFACE_ATTRIBUTE
void __tsan_mutex_post_unlock(void *m, unsigned flagz) { void __tsan_mutex_post_unlock(void *m, unsigned flagz) {
SCOPED_ANNOTATION(__tsan_mutex_post_unlock); SCOPED_ANNOTATION(__tsan_mutex_post_unlock);
ThreadIgnoreSyncEnd(thr, pc); ThreadIgnoreSyncEnd(thr);
ThreadIgnoreEnd(thr, pc); ThreadIgnoreEnd(thr);
} }
INTERFACE_ATTRIBUTE INTERFACE_ATTRIBUTE
void __tsan_mutex_pre_signal(void *addr, unsigned flagz) { void __tsan_mutex_pre_signal(void *addr, unsigned flagz) {
SCOPED_ANNOTATION(__tsan_mutex_pre_signal); SCOPED_ANNOTATION(__tsan_mutex_pre_signal);
ThreadIgnoreBegin(thr, pc, /*save_stack=*/false); ThreadIgnoreBegin(thr, 0);
ThreadIgnoreSyncBegin(thr, pc, /*save_stack=*/false); ThreadIgnoreSyncBegin(thr, 0);
} }
INTERFACE_ATTRIBUTE INTERFACE_ATTRIBUTE
void __tsan_mutex_post_signal(void *addr, unsigned flagz) { void __tsan_mutex_post_signal(void *addr, unsigned flagz) {
SCOPED_ANNOTATION(__tsan_mutex_post_signal); SCOPED_ANNOTATION(__tsan_mutex_post_signal);
ThreadIgnoreSyncEnd(thr, pc); ThreadIgnoreSyncEnd(thr);
ThreadIgnoreEnd(thr, pc); ThreadIgnoreEnd(thr);
} }
INTERFACE_ATTRIBUTE INTERFACE_ATTRIBUTE
void __tsan_mutex_pre_divert(void *addr, unsigned flagz) { void __tsan_mutex_pre_divert(void *addr, unsigned flagz) {
SCOPED_ANNOTATION(__tsan_mutex_pre_divert); SCOPED_ANNOTATION(__tsan_mutex_pre_divert);
// Exit from ignore region started in __tsan_mutex_pre_lock/unlock/signal. // Exit from ignore region started in __tsan_mutex_pre_lock/unlock/signal.
ThreadIgnoreSyncEnd(thr, pc); ThreadIgnoreSyncEnd(thr);
ThreadIgnoreEnd(thr, pc); ThreadIgnoreEnd(thr);
} }
INTERFACE_ATTRIBUTE INTERFACE_ATTRIBUTE
void __tsan_mutex_post_divert(void *addr, unsigned flagz) { void __tsan_mutex_post_divert(void *addr, unsigned flagz) {
SCOPED_ANNOTATION(__tsan_mutex_post_divert); SCOPED_ANNOTATION(__tsan_mutex_post_divert);
ThreadIgnoreBegin(thr, pc, /*save_stack=*/false); ThreadIgnoreBegin(thr, 0);
ThreadIgnoreSyncBegin(thr, pc, /*save_stack=*/false); ThreadIgnoreSyncBegin(thr, 0);
} }
} // extern "C" } // extern "C"

View File

@ -112,9 +112,7 @@ void __tsan_ignore_thread_begin() {
ThreadIgnoreBegin(cur_thread(), CALLERPC); ThreadIgnoreBegin(cur_thread(), CALLERPC);
} }
void __tsan_ignore_thread_end() { void __tsan_ignore_thread_end() { ThreadIgnoreEnd(cur_thread()); }
ThreadIgnoreEnd(cur_thread(), CALLERPC);
}
void __tsan_read_range(void *addr, uptr size) { void __tsan_read_range(void *addr, uptr size) {
MemoryAccessRange(cur_thread(), CALLERPC, (uptr)addr, size, false); MemoryAccessRange(cur_thread(), CALLERPC, (uptr)addr, size, false);

View File

@ -168,7 +168,7 @@ jptr __tsan_java_find(jptr *from_ptr, jptr to) {
void __tsan_java_finalize() { void __tsan_java_finalize() {
SCOPED_JAVA_FUNC(__tsan_java_finalize); SCOPED_JAVA_FUNC(__tsan_java_finalize);
DPrintf("#%d: java_mutex_finalize()\n", thr->tid); DPrintf("#%d: java_mutex_finalize()\n", thr->tid);
AcquireGlobal(thr, 0); AcquireGlobal(thr);
} }
void __tsan_java_mutex_lock(jptr addr) { void __tsan_java_mutex_lock(jptr addr) {

View File

@ -1069,18 +1069,18 @@ void FuncExit(ThreadState *thr) {
thr->shadow_stack_pos--; thr->shadow_stack_pos--;
} }
void ThreadIgnoreBegin(ThreadState *thr, uptr pc, bool save_stack) { void ThreadIgnoreBegin(ThreadState *thr, uptr pc) {
DPrintf("#%d: ThreadIgnoreBegin\n", thr->tid); DPrintf("#%d: ThreadIgnoreBegin\n", thr->tid);
thr->ignore_reads_and_writes++; thr->ignore_reads_and_writes++;
CHECK_GT(thr->ignore_reads_and_writes, 0); CHECK_GT(thr->ignore_reads_and_writes, 0);
thr->fast_state.SetIgnoreBit(); thr->fast_state.SetIgnoreBit();
#if !SANITIZER_GO #if !SANITIZER_GO
if (save_stack && !ctx->after_multithreaded_fork) if (pc && !ctx->after_multithreaded_fork)
thr->mop_ignore_set.Add(CurrentStackId(thr, pc)); thr->mop_ignore_set.Add(CurrentStackId(thr, pc));
#endif #endif
} }
void ThreadIgnoreEnd(ThreadState *thr, uptr pc) { void ThreadIgnoreEnd(ThreadState *thr) {
DPrintf("#%d: ThreadIgnoreEnd\n", thr->tid); DPrintf("#%d: ThreadIgnoreEnd\n", thr->tid);
CHECK_GT(thr->ignore_reads_and_writes, 0); CHECK_GT(thr->ignore_reads_and_writes, 0);
thr->ignore_reads_and_writes--; thr->ignore_reads_and_writes--;
@ -1100,17 +1100,17 @@ uptr __tsan_testonly_shadow_stack_current_size() {
} }
#endif #endif
void ThreadIgnoreSyncBegin(ThreadState *thr, uptr pc, bool save_stack) { void ThreadIgnoreSyncBegin(ThreadState *thr, uptr pc) {
DPrintf("#%d: ThreadIgnoreSyncBegin\n", thr->tid); DPrintf("#%d: ThreadIgnoreSyncBegin\n", thr->tid);
thr->ignore_sync++; thr->ignore_sync++;
CHECK_GT(thr->ignore_sync, 0); CHECK_GT(thr->ignore_sync, 0);
#if !SANITIZER_GO #if !SANITIZER_GO
if (save_stack && !ctx->after_multithreaded_fork) if (pc && !ctx->after_multithreaded_fork)
thr->sync_ignore_set.Add(CurrentStackId(thr, pc)); thr->sync_ignore_set.Add(CurrentStackId(thr, pc));
#endif #endif
} }
void ThreadIgnoreSyncEnd(ThreadState *thr, uptr pc) { void ThreadIgnoreSyncEnd(ThreadState *thr) {
DPrintf("#%d: ThreadIgnoreSyncEnd\n", thr->tid); DPrintf("#%d: ThreadIgnoreSyncEnd\n", thr->tid);
CHECK_GT(thr->ignore_sync, 0); CHECK_GT(thr->ignore_sync, 0);
thr->ignore_sync--; thr->ignore_sync--;

View File

@ -737,10 +737,10 @@ void MemoryRangeImitateWrite(ThreadState *thr, uptr pc, uptr addr, uptr size);
void MemoryRangeImitateWriteOrResetRange(ThreadState *thr, uptr pc, uptr addr, void MemoryRangeImitateWriteOrResetRange(ThreadState *thr, uptr pc, uptr addr,
uptr size); uptr size);
void ThreadIgnoreBegin(ThreadState *thr, uptr pc, bool save_stack = true); void ThreadIgnoreBegin(ThreadState *thr, uptr pc);
void ThreadIgnoreEnd(ThreadState *thr, uptr pc); void ThreadIgnoreEnd(ThreadState *thr);
void ThreadIgnoreSyncBegin(ThreadState *thr, uptr pc, bool save_stack = true); void ThreadIgnoreSyncBegin(ThreadState *thr, uptr pc);
void ThreadIgnoreSyncEnd(ThreadState *thr, uptr pc); void ThreadIgnoreSyncEnd(ThreadState *thr);
void FuncEntry(ThreadState *thr, uptr pc); void FuncEntry(ThreadState *thr, uptr pc);
void FuncExit(ThreadState *thr); void FuncExit(ThreadState *thr);
@ -785,7 +785,7 @@ void Acquire(ThreadState *thr, uptr pc, uptr addr);
// handle Go finalizers. Namely, finalizer goroutine executes AcquireGlobal // handle Go finalizers. Namely, finalizer goroutine executes AcquireGlobal
// right before executing finalizers. This provides a coarse, but simple // right before executing finalizers. This provides a coarse, but simple
// approximation of the actual required synchronization. // approximation of the actual required synchronization.
void AcquireGlobal(ThreadState *thr, uptr pc); void AcquireGlobal(ThreadState *thr);
void Release(ThreadState *thr, uptr pc, uptr addr); void Release(ThreadState *thr, uptr pc, uptr addr);
void ReleaseStoreAcquire(ThreadState *thr, uptr pc, uptr addr); void ReleaseStoreAcquire(ThreadState *thr, uptr pc, uptr addr);
void ReleaseStore(ThreadState *thr, uptr pc, uptr addr); void ReleaseStore(ThreadState *thr, uptr pc, uptr addr);

View File

@ -413,7 +413,7 @@ static void UpdateClockCallback(ThreadContextBase *tctx_base, void *arg) {
thr->clock.set(&thr->proc()->clock_cache, tctx->tid, epoch); thr->clock.set(&thr->proc()->clock_cache, tctx->tid, epoch);
} }
void AcquireGlobal(ThreadState *thr, uptr pc) { void AcquireGlobal(ThreadState *thr) {
DPrintf("#%d: AcquireGlobal\n", thr->tid); DPrintf("#%d: AcquireGlobal\n", thr->tid);
if (thr->ignore_sync) if (thr->ignore_sync)
return; return;