[PATCH] D83949: [TSan] Do not compound reads and writes when separated by atomics

[Marco Elver via Phabricator via llvm-commits]

melver created this revision.
melver added reviewers: dvyukov, glider.
Herald added subscribers: llvm-commits, jfb, hiraditya.
Herald added a project: LLVM.

Do not compound reads and writes to the same address when separated by
atomic operations, including fences.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83949

Files:
  llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
  llvm/test/Instrumentation/ThreadSanitizer/read_before_write.ll


Index: llvm/test/Instrumentation/ThreadSanitizer/read_before_write.ll
===================================================================
--- llvm/test/Instrumentation/ThreadSanitizer/read_before_write.ll
+++ llvm/test/Instrumentation/ThreadSanitizer/read_before_write.ll
@@ -37,6 +37,50 @@
 
 declare void @foo()
 
+define void @IncrementWithFence(i32* nocapture %ptr) nounwind uwtable sanitize_thread {
+entry:
+  %0 = load i32, i32* %ptr, align 4
+  %inc = add nsw i32 %0, 1
+  fence seq_cst
+  store i32 %inc, i32* %ptr, align 4
+  ret void
+}
+
+; CHECK-LABEL: define void @IncrementWithFence
+; CHECK: __tsan_read4
+; CHECK: __tsan_atomic_thread_fence
+; CHECK: __tsan_write4
+; CHECK: ret void
+
+define void @IncrementWithAtomic(i32* nocapture %ptr) nounwind uwtable sanitize_thread {
+entry:
+  %0 = load i32, i32* %ptr, align 4
+  %inc = add nsw i32 %0, 1
+  %1 = load atomic i32, i32* %ptr acquire, align 4
+  store i32 %inc, i32* %ptr, align 4
+  ret void
+}
+
+; CHECK-LABEL: define void @IncrementWithAtomic
+; CHECK: __tsan_read4
+; CHECK: __tsan_atomic32_load
+; CHECK: __tsan_write4
+; CHECK: ret void
+
+define void @IncrementWithAsm(i32* nocapture %ptr) nounwind uwtable sanitize_thread {
+entry:
+  %0 = load i32, i32* %ptr, align 4
+  %inc = add nsw i32 %0, 1
+  call void asm sideeffect "", ""()
+  store i32 %inc, i32* %ptr, align 4
+  ret void
+}
+
+; CHECK-LABEL: define void @IncrementWithAsm
+; CHECK: __tsan_read4
+; CHECK: __tsan_write4
+; CHECK: ret void
+
 define void @VolatileLoad(i32* nocapture %ptr) nounwind uwtable sanitize_thread {
 entry:
   %0 = load volatile i32, i32* %ptr, align 4
Index: llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
===================================================================
--- llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
@@ -546,19 +546,27 @@
   // Traverse all instructions, collect loads/stores/returns, check for calls.
   for (auto &BB : F) {
     for (auto &Inst : BB) {
-      if (isAtomic(&Inst))
+      bool Barrier = false;
+
+      if (isAtomic(&Inst)) {
         AtomicAccesses.push_back(&Inst);
-      else if (isa<LoadInst>(Inst) || isa<StoreInst>(Inst))
+        Barrier = true;
+      } else if (isa<LoadInst>(Inst) || isa<StoreInst>(Inst)) {
         LocalLoadsAndStores.push_back(&Inst);
-      else if (isa<CallInst>(Inst) || isa<InvokeInst>(Inst)) {
+      } else if (isa<CallInst>(Inst) || isa<InvokeInst>(Inst)) {
         if (CallInst *CI = dyn_cast<CallInst>(&Inst))
           maybeMarkSanitizerLibraryCallNoBuiltin(CI, &TLI);
         if (isa<MemIntrinsic>(Inst))
           MemIntrinCalls.push_back(&Inst);
         HasCalls = true;
+        Barrier = true;
+      }
+
+      if (Barrier)
+        // Flush instructions found so far, avoiding compounding instrumentation
+        // with any instructions that follow.
         chooseInstructionsToInstrument(LocalLoadsAndStores, AllLoadsAndStores,
                                        DL);
-      }
     }
     chooseInstructionsToInstrument(LocalLoadsAndStores, AllLoadsAndStores, DL);
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D83949.278457.patch
Type: text/x-patch
Size: 3125 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200716/aa18bf0c/attachment.bin>