[PATCH] D28513: Enable pass pipelines containing LocalStackSlotPass but not StackProtectorPass.

[Kristof Beyls via Phabricator via llvm-commits]

kristof.beyls created this revision.
kristof.beyls added reviewers: silviu.baranga, qcolombet, chandlerc.
kristof.beyls added a subscriber: llvm-commits.

This makes it possible to write llc -start-after, specifying a pass that
runs after StackProtector. Very useful, e.g. to write a regression test
for a machine pass, or to test a MIR-to-assembly partial pass pipeline.

This also fixes PR30324.

I've added the legalmir2asm.mir file to show how I intend to
use MIR-to-assembly testing for GlobalISel development, but I don't intend to commit that
test case as part of this patch. It's just an example of the kind of regression tests
this fix makes possible.


https://reviews.llvm.org/D28513

Files:
  lib/CodeGen/StackProtector.cpp
  test/CodeGen/AArch64/GlobalISel/legalmir2asm.mir


Index: test/CodeGen/AArch64/GlobalISel/legalmir2asm.mir
===================================================================
--- /dev/null
+++ test/CodeGen/AArch64/GlobalISel/legalmir2asm.mir
@@ -0,0 +1,38 @@
+# RUN: llc -O0 -mtriple=aarch64-apple-ios -start-after=instruction-select -verify-machineinstrs -global-isel %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=IOS
+# RUN: llc -O0 -mtriple=aarch64-linux-gnu -start-after=instruction-select -verify-machineinstrs -global-isel %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=LINUX-DEFAULT
+# RUN: llc -O0 -mtriple=aarch64-linux-gnu -relocation-model=pic -start-after=instruction-select -verify-machineinstrs -global-isel %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=LINUX-PIC
+
+# Test the legalized MIR -> assembly translation.
+# As we support more instructions, we need to split this up.
+
+--- |
+  target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
+
+  define void @add_s8_gpr() { ret void }
+...
+
+---
+name:            add_s8_gpr
+legalized:       true
+regBankSelected: true
+selected:        true
+tracksRegLiveness: true
+
+registers:
+  - { id: 0, class: gpr32 }
+  - { id: 1, class: gpr32 }
+  - { id: 2, class: gpr32 }
+
+# CHECK-LABEL: add_s8_gpr:
+# CHECK:    add w0, w0, w1
+# CHECK:    ret
+body:             |
+  bb.0:
+    liveins: %w0, %w1
+
+    %0 = COPY %w0
+    %1 = COPY %w1
+    %2 = ADDWrr %0, %1
+    %w0 = COPY %2
+    RET_ReallyLR implicit %w0
+...
Index: lib/CodeGen/StackProtector.cpp
===================================================================
--- lib/CodeGen/StackProtector.cpp
+++ lib/CodeGen/StackProtector.cpp
@@ -89,18 +89,35 @@
   DominatorTreeWrapperPass *DTWP =
       getAnalysisIfAvailable<DominatorTreeWrapperPass>();
   DT = DTWP ? &DTWP->getDomTree() : nullptr;
-  TLI = TM->getSubtargetImpl(Fn)->getTargetLowering();
   HasPrologue = false;
   HasIRCheck = false;
 
+  // FIXME: There is a strong coupling between this StackProtector pass,
+  // running on LLVM-IR, and the LocalStackSlot MachineFunctionPass. The
+  // LocalStackSlot pass requires the StackProtector pass as an analysis: it
+  // needs quite a bit of information from the StackProtector pass on how the
+  // stack guard was introduced to make sure it doesn't accidentally undo the
+  // protection given by it.  This becomes problematic when a pass pipeline
+  // gets created where the StackProtector pass isn't run, but the
+  // LocalStackSlot pass is, e.g. when testing MIR-to-assembly translation.
+  //
+  // In such a case, the StackProtector pass (at least in the legacy pass
+  // manager) only can get constructed without a TargetMachine, i.e. TM being
+  // null. Therefore, below, checks on whether the stack protector pass needs
+  // to run at all need to be done before trying to access TM: to avoid reading
+  // a null pointer in the case where a pass pipeline is constructed where the
+  // StackProtector pass isn't meant to be run, but the LocalStackSlot pass is.
+
   Attribute Attr = Fn.getFnAttribute("stack-protector-buffer-size");
   if (Attr.isStringAttribute() &&
       Attr.getValueAsString().getAsInteger(10, SSPBufferSize))
     return false; // Invalid integer string
 
   if (!RequiresStackProtector())
     return false;
 
+  TLI = TM->getSubtargetImpl(Fn)->getTargetLowering();
+
   // TODO(etienneb): Functions with funclets are not correctly supported now.
   // Do nothing if this is funclet-based personality.
   if (Fn.hasPersonalityFn()) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D28513.83793.patch
Type: text/x-patch
Size: 3495 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170110/a2544ced/attachment.bin>