[PATCH] D28264: [tsan] Implement a 'ignore_noninstrumented_modules' flag to better suppress false positive races

[Dmitry Vyukov via Phabricator via llvm-commits]

dvyukov added inline comments.


================
Comment at: lib/tsan/rtl/tsan_interceptors.cc:270
   if (flags()->ignore_interceptors_accesses) ThreadIgnoreBegin(thr_, pc_);
+  if (flags()->ignore_noninstrumented_modules &&
+      !libignore()->IsPcInstrumented(pc_)) {
----------------
These changes to ScopedInterceptor does not feel right to me.
We already have checks of libignore()->IsIgnored() and undo of ignoring in all the same places you added new code. So it is better to piggy back on the existing logic to avoid duplication. We can either (1) pass the new flag to IsIgnored:

if (!thr_->in_ignored_lib && libignore()->IsIgnored(pc, flags()->ignore_noninstrumented_modules))

then it will also check the pc against instrumented modules; or (2) collect instrumented modules in LibIgnore only if the flag is set, then IsIgnored will check:

  if pc is in ignored libraries { return true }
  if instrumented libraries are not empty and pc is not in instrumented libraries { return true }
  return false

Or (3) if the flag is set add non-instrumented libraries to ignored libraries.
As far as I understand you don't want to do (3) because there can be lots of non-instrumented libraries.




Repository:
  rL LLVM

https://reviews.llvm.org/D28264