[PATCH] D58744: [CodeGen] Fix some broken IR generated by -fsanitize=unsigned-integer-overflow

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: rjmccall, arphaman, ahatanak.
Herald added subscribers: jdoerfert, jfb, dexonsmith, jkorous.
Herald added a project: clang.

I think the author of the function assumed that `GetInsertBlock()` wouldn't 
change from where `atomicPHI` was created, but this isn't true when 
`-fsanitize=unsigned-integer-overflow` is enabled (we generate an 
overflow/continuation label). Fix by keeping track of the block we want to 
return to to complete the cmpxchg loop.

rdar://48406558

Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D58744

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/sanitize-atomic-int-overflow.c


Index: clang/test/CodeGen/sanitize-atomic-int-overflow.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/sanitize-atomic-int-overflow.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -fsanitize=unsigned-integer-overflow %s -emit-llvm -o -
+
+_Atomic(unsigned) atomic;
+
+void cmpd_assign() {
+  atomic += 1;
+}
+
+void inc() {
+  atomic++;
+}
Index: clang/lib/CodeGen/CGExprScalar.cpp
===================================================================
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -2350,6 +2350,7 @@
 
   QualType type = E->getSubExpr()->getType();
   llvm::PHINode *atomicPHI = nullptr;
+  llvm::BasicBlock *atomicOpBB = nullptr;
   llvm::Value *value;
   llvm::Value *input;
 
@@ -2393,10 +2394,10 @@
     input = value;
     // For every other atomic operation, we need to emit a load-op-cmpxchg loop
     llvm::BasicBlock *startBB = Builder.GetInsertBlock();
-    llvm::BasicBlock *opBB = CGF.createBasicBlock("atomic_op", CGF.CurFn);
+    atomicOpBB = CGF.createBasicBlock("atomic_op", CGF.CurFn);
     value = CGF.EmitToMemory(value, type);
-    Builder.CreateBr(opBB);
-    Builder.SetInsertPoint(opBB);
+    Builder.CreateBr(atomicOpBB);
+    Builder.SetInsertPoint(atomicOpBB);
     atomicPHI = Builder.CreatePHI(value->getType(), 2);
     atomicPHI->addIncoming(value, startBB);
     value = atomicPHI;
@@ -2555,14 +2556,14 @@
   }
 
   if (atomicPHI) {
-    llvm::BasicBlock *opBB = Builder.GetInsertBlock();
+    llvm::BasicBlock *curBlock = Builder.GetInsertBlock();
     llvm::BasicBlock *contBB = CGF.createBasicBlock("atomic_cont", CGF.CurFn);
     auto Pair = CGF.EmitAtomicCompareExchange(
         LV, RValue::get(atomicPHI), RValue::get(value), E->getExprLoc());
     llvm::Value *old = CGF.EmitToMemory(Pair.first.getScalarVal(), type);
     llvm::Value *success = Pair.second;
-    atomicPHI->addIncoming(old, opBB);
-    Builder.CreateCondBr(success, contBB, opBB);
+    atomicPHI->addIncoming(old, curBlock);
+    Builder.CreateCondBr(success, contBB, atomicOpBB);
     Builder.SetInsertPoint(contBB);
     return isPre ? value : input;
   }
@@ -2838,6 +2839,7 @@
   LValue LHSLV = EmitCheckedLValue(E->getLHS(), CodeGenFunction::TCK_Store);
 
   llvm::PHINode *atomicPHI = nullptr;
+  llvm::BasicBlock *atomicOpBB = nullptr;
   if (const AtomicType *atomicTy = LHSTy->getAs<AtomicType>()) {
     QualType type = atomicTy->getValueType();
     if (!type->isBooleanType() && type->isIntegerType() &&
@@ -2884,11 +2886,11 @@
     // FIXME: For floating point types, we should be saving and restoring the
     // floating point environment in the loop.
     llvm::BasicBlock *startBB = Builder.GetInsertBlock();
-    llvm::BasicBlock *opBB = CGF.createBasicBlock("atomic_op", CGF.CurFn);
+    atomicOpBB = CGF.createBasicBlock("atomic_op", CGF.CurFn);
     OpInfo.LHS = EmitLoadOfLValue(LHSLV, E->getExprLoc());
     OpInfo.LHS = CGF.EmitToMemory(OpInfo.LHS, type);
-    Builder.CreateBr(opBB);
-    Builder.SetInsertPoint(opBB);
+    Builder.CreateBr(atomicOpBB);
+    Builder.SetInsertPoint(atomicOpBB);
     atomicPHI = Builder.CreatePHI(OpInfo.LHS->getType(), 2);
     atomicPHI->addIncoming(OpInfo.LHS, startBB);
     OpInfo.LHS = atomicPHI;
@@ -2909,14 +2911,14 @@
                                 Loc, ScalarConversionOpts(CGF.SanOpts));
 
   if (atomicPHI) {
-    llvm::BasicBlock *opBB = Builder.GetInsertBlock();
+    llvm::BasicBlock *curBlock = Builder.GetInsertBlock();
     llvm::BasicBlock *contBB = CGF.createBasicBlock("atomic_cont", CGF.CurFn);
     auto Pair = CGF.EmitAtomicCompareExchange(
         LHSLV, RValue::get(atomicPHI), RValue::get(Result), E->getExprLoc());
     llvm::Value *old = CGF.EmitToMemory(Pair.first.getScalarVal(), LHSTy);
     llvm::Value *success = Pair.second;
-    atomicPHI->addIncoming(old, opBB);
-    Builder.CreateCondBr(success, contBB, opBB);
+    atomicPHI->addIncoming(old, curBlock);
+    Builder.CreateCondBr(success, contBB, atomicOpBB);
     Builder.SetInsertPoint(contBB);
     return LHSLV;
   }

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits