[PATCH] D58362: [PEI] Don't re-allocate a pre-allocated stack protector slot

[Francis Visoiu Mistrih via Phabricator via llvm-commits]

thegameg created this revision.
thegameg added reviewers: ab, t.p.northover, arsenm, aemerson, eli.friedman, paquette.
Herald added subscribers: jdoerfert, hiraditya, wdng.
Herald added a project: LLVM.
thegameg added a comment.

Still working on a clean test case.


The LocalStackSlotPass pre-allocates a stack protector and makes sure that it comes before the local variables on the stack.

We need to make sure that later during PEI we don't re-allocate a new stack protector slot. If that happens, the new stack protector slot will end up being **after** the local variables that it should be protecting.

Therefore, we would have two slots assigned for two different stack protectors, one at the top of the stack, and one at the bottom. Since PEI will overwrite the assigned slot for the stack protector, the load that is used to compare the value of the stack protector will use the slot assigned by PEI, which is wrong.

For this, we need to check if the object is pre-allocated, and re-use that pre-allocated slot.


https://reviews.llvm.org/D58362

Files:
  llvm/lib/CodeGen/LocalStackSlotAllocation.cpp
  llvm/lib/CodeGen/PrologEpilogInserter.cpp


Index: llvm/lib/CodeGen/PrologEpilogInserter.cpp
===================================================================
--- llvm/lib/CodeGen/PrologEpilogInserter.cpp
+++ llvm/lib/CodeGen/PrologEpilogInserter.cpp
@@ -893,13 +893,18 @@
   // Make sure that the stack protector comes before the local variables on the
   // stack.
   SmallSet<int, 16> ProtectedObjs;
-  if (MFI.getStackProtectorIndex() >= 0) {
+  int StackProtectorFI = MFI.getStackProtectorIndex();
+  // If we have a stack protector index, we need to make sure that
+  // LocalStackSlotPass didn't already allocate a slot for it. If it did, we
+  // need to use that one.
+  if (StackProtectorFI >= 0 && !(MFI.isObjectPreAllocated(StackProtectorFI) &&
+                                 MFI.getUseLocalStackAllocationBlock())) {
     StackObjSet LargeArrayObjs;
     StackObjSet SmallArrayObjs;
     StackObjSet AddrOfObjs;
 
-    AdjustStackOffset(MFI, MFI.getStackProtectorIndex(), StackGrowsDown,
-                      Offset, MaxAlign, Skew);
+    AdjustStackOffset(MFI, StackProtectorFI, StackGrowsDown, Offset, MaxAlign,
+                      Skew);
 
     // Assign large stack objects first.
     for (unsigned i = 0, e = MFI.getObjectIndexEnd(); i != e; ++i) {
@@ -912,8 +917,7 @@
         continue;
       if (MFI.isDeadObjectIndex(i))
         continue;
-      if (MFI.getStackProtectorIndex() == (int)i ||
-          EHRegNodeFrameIndex == (int)i)
+      if (StackProtectorFI == (int)i || EHRegNodeFrameIndex == (int)i)
         continue;
 
       switch (MFI.getObjectSSPLayout(i)) {
Index: llvm/lib/CodeGen/LocalStackSlotAllocation.cpp
===================================================================
--- llvm/lib/CodeGen/LocalStackSlotAllocation.cpp
+++ llvm/lib/CodeGen/LocalStackSlotAllocation.cpp
@@ -199,19 +199,28 @@
   // Make sure that the stack protector comes before the local variables on the
   // stack.
   SmallSet<int, 16> ProtectedObjs;
+  int StackProtectorFI = MFI.getStackProtectorIndex();
   if (MFI.getStackProtectorIndex() >= 0) {
+    // We need to make sure we didn't pre-allocate the stack protector when
+    // doing this.
+    // If we already have a stack protector, this will re-assign it to a slot
+    // that is **not** covering the protected objects.
+    if (MFI.getUseLocalStackAllocationBlock() &&
+        MFI.isObjectPreAllocated(StackProtectorFI))
+      llvm_unreachable(
+          "Stack protector pre-allocated in LocalStackSlotAllocation");
+
     StackObjSet LargeArrayObjs;
     StackObjSet SmallArrayObjs;
     StackObjSet AddrOfObjs;
 
-    AdjustStackOffset(MFI, MFI.getStackProtectorIndex(), Offset,
-                      StackGrowsDown, MaxAlign);
+    AdjustStackOffset(MFI, StackProtectorFI, Offset, StackGrowsDown, MaxAlign);
 
     // Assign large stack objects first.
     for (unsigned i = 0, e = MFI.getObjectIndexEnd(); i != e; ++i) {
       if (MFI.isDeadObjectIndex(i))
         continue;
-      if (MFI.getStackProtectorIndex() == (int)i)
+      if (StackProtectorFI == (int)i)
         continue;
 
       switch (MFI.getObjectSSPLayout(i)) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D58362.187285.patch
Type: text/x-patch
Size: 3088 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190219/d49a34c8/attachment.bin>