[PATCH] D28836: [tsan] Provide API for libraries for race detection on custom objects

[Dmitry Vyukov via Phabricator via llvm-commits]

dvyukov added inline comments.


================
Comment at: lib/tsan/rtl/tsan_external.cc:55
+  
+SANITIZER_INTERFACE_ATTRIBUTE void __tsan_external_read(void *addr, void *caller_pc, void *tag) {
+  ThreadState *thr = cur_thread();
----------------
kubamracek wrote:
> dvyukov wrote:
> > caller_pc is unsed
> > 80 columns please
> I kept this unused "for now" and only planned to use it in subsequent patches.  I think it's important to have access to caller_pc, because it's the "responsible frame" (frame that contains the bug), and it might not necessarily be `__builtin_return_address(1)`.  One thing is duplicate suppression -- we might want to base dup detection on the caller rather than the library frame.  Second thing is the "SUMMARY" line, which should point to the responsible frame (but it doesn't with the current patch).  Third thing is knowing which module contains the responsible frame -- the module should be used for `ignore_noninstrumented_modules` flag (and not the library itself).
> 
> Sorry for not explaining before.  Let me know if you think I need to address those in the patch already, or if you think we don't need caller_pc at all.
Why don't you call FuncEntry(caller_pc)/FuncExit(caller_pc) around the accesses?
That will use the argument, print the missing frame in reports and solve the suppressions problem.


https://reviews.llvm.org/D28836