[llvm] r264455 - CodeGen: Don't iterate over operands after we've erased an MI

[Justin Bogner via llvm-commits]

Author: bogner
Date: Fri Mar 25 15:03:28 2016
New Revision: 264455

URL: http://llvm.org/viewvc/llvm-project?rev=264455&view=rev
Log:
CodeGen: Don't iterate over operands after we've erased an MI

This fixes a use-after-free introduced 3 years ago, in r182872 ;)

The code more or less worked because the memory that CopyMI was
pointing to happened to still be valid, but lots of tests would crash
if you ran under ASAN with the recycling allocator changes from
llvm.org/PR26808

Modified:
    llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp

Modified: llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp?rev=264455&r1=264454&r2=264455&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp (original)
+++ llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp Fri Mar 25 15:03:28 2016
@@ -979,6 +979,23 @@ bool RegisterCoalescer::reMaterializeTri
     }
   }
 
+  // CopyMI may have implicit operands, save them so that we can transfer them
+  // over to the newly materialized instruction after CopyMI is removed.
+  SmallVector<MachineOperand, 4> ImplicitOps;
+  ImplicitOps.reserve(CopyMI->getNumOperands() -
+                      CopyMI->getDesc().getNumOperands());
+  for (unsigned I = CopyMI->getDesc().getNumOperands(),
+                E = CopyMI->getNumOperands();
+       I != E; ++I) {
+    MachineOperand &MO = CopyMI->getOperand(I);
+    if (MO.isReg()) {
+      assert(MO.isImplicit() && "No explicit operands after implict operands.");
+      // Discard VReg implicit defs.
+      if (TargetRegisterInfo::isPhysicalRegister(MO.getReg()))
+        ImplicitOps.push_back(MO);
+    }
+  }
+
   LIS->ReplaceMachineInstrInMaps(*CopyMI, *NewMI);
   CopyMI->eraseFromParent();
   ErasedInstrs.insert(CopyMI);
@@ -1082,19 +1099,9 @@ bool RegisterCoalescer::reMaterializeTri
   if (NewMI->getOperand(0).getSubReg())
     NewMI->getOperand(0).setIsUndef();
 
-  // CopyMI may have implicit operands, transfer them over to the newly
-  // rematerialized instruction. And update implicit def interval valnos.
-  for (unsigned i = CopyMI->getDesc().getNumOperands(),
-         e = CopyMI->getNumOperands(); i != e; ++i) {
-    MachineOperand &MO = CopyMI->getOperand(i);
-    if (MO.isReg()) {
-      assert(MO.isImplicit() && "No explicit operands after implict operands.");
-      // Discard VReg implicit defs.
-      if (TargetRegisterInfo::isPhysicalRegister(MO.getReg())) {
-        NewMI->addOperand(MO);
-      }
-    }
-  }
+  // Transfer over implicit operands to the rematerialized instruction.
+  for (MachineOperand &MO : ImplicitOps)
+    NewMI->addOperand(MO);
 
   SlotIndex NewMIIdx = LIS->getInstructionIndex(*NewMI);
   for (unsigned i = 0, e = NewMIImplDefs.size(); i != e; ++i) {