[PATCH] D31553: [tsan] Ignore memory accesses for libignored modules for "external" races
Dmitry Vyukov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 21 06:05:39 PDT 2017
dvyukov accepted this revision.
dvyukov added inline comments.
This revision is now accepted and ready to land.
================
Comment at: lib/tsan/rtl/tsan_external.cc:61
thr->external_tag = (uptr)tag;
- FuncEntry(thr, (uptr)caller_pc);
- MemoryRead(thr, CALLERPC, (uptr)addr, kSizeLog8);
- FuncExit(thr);
+ if (caller_pc) FuncEntry(thr, (uptr)caller_pc);
+ bool in_ignored_lib;
----------------
I think I understand why I was confused. I assumed that caller_pc is literally caller pc. And caller can well point into the same library, and then this all does not make sense. But you seem to assume that caller_pc points user code (can be caller pc or several frames above that).
We now have include/sanitizer/tsan_interface.h, so please add __tsan_external_* functions there with proper comments that document these assumptions.
================
Comment at: lib/tsan/rtl/tsan_external.cc:64
+ if (!caller_pc || !libignore()->IsIgnored((uptr)caller_pc, &in_ignored_lib)) {
+ MemoryRead(thr, CALLERPC, (uptr)addr, kSizeLog8);
+ }
----------------
Unrelated to this change, but why did we decide to use kSizeLog8? It would be more reasonable to use kSizeLog1. A library may need several different addresses to model several unrelated things and if the object is only 8 bytes, it won't be able to do so. Also if size of the object is only, say, 4 bytes, 8-byte access can lead to false positives.
Is there any reason to use 8? If not, please change it to 1.
Repository:
rL LLVM
https://reviews.llvm.org/D31553
More information about the llvm-commits
mailing list