[PATCH] D99434: [TSAN] Honor acquire failure mode on AtomicCAS

Bruno Cardoso Lopes via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 6 17:33:06 PDT 2021


bruno added a comment.

Thanks both of you for the review @delcypher and @yln, will wait for @dvyukov to sign-off!



================
Comment at: compiler-rt/lib/tsan/rtl/tsan_interface_atomic.cpp:433
+  // Honor failure memory order.
+  CHECK(IsLoadOrder(fmo));
+  if (fmo != mo_acquire)
----------------
yln wrote:
> delcypher wrote:
> > Is this something user facing code can set? If yes, then we might want to emit a warning rather than crashing the process.
> I think calls are generated by the compiler in `ThreadSanitizer::instrumentAtomic()`:
> ```
>     Value *Args[] = {IRB.CreatePointerCast(Addr, PtrTy),
>                      CmpOperand,
>                      NewOperand,
>                      createOrdering(&IRB, CASI->getSuccessOrdering()),
>                      createOrdering(&IRB, CASI->getFailureOrdering())};
>     CallInst *C = IRB.CreateCall(TsanAtomicCAS[Idx], Args);
> ```
Right, but before things are expanded for instrumentation, there's some sanitization of the failure memory order in `emitAtomicCmpXchgFailureSet`, so an invalid combination shouldn't get to this point.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99434/new/

https://reviews.llvm.org/D99434



More information about the llvm-commits mailing list