[PATCH] D155245: AArch64: don't crash when .cfi_startproc/.cfi_endproc are improperly nested

[Jon Roelofs via Phabricator via llvm-commits]

jroelofs created this revision.
jroelofs added reviewers: ab, t.p.northover, iains, respindola.
Herald added subscribers: hiraditya, kristof.beyls.
Herald added a project: All.
jroelofs requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This showed up when simplifying some large testcase, where the cfi directives became out of sync with the proc's they enclose. I don't see a good way to catch and diagnose this weird case, so instead let's fall back on the pre-r326966 expansion behavior here.

      

This is a partial revert of 06c064824ef29425db785a31fed03821777fbf12

      

rdar://111459507


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155245

Files:
  llvm/lib/Target/AArch64/MCTargetDesc/AArch64MachObjectWriter.cpp
  llvm/test/MC/MachO/AArch64/cfi-bad-nesting.s


Index: llvm/test/MC/MachO/AArch64/cfi-bad-nesting.s
===================================================================
--- /dev/null
+++ llvm/test/MC/MachO/AArch64/cfi-bad-nesting.s
@@ -0,0 +1,33 @@
+; RUN: llvm-mc -triple arm64-apple-darwin10 %s -filetype=obj -o - | llvm-readobj -r --expand-relocs - | FileCheck %s
+
+	.section	__TEXT,locomotive,regular,pure_instructions
+
+	.globl	_locomotive
+	.p2align	2
+_locomotive:
+	.cfi_startproc
+	ret
+
+	.section	__TEXT,__text,regular,pure_instructions
+	.globl	_caboose
+	.p2align	2
+_caboose:
+	ret
+	.cfi_endproc
+
+.subsections_via_symbols
+
+; This is a regression test making sure we don't crash when
+; .cfi_startproc/.cfi_endproc are improperly nested.
+
+; CHECK:      Relocations [
+; CHECK-NEXT:   Section __compact_unwind {
+; CHECK-NEXT:     Relocation {
+; CHECK-NEXT:       Offset: 0x0
+; CHECK-NEXT:       PCRel: 0
+; CHECK-NEXT:       Length: 3
+; CHECK-NEXT:       Type: ARM64_RELOC_UNSIGNED (0)
+; CHECK-NEXT:       Section: locomotive (2)
+; CHECK-NEXT:     }
+; CHECK-NEXT:   }
+; CHECK-NEXT: ]
Index: llvm/lib/Target/AArch64/MCTargetDesc/AArch64MachObjectWriter.cpp
===================================================================
--- llvm/lib/Target/AArch64/MCTargetDesc/AArch64MachObjectWriter.cpp
+++ llvm/lib/Target/AArch64/MCTargetDesc/AArch64MachObjectWriter.cpp
@@ -318,9 +318,21 @@
     }
 
     const MCSymbol *Base = Asm.getAtom(*Symbol);
-    // If the symbol is a variable it can either be in a section and
-    // we have a base or it is absolute and should have been expanded.
-    assert(!Symbol->isVariable() || Base);
+
+    // If the symbol is a variable and we weren't able to get a Base for it
+    // (i.e., it's not in the symbol table associated with a section) resolve
+    // the relocation based its expansion instead.
+    if (Symbol->isVariable() && !Base) {
+      // When the evaluation is an absolute value, just use that directly
+      // to keep things easy.
+      int64_t Res;
+      bool Absolute = Symbol->getVariableValue()->evaluateAsAbsolute(
+              Res, Layout, Writer->getSectionAddressMap());
+      (void)Absolute;
+      assert(Absolute && "expected an absolute symbol");
+      FixedValue = Res;
+      return;
+    }
 
     // Relocations inside debug sections always use local relocations when
     // possible. This seems to be done because the debugger doesn't fully


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D155245.540206.patch
Type: text/x-patch
Size: 2399 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230713/f8f78ee9/attachment.bin>