[PATCH] D64759: [CodeGen] Don't resolve the stack protector frame accesses until PEI

[Francis Visoiu Mistrih via Phabricator via llvm-commits]

thegameg created this revision.
thegameg added reviewers: john.brawn, aadg.
Herald added subscribers: arphaman, hiraditya, javed.absar.
Herald added a project: LLVM.

Currently, stack protector loads and stores are resolved during LocalStackSlotAllocation (if the pass needs to run). When this is the case, the base register assigned to the frame access is going to be one of the vregs created during LocalStackSlotAllocation. This means that we are keeping a pointer to the stack protector slot, and we're using this pointer to load and store to it.

In case register pressure goes up, we may end up spilling this pointer to the stack, which can be a security concern.

Instead, leave it to PEI to resolve the frame accesses. In order to do that, we make all stack protector accesses go through frame index operands, then PEI will resolve this using an offset from sp/fp/bp.


https://reviews.llvm.org/D64759

Files:
  llvm/lib/CodeGen/LocalStackSlotAllocation.cpp
  llvm/test/CodeGen/Thumb/stack_guard_remat.ll
  test/CodeGen/AArch64/stack-protector-frame-index.mir


Index: test/CodeGen/AArch64/stack-protector-frame-index.mir
===================================================================
--- /dev/null
+++ test/CodeGen/AArch64/stack-protector-frame-index.mir
@@ -0,0 +1,27 @@
+# RUN: llc -mtriple=arm64-apple-ios -run-pass=localstackalloc -o - %s | FileCheck %s
+# Check that the pass doesn't re-write stack protector accesses using a virtual
+# base register.
+
+# RUN: llc -mtriple=arm64-apple-ios -start-before=localstackalloc -stop-after=prologepilog -o - %s | FileCheck %s --check-prefix=CHECK-PEI
+# Check that PEI handles the re-write and uses x29 as a base reg.
+
+---
+name:            main
+tracksRegLiveness: true
+frameInfo:
+  stackProtector:  '%stack.0'
+stack:
+  - { id: 0, size: 8, alignment: 8, stack-id: 0 }
+body:             |
+  bb.0:
+
+    %0:gpr64common = IMPLICIT_DEF
+    STRXui killed %0, %stack.0, 0 :: (volatile store 8 into %stack.0)
+    %1:gpr64 = LDRXui %stack.0, 0 :: (volatile load 8 from %stack.0)
+    ; CHECK:      STRXui killed %0, %stack.0, 0 :: (volatile store 8 into %stack.0)
+    ; CHECK-NEXT: %1:gpr64 = LDRXui %stack.0, 0 :: (volatile load 8 from %stack.0)
+    ; CHECK-PEI:      STRXui undef renamable $[[REG:x[0-9]+]], $sp, 1 :: (volatile store 8 into %stack.0)
+    ; CHECK-PEI-NEXT: dead renamable $[[REG]] = LDRXui $sp, 1 :: (volatile load 8 from %stack.0)
+    RET_ReallyLR implicit undef $w0
+
+...
Index: llvm/test/CodeGen/Thumb/stack_guard_remat.ll
===================================================================
--- llvm/test/CodeGen/Thumb/stack_guard_remat.ll
+++ llvm/test/CodeGen/Thumb/stack_guard_remat.ll
@@ -3,6 +3,7 @@
 ; RUN: llc < %s -mtriple=thumb-apple-darwin -relocation-model=dynamic-no-pic -no-integrated-as | FileCheck %s  -check-prefix=NO-PIC -check-prefix=DYNAMIC-NO-PIC
 
 ;PIC:   foo2
+;PIC:   ldr {{r[0-9]+}}, {{LCPI[0-9_]+}}
 ;PIC:   ldr [[R0:r[0-9]+]], [[LABEL0:LCPI[0-9_]+]]
 ;PIC: [[LABEL1:LPC[0-9_]+]]:
 ;PIC:   add [[R0]], pc
@@ -13,6 +14,7 @@
 ;PIC-NEXT:   .long L___stack_chk_guard$non_lazy_ptr-([[LABEL1]]+4)
 
 ;NO-PIC:   foo2
+;NO-PIC:   ldr {{r[0-9]+}}, {{LCPI[0-9_]+}}
 ;NO-PIC:   ldr [[R0:r[0-9]+]], [[LABEL0:LCPI[0-9_]+]]
 ;NO-PIC-NOT: LPC
 ;NO-PIC:   ldr {{r[0-9]+}}, {{\[}}[[R0]]{{\]}}
Index: llvm/lib/CodeGen/LocalStackSlotAllocation.cpp
===================================================================
--- llvm/lib/CodeGen/LocalStackSlotAllocation.cpp
+++ llvm/lib/CodeGen/LocalStackSlotAllocation.cpp
@@ -350,6 +350,14 @@
     assert(MFI.isObjectPreAllocated(FrameIdx) &&
            "Only pre-allocated locals expected!");
 
+    // We need to keep the references to the stack protector slot through frame
+    // index operands so that it gets resolved by PEI rather than this pass.
+    // This avoids accesses to the stack protector though virtual base
+    // registers, and forces PEI to address it using fp/sp/bp.
+    if (MFI.hasStackProtectorIndex() &&
+        FrameIdx == MFI.getStackProtectorIndex())
+      continue;
+
     LLVM_DEBUG(dbgs() << "Considering: " << MI);
 
     unsigned idx = 0;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D64759.209899.patch
Type: text/x-patch
Size: 3051 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190715/41701ea0/attachment.bin>