[safestack] Sink unsafe address computation to each use.

This is a fix for PR27844.
When replacing uses of unsafe allocas, emit the new location
immediately after each use. Without this, the pointer stays live from
the function entry to the last use, while it's usually cheaper to
recalculate.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@272969 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/CodeGen/SafeStack.cpp b/lib/CodeGen/SafeStack.cpp
index 28f5ed861ab9baec862b08edd829d695d9b99618..457acb1503c9b3f863d751e4bff516af81b26214 100644 (file)
--- a/lib/CodeGen/SafeStack.cpp
+++ b/lib/CodeGen/SafeStack.cpp
@@ -606,16 +606,39 @@ Value *SafeStack::moveStaticAllocasToUnsafeStack(
     StaticOffset += Size;
     StaticOffset = alignTo(StaticOffset, Align);
 
-    Value *Off = IRB.CreateGEP(BasePointer, // BasePointer is i8*
-                               ConstantInt::get(Int32Ty, -StaticOffset));
-    Value *NewAI = IRB.CreateBitCast(Off, AI->getType(), AI->getName());
-    if (AI->hasName() && isa<Instruction>(NewAI))
-      cast<Instruction>(NewAI)->takeName(AI);
-
-    // Replace alloc with the new location.
     replaceDbgDeclareForAlloca(AI, BasePointer, DIB, /*Deref=*/true, -StaticOffset);
     replaceDbgValueForAlloca(AI, BasePointer, DIB, -StaticOffset);
-    AI->replaceAllUsesWith(NewAI);
+
+    // Replace uses of the alloca with the new location.
+    // Insert address calculation close to each use to work around PR27844.
+    std::string Name = std::string(AI->getName()) + ".unsafe";
+    while (!AI->use_empty()) {
+      Use &U = *AI->use_begin();
+      Instruction *User = cast<Instruction>(U.getUser());
+
+      Instruction *InsertBefore;
+      if (auto *PHI = dyn_cast<PHINode>(User))
+        InsertBefore = PHI->getIncomingBlock(U)->getTerminator();
+      else
+        InsertBefore = User;
+
+      IRBuilder<> IRBUser(InsertBefore);
+      Value *Off = IRBUser.CreateGEP(BasePointer, // BasePointer is i8*
+                                     ConstantInt::get(Int32Ty, -StaticOffset));
+      Value *Replacement = IRBUser.CreateBitCast(Off, AI->getType(), Name);
+
+      if (auto *PHI = dyn_cast<PHINode>(User)) {
+        // PHI nodes may have multiple incoming edges from the same BB (why??),
+        // all must be updated at once with the same incoming value.
+        auto *BB = PHI->getIncomingBlock(U);
+        for (unsigned I = 0; I < PHI->getNumIncomingValues(); ++I)
+          if (PHI->getIncomingBlock(I) == BB)
+            PHI->setIncomingValue(I, Replacement);
+      } else {
+        U.set(Replacement);
+      }
+    }
+
     AI->eraseFromParent();
   }
 

diff --git a/test/Transforms/SafeStack/array-aligned.ll b/test/Transforms/SafeStack/array-aligned.ll
index 4676903ec77248b2547000c5209b2580327b12cf..26558e4fa812e295e537fd09c219f5ec4e37a115 100644 (file)
--- a/test/Transforms/SafeStack/array-aligned.ll
+++ b/test/Transforms/SafeStack/array-aligned.ll
@@ -13,16 +13,15 @@ entry:
 
   ; CHECK: store i8* %[[USST]], i8** @__safestack_unsafe_stack_ptr
 
-  ; CHECK: %[[AADDR:.*]] = alloca i8*, align 8
   %a.addr = alloca i8*, align 8
-
-  ; CHECK: %[[BUFPTR:.*]] = getelementptr i8, i8* %[[USP]], i32 -16
-  ; CHECK: %[[BUFPTR2:.*]] = bitcast i8* %[[BUFPTR]] to [16 x i8]*
   %buf = alloca [16 x i8], align 16
 
+  ; CHECK: %[[AADDR:.*]] = alloca i8*, align 8
   ; CHECK: store i8* {{.*}}, i8** %[[AADDR]], align 8
   store i8* %a, i8** %a.addr, align 8
 
+  ; CHECK: %[[BUFPTR:.*]] = getelementptr i8, i8* %[[USP]], i32 -16
+  ; CHECK: %[[BUFPTR2:.*]] = bitcast i8* %[[BUFPTR]] to [16 x i8]*
   ; CHECK: %[[GEP:.*]] = getelementptr inbounds [16 x i8], [16 x i8]* %[[BUFPTR2]], i32 0, i32 0
   %gep = getelementptr inbounds [16 x i8], [16 x i8]* %buf, i32 0, i32 0
 

diff --git a/test/Transforms/SafeStack/array.ll b/test/Transforms/SafeStack/array.ll
index 564213e6d58fa3d02cce7105ff17ef645e95aafd..7dcf7fa50d94957e080473a5174050b0099d4932 100644 (file)
--- a/test/Transforms/SafeStack/array.ll
+++ b/test/Transforms/SafeStack/array.ll
@@ -17,16 +17,15 @@ entry:
 
   ; CHECK: store i8* %[[USST]], i8** @__safestack_unsafe_stack_ptr
 
-  ; CHECK: %[[AADDR:.*]] = alloca i8*, align 8
   %a.addr = alloca i8*, align 8
-
-  ; CHECK: %[[BUFPTR:.*]] = getelementptr i8, i8* %[[USP]], i32 -4
-  ; CHECK: %[[BUFPTR2:.*]] = bitcast i8* %[[BUFPTR]] to [4 x i8]*
   %buf = alloca [4 x i8], align 1
 
+  ; CHECK: %[[AADDR:.*]] = alloca i8*, align 8
   ; CHECK: store i8* {{.*}}, i8** %[[AADDR]], align 8
   store i8* %a, i8** %a.addr, align 8
 
+  ; CHECK: %[[BUFPTR:.*]] = getelementptr i8, i8* %[[USP]], i32 -4
+  ; CHECK: %[[BUFPTR2:.*]] = bitcast i8* %[[BUFPTR]] to [4 x i8]*
   ; CHECK: %[[GEP:.*]] = getelementptr inbounds [4 x i8], [4 x i8]* %[[BUFPTR2]], i32 0, i32 0
   %gep = getelementptr inbounds [4 x i8], [4 x i8]* %buf, i32 0, i32 0
 

diff --git a/test/Transforms/SafeStack/phi.ll b/test/Transforms/SafeStack/phi.ll
new file mode 100644 (file)
index 0000000..3ee56aa
--- /dev/null
+++ b/test/Transforms/SafeStack/phi.ll
@@ -0,0 +1,35 @@
+; RUN: opt -safe-stack -S -mtriple=i386-pc-linux-gnu < %s -o - | FileCheck %s
+; RUN: opt -safe-stack -S -mtriple=x86_64-pc-linux-gnu < %s -o - | FileCheck %s
+
+define void @f(i1 %d1, i1 %d2) safestack {
+entry:
+; CHECK-LABEL: define void @f(
+; CHECK:         %[[USP:.*]] = load i8*, i8** @__safestack_unsafe_stack_ptr
+; CHECK-NEXT:    getelementptr i8, i8* %[[USP]], i32 -16
+; CHECK:         br i1 %d1, label %[[BB0:.*]], label %[[BB1:.*]]
+  %a = alloca i32, align 8
+  %b = alloca i32, align 8
+  br i1 %d1, label %bb0, label %bb1
+
+bb0:
+; CHECK: [[BB0]]:
+; CHECK: %[[Ai8:.*]] = getelementptr i8, i8* %unsafe_stack_ptr, i32
+; CHECK: %[[AUNSAFE:.*]] = bitcast i8* %[[Ai8]] to i32*
+; CHECK: br i1
+  br i1 %d2, label %bb2, label %bb2
+
+bb1:
+; CHECK: [[BB1]]:
+; CHECK: %[[Bi8:.*]] = getelementptr i8, i8* %unsafe_stack_ptr, i32
+; CHECK: %[[BUNSAFE:.*]] = bitcast i8* %[[Bi8]] to i32*
+; CHECK: br label
+  br label %bb2
+
+bb2:
+; CHECK: phi i32* [ %[[AUNSAFE]], %[[BB0]] ], [ %[[AUNSAFE]], %[[BB0]] ], [ %[[BUNSAFE]], %[[BB1]] ]
+  %c = phi i32* [ %a, %bb0 ], [ %a, %bb0 ], [ %b, %bb1 ]
+  call void @capture(i32* %c)
+  ret void
+}
+
+declare void @capture(i32*)

diff --git a/test/Transforms/SafeStack/sink-to-use.ll b/test/Transforms/SafeStack/sink-to-use.ll
new file mode 100644 (file)
index 0000000..e208ce1
--- /dev/null
+++ b/test/Transforms/SafeStack/sink-to-use.ll
@@ -0,0 +1,22 @@
+; Test that unsafe alloca address calculation is done immediately before each use.
+; RUN: opt -safe-stack -S -mtriple=x86_64-pc-linux-gnu < %s -o - | FileCheck %s
+; RUN: opt -safe-stack -S -mtriple=i386-pc-linux-gnu < %s -o - | FileCheck %s
+
+define void @f() safestack {
+entry:
+  %x0 = alloca i32, align 4
+  %x1 = alloca i32, align 4
+
+; CHECK: %[[A:.*]] = getelementptr i8, i8* %{{.*}}, i32 -4
+; CHECK: %[[X0:.*]] = bitcast i8* %[[A]] to i32*
+; CHECK: call void @use(i32* %[[X0]])
+  call void @use(i32* %x0)
+
+; CHECK: %[[B:.*]] = getelementptr i8, i8* %{{.*}}, i32 -8
+; CHECK: %[[X1:.*]] = bitcast i8* %[[B]] to i32*
+; CHECK: call void @use(i32* %[[X1]])
+  call void @use(i32* %x1)
+  ret void
+}
+
+declare void @use(i32*)

diff --git a/test/Transforms/SafeStack/struct.ll b/test/Transforms/SafeStack/struct.ll
index 12a0085a2cc3437e5ddd6e9950788c228b6ea289..b64803d160c669693403b1454210046f72f24479 100644 (file)
--- a/test/Transforms/SafeStack/struct.ll
+++ b/test/Transforms/SafeStack/struct.ll
@@ -15,16 +15,15 @@ entry:
 
   ; CHECK: store i8* %[[USST]], i8** @__safestack_unsafe_stack_ptr
 
-  ; CHECK: %[[AADDR:.*]] = alloca i8*, align 8
   %a.addr = alloca i8*, align 8
-
-  ; CHECK: %[[BUFPTR:.*]] = getelementptr i8, i8* %[[USP]], i32 -16
-  ; CHECK: %[[BUFPTR2:.*]] = bitcast i8* %[[BUFPTR]] to %struct.foo*
   %buf = alloca %struct.foo, align 1
 
+  ; CHECK: %[[AADDR:.*]] = alloca i8*, align 8
   ; CHECK: store i8* {{.*}}, i8** %[[AADDR]], align 8
   store i8* %a, i8** %a.addr, align 8
 
+  ; CHECK: %[[BUFPTR:.*]] = getelementptr i8, i8* %[[USP]], i32 -16
+  ; CHECK: %[[BUFPTR2:.*]] = bitcast i8* %[[BUFPTR]] to %struct.foo*
   ; CHECK: %[[GEP:.*]] = getelementptr inbounds %struct.foo, %struct.foo* %[[BUFPTR2]], i32 0, i32 0, i32 0
   %gep = getelementptr inbounds %struct.foo, %struct.foo* %buf, i32 0, i32 0, i32 0