From 0cd19b93017fcaba737b15ea4da39c460feb5670 Mon Sep 17 00:00:00 2001
From: Nadav Rotem <nrotem@apple.com>
Date: Thu, 13 Sep 2012 12:38:37 +0000
Subject: [PATCH] Stack Coloring: We have code that checks that all of the uses
 of allocas are within the lifetime zone. Sometime legitimate usages of
 allocas are hoisted outside of the lifetime zone. For example, GEPS may
 calculate the address of a member of an allocated struct. This commit makes
 sure that we only check (abort regions or assert) for instructions that read
 and write memory using stack frames directly. Notice that by allowing
 legitimate usages outside the lifetime zone we also stop checking for
 instructions which use derivatives of allocas. We will catch less bugs in
 user code and in the compiler itself.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@163791 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/CodeGen/StackColoring.cpp     | 22 +++++++++++++++++-----
 test/CodeGen/X86/StackColoring.ll | 24 ++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/lib/CodeGen/StackColoring.cpp b/lib/CodeGen/StackColoring.cpp
index a14d730025f..1a7801abd8a 100644
--- a/lib/CodeGen/StackColoring.cpp
+++ b/lib/CodeGen/StackColoring.cpp
@@ -73,7 +73,6 @@ STATISTIC(StackSlotMerged, "Number of stack slot merged.");
 STATISTIC(EscapedAllocas,
           "Number of allocas that escaped the lifetime region");
 
-
 //===----------------------------------------------------------------------===//
 //                           StackColoring Pass
 //===----------------------------------------------------------------------===//
@@ -259,8 +258,8 @@ unsigned StackColoring::collectMarkers(unsigned NumSlot) {
 
       const Value *Allocation = MFI->getObjectAllocation(Slot);
       if (Allocation) {
-        DEBUG(dbgs()<<"Found lifetime marker for allocation: "<<
-              Allocation->getName()<<"\n");
+        DEBUG(dbgs()<<"Found a lifetime marker for slot #"<<Slot<<
+              " with allocation: "<< Allocation->getName()<<"\n");
       }
 
       if (IsStart) {
@@ -538,8 +537,12 @@ void StackColoring::remapInstructions(DenseMap<int, int> &SlotRemap) {
         // inside the expected live range. If the instruction is not inside
         // the calculated range then it means that the alloca usage moved
         // outside of the lifetime markers.
+        // NOTE: Alloca address calculations which happen outside the lifetime
+        // zone are are okay, despite the fact that we don't have a good way
+        // for validating all of the usages of the calculation.
 #ifndef NDEBUG
-        if (!I->isDebugValue()) {
+        bool TouchesMemory = I->mayLoad() || I->mayStore();
+        if (!I->isDebugValue() && TouchesMemory) {
           SlotIndex Index = Indexes->getInstructionIndex(I);
           LiveInterval *Interval = Intervals[FromSlot];
           assert(Interval->find(Index) != Interval->end() &&
@@ -569,6 +572,15 @@ void StackColoring::removeInvalidSlotRanges() {
           I->getOpcode() == TargetOpcode::LIFETIME_END || I->isDebugValue())
         continue;
 
+      // Some intervals are suspicious! In some cases we find address
+      // calculations outside of the lifetime zone, but not actual memory
+      // read or write. Memory accesses outside of the lifetime zone are a clear
+      // violation, but address calculations are okay. This can happen when
+      // GEPs are hoisted outside of the lifetime zone.
+      // So, in here we only check instrucitons which can read or write memory.
+      if (!I->mayLoad() && !I->mayStore())
+        continue;
+
       // Check all of the machine operands.
       for (unsigned i = 0 ; i <  I->getNumOperands(); ++i) {
         MachineOperand &MO = I->getOperand(i);
@@ -652,7 +664,7 @@ bool StackColoring::runOnMachineFunction(MachineFunction &Func) {
   DEBUG(dbgs()<<"Total Stack size: "<<TotalSize<<" bytes\n\n");
 
   // Don't continue because there are not enough lifetime markers, or the
-  // stack or too small, or we are told not to optimize the slots.
+  // stack is too small, or we are told not to optimize the slots.
   if (NumMarkers < 2 || TotalSize < 16 || DisableColoring) {
     DEBUG(dbgs()<<"Will not try to merge slots.\n");
     return removeAllMarkers();
diff --git a/test/CodeGen/X86/StackColoring.ll b/test/CodeGen/X86/StackColoring.ll
index a83a4ebbe2e..f8ae74f292d 100644
--- a/test/CodeGen/X86/StackColoring.ll
+++ b/test/CodeGen/X86/StackColoring.ll
@@ -376,6 +376,30 @@ block2:
 }
 
 
+; Check that we don't assert and crash even when there are usages
+; of allocas which do not read or write outside the declared lifetime regions.
+;YESCOLOR: shady_range
+;NOCOLOR:  shady_range
+
+%struct.Klass = type { i32, i32 }
+
+define i32 @shady_range(i32 %argc, i8** nocapture %argv) uwtable {
+  %a.i = alloca [4 x %struct.Klass], align 16
+  %b.i = alloca [4 x %struct.Klass], align 16
+  %a8 = bitcast [4 x %struct.Klass]* %a.i to i8*
+  %b8 = bitcast [4 x %struct.Klass]* %b.i to i8*
+  ; I am used outside the lifetime zone below:
+  %z2 = getelementptr inbounds [4 x %struct.Klass]* %a.i, i64 0, i64 0, i32 0
+  call void @llvm.lifetime.start(i64 -1, i8* %a8)
+  call void @llvm.lifetime.start(i64 -1, i8* %b8)
+  %z3 = load i32* %z2, align 16
+  %r = call i32 @foo(i32 %z3, i8* %a8)
+  %r2 = call i32 @foo(i32 %z3, i8* %b8)
+  call void @llvm.lifetime.end(i64 -1, i8* %a8)
+  call void @llvm.lifetime.end(i64 -1, i8* %b8)
+  ret i32 9
+}
+
 declare void @bar([100 x i32]* , [100 x i32]*) nounwind
 
 declare void @llvm.lifetime.start(i64, i8* nocapture) nounwind