A previous change [1] moved retrieval of the caller PC
(__builtin_return_address(0) via CALLERPC) from an
interface-boundary function into a shared helper function
ExternalAccess. If this function does not get inlined, we fail to
collect the appropriate caller PC for the "TSan interface boundary".
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
compiler-rt/lib/tsan/rtl/tsan_external.cpp | ||
---|---|---|
62 | Another potential solution would have been marking this function with ALWAYS_INLINE plus a comment explaining that we need it to ensure CALLERPC does the right thing. I chose the current solution since:
|
compiler-rt/lib/tsan/rtl/tsan_external.cpp | ||
---|---|---|
71 | Not related to the current change. |
compiler-rt/lib/tsan/rtl/tsan_external.cpp | ||
---|---|---|
71 | This is the change that added handling for the external tag: https://reviews.llvm.org/D32382
I don't know the answer to this question. @kubamracek, do you remember? // - __tsan_external_read/__tsan_external_write annotates the logical reads // and writes of the object at the specified address. 'caller_pc' should // be the PC of the library user, which the library can obtain with e.g. // `__builtin_return_address(0)`. My understanding of of the handling of the caller PC is this. The caller of __tsan_external_[write]read can decide who to "blame" for the memory operation. User code would want to say "myself"; library code would want to say "my caller":
So this means we execute the following sequence when we are called from user code (caller_pc=nullptr): InsertShadowStackFrameForTag(thr, (uptr)tag); MemoryAccess(thr, tsan_caller_pc, (uptr)addr, 1, typ); FuncExit(thr); For calls from library code (caller_pc=<user code addr>): FuncEntry(thr, caller_pc); InsertShadowStackFrameForTag(thr, (uptr)tag); if (... || !libignore()->IsIgnored(caller_pc, &in_ignored_lib)) // <user code addr> not in ignored MemoryAccess(thr, tsan_caller_pc, (uptr)addr, 1, typ); FuncExit(thr); FuncExit(thr); So I think the FuncEntry/Exit are superfluous in case we are calling from an ignored module into an annotated library that wants to attribute the access to the calling user code. I am also not sure why we always use tsan_caller_pc even if we have a valid, "not ignored" user address for caller_pc. I think the function could look like this: void ExternalAccess(void *addr, uptr caller_pc, uptr tsan_caller_pc, void *tag, AccessType typ) { CHECK_LT(tag, atomic_load(&used_tags, memory_order_relaxed)); bool in_ignored_lib; if (caller_pc && libignore()->IsIgnored(caller_pc, &in_ignored_lib)) return; // <-- early return in case we don't do a memory access check ThreadState *thr = cur_thread(); if (caller_pc) FuncEntry(thr, caller_pc); InsertShadowStackFrameForTag(thr, (uptr)tag); MemoryAccess(thr, (caller_pc ? caller_pc : tsan_caller_pc), (uptr)addr, 1, typ); // <-- use caller_pc if we have it to remain consistent FuncExit(thr); if (caller_pc) FuncExit(thr); } @dvyukov, what do you think? (as a follow-up) |
Another potential solution would have been marking this function with ALWAYS_INLINE plus a comment explaining that we need it to ensure CALLERPC does the right thing. I chose the current solution since: