From 892ae2e2b65004c22e3cfbfe4e1c44717f4214d0 Mon Sep 17 00:00:00 2001
From: Reid Kleckner <rnk@google.com>
Date: Sat, 27 Feb 2016 00:53:54 +0000
Subject: [PATCH] [InstCombine] Be more conservative about removing
 stackrestore

We ended up removing a save/restore pair around an inalloca call,
leading to a miscompile in Chromium.

llvm-svn: 262095
---
 .../Transforms/InstCombine/InstCombineCalls.cpp    |  8 ++-
 .../Transforms/InstCombine/stacksaverestore.ll     | 58 +++++++++++++++++++++-
 2 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index fbbe29c..bd58e81 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -1821,7 +1821,13 @@ Instruction *InstCombiner::visitCallInst(CallInst &CI) {
           // If there is a stackrestore below this one, remove this one.
           if (II->getIntrinsicID() == Intrinsic::stackrestore)
             return eraseInstFromFunction(CI);
-          // Otherwise, ignore the intrinsic.
+
+          // Bail if we cross over an intrinsic with side effects, such as
+          // llvm.stacksave, llvm.read_register, or llvm.setjmp.
+          if (II->mayHaveSideEffects()) {
+            CannotRemove = true;
+            break;
+          }
         } else {
           // If we found a non-intrinsic call, we can't remove the stack
           // restore.
diff --git a/llvm/test/Transforms/InstCombine/stacksaverestore.ll b/llvm/test/Transforms/InstCombine/stacksaverestore.ll
index de400e9..9eb0efb 100644
--- a/llvm/test/Transforms/InstCombine/stacksaverestore.ll
+++ b/llvm/test/Transforms/InstCombine/stacksaverestore.ll
@@ -1,4 +1,6 @@
-; RUN: opt < %s -instcombine -S | grep "call.*stackrestore" | count 1
+; RUN: opt < %s -instcombine -S | FileCheck %s
+
+@glob = global i32 0
 
 declare i8* @llvm.stacksave()
 declare void @llvm.stackrestore(i8*)
@@ -11,11 +13,19 @@ define i32* @test1(i32 %P) {
 	ret i32* %A
 }
 
+; CHECK-LABEL: define i32* @test1(
+; CHECK-NOT: call void @llvm.stackrestore
+; CHECK: ret i32*
+
 define void @test2(i8* %X) {
 	call void @llvm.stackrestore( i8* %X )  ;; no allocas before return.
 	ret void
 }
 
+; CHECK-LABEL: define void @test2(
+; CHECK-NOT: call void @llvm.stackrestore
+; CHECK: ret void
+
 define void @foo(i32 %size) nounwind  {
 entry:
 	%tmp118124 = icmp sgt i32 %size, 0		; <i1> [#uses=1]
@@ -52,5 +62,51 @@ return:		; preds = %bb, %entry
 	ret void
 }
 
+; CHECK-LABEL: define void @foo(
+; CHECK: %tmp = call i8* @llvm.stacksave()
+; CHECK: alloca i8
+; CHECK-NOT: stacksave
+; CHECK: call void @bar(
+; CHECK-NEXT: call void @llvm.stackrestore(i8* %tmp)
+; CHECK: ret void
+
 declare void @bar(i32, i8*, i8*, i8*, i8*, i32)
 
+declare void @inalloca_callee(i32* inalloca)
+
+define void @test3(i32 %c) {
+entry:
+  br label %loop
+
+loop:
+  %i = phi i32 [0, %entry], [%i1, %loop]
+  %save1 = call i8* @llvm.stacksave()
+  %argmem = alloca inalloca i32
+  store i32 0, i32* %argmem
+  call void @inalloca_callee(i32* inalloca %argmem)
+
+  ; This restore cannot be deleted, the restore below does not make it dead.
+  call void @llvm.stackrestore(i8* %save1)
+
+  ; FIXME: We should be able to remove this save/restore pair, but we don't.
+  %save2 = call i8* @llvm.stacksave()
+  store i32 0, i32* @glob
+  call void @llvm.stackrestore(i8* %save2)
+  %i1 = add i32 1, %i
+  %done = icmp eq i32 %i1, %c
+  br i1 %done, label %loop, label %return
+
+return:
+  ret void
+}
+
+; CHECK-LABEL: define void @test3(
+; CHECK: loop:
+; CHECK: %i = phi i32 [ 0, %entry ], [ %i1, %loop ]
+; CHECK: %save1 = call i8* @llvm.stacksave()
+; CHECK: %argmem = alloca inalloca i32
+; CHECK: store i32 0, i32* %argmem
+; CHECK: call void @inalloca_callee(i32* inalloca {{.*}} %argmem)
+; CHECK: call void @llvm.stackrestore(i8* %save1)
+; CHECK: br i1 %done, label %loop, label %return
+; CHECK: ret void
-- 
2.7.4