[PATCH] D41300: [ARM] Fix PR35379 - incorrect unwind information when compiling with -Oz

[Momchil Velikov via Phabricator via llvm-commits]

chill created this revision.
chill added reviewers: rengolin, efriedma, john.brawn.
Herald added subscribers: kristof.beyls, javed.absar, aemerson.

Compiling with `-Oz` enable an optimisation, where the compiler tries to fold an update to `SP` by adding extra registers to existing push/pop instructions. This doesn't interact correctly with `ARMAsmPrinter::EmitUnwindingInstruction` - the generated unwind information causes the extra registers to be restored when an exception is propagated through the function, but the corresponding stack slots can be already clobbered.

When adding these registers to a push, the register operands are marked as "undef". This patch modifies `ARMAsmPrinter::EmitUnwindingInstruction`  to examine the register operand flags and not mention in the `.save` the registers, which
have this flag set.


Repository:
  rL LLVM

https://reviews.llvm.org/D41300

Files:
  lib/Target/ARM/ARMAsmPrinter.cpp
  lib/Target/ARM/ARMBaseInstrInfo.cpp
  test/CodeGen/ARM/PR35379.ll


Index: test/CodeGen/ARM/PR35379.ll
===================================================================
--- /dev/null
+++ test/CodeGen/ARM/PR35379.ll
@@ -0,0 +1,24 @@
+; RUN: llc -mtriple=armv7a-eabi < %s | FileCheck %s --check-prefix=CHECK-ARM
+; RUN: llc -mtriple=armv6m-eabi < %s | FileCheck %s --check-prefix=CHECK-THM
+
+; Function Attrs: minsize optsize
+define void @work() local_unnamed_addr #0 {
+entry:
+  %i = alloca i32, align 4
+  %0 = bitcast i32* %i to i8*
+  store i32 1, i32* %i, align 4
+  call void @impl(i32* nonnull %i)
+  ret void
+}
+; CHECK-ARM:      .save {r11, lr}
+; CHECK-ARM-NEXT: push {r9, r10, r11, lr}
+; CHECK-ARM:      pop {r2, r3, r11, pc}
+
+; CHECK-THM:      .save {r7, lr}
+; CHECK-THM-NEXT: push {r5, r6, r7, lr}
+; CHECK-THM:      pop {r2, r3, r7, pc}
+
+; Function Attrs: minsize optsize
+declare void @impl(i32*) local_unnamed_addr #0
+
+attributes #0 = { minsize optsize }
Index: lib/Target/ARM/ARMBaseInstrInfo.cpp
===================================================================
--- lib/Target/ARM/ARMBaseInstrInfo.cpp
+++ lib/Target/ARM/ARMBaseInstrInfo.cpp
@@ -2277,9 +2277,9 @@
        --CurRegEnc) {
     unsigned CurReg = RegClass->getRegister(CurRegEnc);
     if (!IsPop) {
-      // Pushing any register is completely harmless, mark the
-      // register involved as undef since we don't care about it in
-      // the slightest.
+      // Pushing any register is completely harmless, mark the register involved
+      // as undef since we don't care about its value and must not restore it
+      // during stack unwinding.
       RegList.push_back(MachineOperand::CreateReg(CurReg, false, false,
                                                   false, false, true));
       --RegsNeeded;
Index: lib/Target/ARM/ARMAsmPrinter.cpp
===================================================================
--- lib/Target/ARM/ARMAsmPrinter.cpp
+++ lib/Target/ARM/ARMAsmPrinter.cpp
@@ -1111,6 +1111,12 @@
         // temporary to workaround PR11902.
         if (MO.isImplicit())
           continue;
+        // Registers, pushed as a part of folding an SP update into the push
+        // instruction are marked as undef and should not be restored when
+        // unwinding, because the function can modify the corresponding stack
+        // slots.
+        if (MO.isUndef())
+          continue;
         RegList.push_back(MO.getReg());
       }
       break;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D41300.127150.patch
Type: text/x-patch
Size: 2412 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171215/2e8a349e/attachment.bin>