[MC] Emit Stackmaps before debug info

[zero9178]

Summary

This patch is essentially an alternative to https://reviews.llvm.org/D75836 and was mentioned by @lhames in a comment.

The gist of the issue is that Mach-O has restrictions on which kind of sections are allowed after debug info has been emitted, which is also properly asserted within LLVM. Problem is that stack maps are currently emitted as one of the last sections in each target-specific AsmPrinter so far, which would cause the assertion to trigger. The current approach of special casing for the __LLVM_STACKMAPS section is not viable either, as downstream users can overwrite the stackmap format using plugins, which may want to use different sections.

This patch fixes the issue by emitting the stack map earlier, right before debug info is emitted. The way this is implemented is by taking the choice when to emit the StackMap away from the target AsmPrinter and doing so in the base class. The only disadvantage of this approach is that the StackMaps member is now part of the base class, even for targets that do not support them. This is functionaly not a problem however, as emitting an empty StackMaps is a no-op.

[zero9178]

Fix diff creation

[zero9178]

Don't forget about PowerPC and SystemZ. Boop CI

[zero9178]

Friendly ping

[dantrushin]

I did a quick testing downstream (on X86) and did not encounter any issues. Looks good to me.

[lhames]

The current approach of special casing for the __LLVM_STACKMAPS section is not viable either, as downstream users can overwrite the stackmap format using plugins, which may want to use different sections.

@zero9178 Could you elaborate on this? Was __LLVM_STACKMAPS handling being special cased in assertions in LLVM, or in tools that consumed MachO?

What do you mean by "downstream users can overwrite the stackmap format using plugins"? When do those plugins run, and how would they interfere with the special casing?

I don't mind this approach, but am curious about the design constraints on stack maps and how they're being used at the moment.

[zero9178]

In D132708#3773282, @lhames wrote:

The current approach of special casing for the __LLVM_STACKMAPS section is not viable either, as downstream users can overwrite the stackmap format using plugins, which may want to use different sections.

@zero9178 Could you elaborate on this? Was __LLVM_STACKMAPS handling being special cased in assertions in LLVM, or in tools that consumed MachO?

What do you mean by "downstream users can overwrite the stackmap format using plugins"? When do those plugins run, and how would they interfere with the special casing?

I don't mind this approach, but am curious about the design constraints on stack maps and how they're being used at the moment.

With the special case I was simply referring to the assertion.
These plugins I was referring to are the GCMetaDataPrinter defined here https://github.com/llvm/llvm-project/blob/f049b2c3fcbae739ac965f97fd6855ca1aab77b8/llvm/include/llvm/CodeGen/GCMetadataPrinter.h
The emitStackmaps function that was moved here calls any implementations in the registry to write out the stackmap [0]. The default stackmap is only used if a GCMetaDataPrinter does not emit a stackmap.
Since users of the GCMetaDataPrinter can use the AsmPrinter arbitrarly and can choose not to use the __LLVMSTACKMAP section it would then lead to the assertion being triggered.

[0] https://github.com/llvm/llvm-project/blob/f049b2c3fcbae739ac965f97fd6855ca1aab77b8/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp#L3765

[lhames]

@zero9178 Thanks very much for the explanation! I had no idea we had a customization point for those.

llvm/include/llvm/CodeGen/AsmPrinter.h

 #define LLVM_CODEGEN_ASMPRINTER_H
 
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/CodeGen/AsmPrinterHandler.h"
 #include "llvm/CodeGen/DwarfStringPoolEntry.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
+#include "llvm/CodeGen/StackMaps.h"
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <cstdint>
 #include <memory>
 #include <utility>
 #include <vector>
 
 namespace llvm {

 class MCStreamer;
 class MCSubtargetInfo;
 class MCSymbol;
 class MCTargetOptions;
 class MDNode;
 class Module;
 class PseudoProbeHandler;
 class raw_ostream;
-class StackMaps;
 class StringRef;
 class TargetLoweringObjectFile;
 class TargetMachine;
 class Twine;
 
 namespace remarks {
 class RemarkStreamer;
 }
protected:
   /// For dso_local functions, the current $local alias for the function.
   MCSymbol *CurrentFnBeginLocal = nullptr;
 
   /// A vector of all debug/EH info emitters we should use. This vector
   /// maintains ownership of the emitters.
   std::vector<HandlerInfo> Handlers;
   size_t NumUserHandlers = 0;
 
+  StackMaps SM;
+
 private:
   /// If generated on the fly this own the instance.
   std::unique_ptr<MachineDominatorTree> OwnedMDT;
 
   /// If generated on the fly this own the instance.
   std::unique_ptr<MachineLoopInfo> OwnedMLI;
 
   /// If the target supports dwarf debug info, this pointer is non-null.
public:
   void computeGlobalGOTEquivs(Module &M);
 
   /// Constant expressions using GOT equivalent globals may not be
   /// eligible for PC relative GOT entry conversion, in such cases we need to
   /// emit the proxies we previously omitted in EmitGlobalVariable.
   void emitGlobalGOTEquivs();
 
   /// Emit the stack maps.
-  void emitStackMaps(StackMaps &SM);
+  void emitStackMaps();
 
   //===------------------------------------------------------------------===//
   // Overridable Hooks
   //===------------------------------------------------------------------===//
 
   void addAsmPrinterHandler(HandlerInfo Handler) {
     Handlers.insert(Handlers.begin(), std::move(Handler));
     NumUserHandlers++;

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

Align AsmPrinter::getGVAlignment(const GlobalObject *GV, const DataLayout &DL,
   // NumBits because the GV has an assigned section, obey it.
   if (*GVAlign > Alignment || GV->hasSection())
     Alignment = *GVAlign;
   return Alignment;
 }
 
 AsmPrinter::AsmPrinter(TargetMachine &tm, std::unique_ptr<MCStreamer> Streamer)
     : MachineFunctionPass(ID), TM(tm), MAI(tm.getMCAsmInfo()),
-      OutContext(Streamer->getContext()), OutStreamer(std::move(Streamer)) {
+      OutContext(Streamer->getContext()), OutStreamer(std::move(Streamer)),
+      SM(*this) {
   VerboseAsm = OutStreamer->isVerboseAsm();
 }
 
 AsmPrinter::~AsmPrinter() {
   assert(!DD && Handlers.size() == NumUserHandlers &&
          "Debug/EH info didn't get finalized");
 
   if (GCMetadataPrinters) {
if (TM.getTargetTriple().isOSBinFormatCOFF()) {
     }
   }
 
   // This needs to happen before emitting debug information since that can end
   // arbitrary sections.
   if (auto *TS = OutStreamer->getTargetStreamer())
     TS->emitConstantPools();
 
+  // Emit Stack maps before any debug info. Mach-O requires that no data or
+  // text sections come after debug info has been emitted. This matters for
+  // stack maps as they are arbitrary data, and may even have a custom format
+  // through user plugins.
+  emitStackMaps();
+
   // Finalize debug and EH information.
   for (const HandlerInfo &HI : Handlers) {
     NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
                        HI.TimerGroupDescription, TimePassesIsEnabled);
     HI.Handler->endModule();
   }
 
   // This deletes all the ephemeral handlers that AsmPrinter added, while
if (Name == GCMetaPrinter.getName()) {
       GMP->S = &S;
       auto IterBool = GCMap.insert(std::make_pair(&S, std::move(GMP)));
       return IterBool.first->second.get();
     }
 
   report_fatal_error("no GCMetadataPrinter registered for GC: " + Twine(Name));
 }
 
-void AsmPrinter::emitStackMaps(StackMaps &SM) {
+void AsmPrinter::emitStackMaps() {
   GCModuleInfo *MI = getAnalysisIfAvailable<GCModuleInfo>();
   assert(MI && "AsmPrinter didn't require GCModuleInfo?");
   bool NeedsDefault = false;
   if (MI->begin() == MI->end())
     // No GC strategy, use the default format.
     NeedsDefault = true;
   else
     for (const auto &I : *MI) {

llvm/lib/MC/MCMachOStreamer.cpp

void MCMachOStreamer::changeSection(MCSection *Section,
                                     const MCExpr *Subsection) {
   // Change the section normally.
   bool Created = changeSectionImpl(Section, Subsection);
   const MCSectionMachO &MSec = *cast<MCSectionMachO>(Section);
   StringRef SegName = MSec.getSegmentName();
   if (SegName == "__DWARF")
     CreatedADWARFSection = true;
   else if (Created && DWARFMustBeAtTheEnd && !canGoAfterDWARF(MSec))
-    assert((!CreatedADWARFSection ||
-            Section == getContext().getObjectFileInfo()->getStackMapSection())
-           && "Creating regular section after DWARF");
+    assert(!CreatedADWARFSection && "Creating regular section after DWARF");
 
   // Output a linker-local symbol so we don't need section-relative local
   // relocations. The linker hates us when we do that.
   if (LabelSections && !HasSectionLabel[Section] &&
       !Section->getBeginSymbol()) {
     MCSymbol *Label = getContext().createLinkerPrivateTempSymbol();
     Section->setBeginSymbol(Label);
     HasSectionLabel[Section] = true;

llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp

 using namespace llvm;
 
 #define DEBUG_TYPE "asm-printer"
 
 namespace {
 
 class AArch64AsmPrinter : public AsmPrinter {
   AArch64MCInstLower MCInstLowering;
-  StackMaps SM;
   FaultMaps FM;
   const AArch64Subtarget *STI;
   bool ShouldEmitWeakSwiftAsyncExtendedFramePointerFlags = false;
 
 public:
   AArch64AsmPrinter(TargetMachine &TM, std::unique_ptr<MCStreamer> Streamer)
       : AsmPrinter(TM, std::move(Streamer)), MCInstLowering(OutContext, *this),
-        SM(*this), FM(*this) {}
+        FM(*this) {}
 
   StringRef getPassName() const override { return "AArch64 Assembly Printer"; }
 
   /// Wrapper for MCInstLowering.lowerOperand() for the
   /// tblgen'erated pseudo lowering.
   bool lowerOperand(const MachineOperand &MO, MCOperand &MCOp) const {
     return MCInstLowering.lowerOperand(MO, MCOp);
   }
void AArch64AsmPrinter::emitEndOfAsmFile(Module &M) {
   if (TT.isOSBinFormatMachO()) {
     // Funny Darwin hack: This flag tells the linker that no global symbols
     // contain code that falls through to other global symbols (e.g. the obvious
     // implementation of multiple entry points).  If this doesn't occur, the
     // linker can safely perform dead code stripping.  Since LLVM never
     // generates code that does this, it is always safe to set.
     OutStreamer->emitAssemblerFlag(MCAF_SubsectionsViaSymbols);
   }
-  
   // Emit stack and fault map information.
-  emitStackMaps(SM);
   FM.serializeToFaultMapSection();
 
 }
 
 void AArch64AsmPrinter::emitLOHs() {
   SmallVector<MCSymbol *, 3> MCArgs;
 
   for (const auto &D : AArch64FI->getLOHContainer()) {

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

protected:
   // VariantKind. For example, we need to be able to identify the following
   // entry as a TLSGD entry so we can add the @m relocation:
   //   .tc .i[TC],i[TL]@m
   // By default, VK_None is used for the VariantKind.
   MapVector<std::pair<const MCSymbol *, MCSymbolRefExpr::VariantKind>,
             MCSymbol *>
       TOC;
   const PPCSubtarget *Subtarget = nullptr;
-  StackMaps SM;
 
 public:
   explicit PPCAsmPrinter(TargetMachine &TM,
                          std::unique_ptr<MCStreamer> Streamer)
-      : AsmPrinter(TM, std::move(Streamer)), SM(*this) {}
+      : AsmPrinter(TM, std::move(Streamer)) {}
 
   StringRef getPassName() const override { return "PowerPC Assembly Printer"; }
 
   MCSymbol *lookUpOrCreateTOCEntry(const MCSymbol *Sym,
                                    MCSymbolRefExpr::VariantKind Kind =
                                        MCSymbolRefExpr::VariantKind::VK_None);
 
   bool doInitialization(Module &M) override {
public:
   void printOperand(const MachineInstr *MI, unsigned OpNo, raw_ostream &O);
 
   void PrintSymbolOperand(const MachineOperand &MO, raw_ostream &O) override;
   bool PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
                        const char *ExtraCode, raw_ostream &O) override;
   bool PrintAsmMemoryOperand(const MachineInstr *MI, unsigned OpNo,
                              const char *ExtraCode, raw_ostream &O) override;
 
-  void emitEndOfAsmFile(Module &M) override;
-
   void LowerSTACKMAP(StackMaps &SM, const MachineInstr &MI);
   void LowerPATCHPOINT(StackMaps &SM, const MachineInstr &MI);
   void EmitTlsCall(const MachineInstr *MI, MCSymbolRefExpr::VariantKind VK);
   bool runOnMachineFunction(MachineFunction &MF) override {
     Subtarget = &MF.getSubtarget<PPCSubtarget>();
     bool Changed = AsmPrinter::runOnMachineFunction(MF);
     emitXRayTable();
     return Changed;

 PPCAsmPrinter::lookUpOrCreateTOCEntry(const MCSymbol *Sym,
                                       MCSymbolRefExpr::VariantKind Kind) {
   MCSymbol *&TOCEntry = TOC[{Sym, Kind}];
   if (!TOCEntry)
     TOCEntry = createTempSymbol("C");
   return TOCEntry;
 }
 
-void PPCAsmPrinter::emitEndOfAsmFile(Module &M) {
-  emitStackMaps(SM);
-}
-
 void PPCAsmPrinter::LowerSTACKMAP(StackMaps &SM, const MachineInstr &MI) {
   unsigned NumNOPBytes = MI.getOperand(1).getImm();
   
   auto &Ctx = OutStreamer->getContext();
   MCSymbol *MILabel = Ctx.createTempSymbol();
   OutStreamer->emitLabel(MILabel);
 
   SM.recordStackMap(*MILabel, MI);

llvm/lib/Target/SystemZ/SystemZAsmPrinter.h

 namespace llvm {
 class MCStreamer;
 class MachineInstr;
 class Module;
 class raw_ostream;
 
 class LLVM_LIBRARY_VISIBILITY SystemZAsmPrinter : public AsmPrinter {
 private:
-  StackMaps SM;
   MCSymbol *CurrentFnPPA1Sym;     // PPA1 Symbol.
   MCSymbol *CurrentFnEPMarkerSym; // Entry Point Marker.
 
   SystemZTargetStreamer *getTargetStreamer() {
     MCTargetStreamer *TS = OutStreamer->getTargetStreamer();
     assert(TS && "do not have a target streamer");
     return static_cast<SystemZTargetStreamer *>(TS);
   }
enum class CallType {
     BALR1415 = 6, // b'x110' == BALR  r14,r15
     BASR33 = 7,   // b'x111' == BASR  r3,r3
   };
 
   void emitPPA1(MCSymbol *FnEndSym);
 
 public:
   SystemZAsmPrinter(TargetMachine &TM, std::unique_ptr<MCStreamer> Streamer)
-      : AsmPrinter(TM, std::move(Streamer)), SM(*this),
-        CurrentFnPPA1Sym(nullptr), CurrentFnEPMarkerSym(nullptr) {}
+      : AsmPrinter(TM, std::move(Streamer)), CurrentFnPPA1Sym(nullptr),
+        CurrentFnEPMarkerSym(nullptr) {}
 
   // Override AsmPrinter.
   StringRef getPassName() const override { return "SystemZ Assembly Printer"; }
   void emitInstruction(const MachineInstr *MI) override;
   void emitMachineConstantPoolValue(MachineConstantPoolValue *MCPV) override;
-  void emitEndOfAsmFile(Module &M) override;
   bool PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
                        const char *ExtraCode, raw_ostream &OS) override;
   bool PrintAsmMemoryOperand(const MachineInstr *MI, unsigned OpNo,
                              const char *ExtraCode, raw_ostream &OS) override;
 
   bool doInitialization(Module &M) override {
     SM.reset();
     return AsmPrinter::doInitialization(M);

llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp

bool SystemZAsmPrinter::PrintAsmMemoryOperand(const MachineInstr *MI,
                                               raw_ostream &OS) {
   SystemZInstPrinter::
     printAddress(MAI, MI->getOperand(OpNo).getReg(),
                  MCOperand::createImm(MI->getOperand(OpNo + 1).getImm()),
                  MI->getOperand(OpNo + 2).getReg(), OS);
   return false;
 }
 
-void SystemZAsmPrinter::emitEndOfAsmFile(Module &M) {
-  emitStackMaps(SM);
-}
-
 void SystemZAsmPrinter::emitFunctionBodyEnd() {
   if (TM.getTargetTriple().isOSzOS()) {
     // Emit symbol for the end of function if the z/OS target streamer
     // is used. This is needed to calculate the size of the function.
     MCSymbol *FnEndSym = createTempSymbol("func_end");
     OutStreamer->emitLabel(FnEndSym);
 
     OutStreamer->pushSection();

llvm/lib/Target/X86/X86AsmPrinter.h

 namespace llvm {
 class MCCodeEmitter;
 class MCStreamer;
 class X86Subtarget;
 class TargetMachine;
 
 class LLVM_LIBRARY_VISIBILITY X86AsmPrinter : public AsmPrinter {
   const X86Subtarget *Subtarget = nullptr;
-  StackMaps SM;
   FaultMaps FM;
   std::unique_ptr<MCCodeEmitter> CodeEmitter;
   bool EmitFPOData = false;
   bool ShouldEmitWeakSwiftAsyncExtendedFramePointerFlags = false;
   bool IndCSPrefix = false;
 
   // This utility class tracks the length of a stackmap instruction's 'shadow'.
   // It is used by the X86AsmPrinter to ensure that the stackmap shadow

llvm/lib/Target/X86/X86AsmPrinter.cpp

 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MachineValueType.h"
 #include "llvm/Target/TargetMachine.h"
 
 using namespace llvm;
 
 X86AsmPrinter::X86AsmPrinter(TargetMachine &TM,
                              std::unique_ptr<MCStreamer> Streamer)
-    : AsmPrinter(TM, std::move(Streamer)), SM(*this), FM(*this) {}
+    : AsmPrinter(TM, std::move(Streamer)), FM(*this) {}
 
 //===----------------------------------------------------------------------===//
 // Primitive Helper Functions.
 //===----------------------------------------------------------------------===//
 
 /// runOnMachineFunction - Emit the function body.
 ///
 bool X86AsmPrinter::runOnMachineFunction(MachineFunction &MF) {

 void X86AsmPrinter::emitEndOfAsmFile(Module &M) {
   const Triple &TT = TM.getTargetTriple();
 
   if (TT.isOSBinFormatMachO()) {
     // Mach-O uses non-lazy symbol stubs to encode per-TU information into
     // global table for symbol lookup.
     emitNonLazyStubs(MMI, *OutStreamer);
 
-    // Emit stack and fault map information.
-    emitStackMaps(SM);
+    // Emit fault map information.
     FM.serializeToFaultMapSection();
 
     // This flag tells the linker that no global symbols contain code that fall
     // through to other global symbols (e.g. an implementation of multiple entry
     // points). If this doesn't occur, the linker can safely perform dead code
     // stripping. Since LLVM never generates code that does this, it is always
     // safe to set.
     OutStreamer->emitAssemblerFlag(MCAF_SubsectionsViaSymbols);
if (MMI->usesMSVCFloatingPoint()) {
       // that only has: `scanf("%f", &global_float);` may fail to trigger this,
       // but oh well...that's a documented issue.
       StringRef SymbolName =
           (TT.getArch() == Triple::x86) ? "__fltused" : "_fltused";
       MCSymbol *S = MMI->getContext().getOrCreateSymbol(SymbolName);
       OutStreamer->emitSymbolAttribute(S, MCSA_Global);
       return;
     }
-    emitStackMaps(SM);
   } else if (TT.isOSBinFormatELF()) {
-    emitStackMaps(SM);
     FM.serializeToFaultMapSection();
   }
 
   // Emit __morestack address if needed for indirect calls.
   if (TT.getArch() == Triple::x86_64 && TM.getCodeModel() == CodeModel::Large) {
     if (MCSymbol *AddrSymbol = OutContext.lookupSymbol("__morestack_addr")) {
       Align Alignment(1);
       MCSection *ReadOnlySection = getObjFileLowering().getSectionForConstant(

llvm/test/CodeGen/X86/stackmap-macho.ll

+; RUN: llc  -verify-machineinstrs < %s | FileCheck %s
+
+; Used to crash with assertions when emitting object files.
+; RUN: llc -filetype=obj %s -o /dev/null
+
+; Check stack map section is emitted before debug info.
+; CHECK: .section __LLVM_STACKMAPS
+; CHECK: .section __DWARF
+
+target triple = "x86_64-apple-macosx12.0"
+
+declare void @func()
+
+define ptr addrspace(1) @test1(ptr addrspace(1) %arg) gc "statepoint-example" {
+entry:
+  %safepoint_token = call token (i64, i32, ptr, i32, i32, ...) @llvm.experimental.gc.statepoint.p0(i64 0, i32 0, ptr elementtype(void ()) @func, i32 0, i32 0, i32 0, i32 0) ["gc-live"(ptr addrspace(1) %arg)]
+  %reloc1 = call ptr addrspace(1) @llvm.experimental.gc.relocate.p1(token %safepoint_token,  i32 0, i32 0)
+  ret ptr addrspace(1) %reloc1
+}
+
+declare token @llvm.experimental.gc.statepoint.p0(i64, i32, ptr, i32, i32, ...)
+declare ptr addrspace(1) @llvm.experimental.gc.relocate.p1(token, i32, i32)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2}
+!0 = distinct !DICompileUnit(language: DW_LANG_C89, producer: "clang", isOptimized: true, emissionKind: FullDebug, file: !1, enums: !{}, retainedTypes: !{})
+!1 = !DIFile(filename: "t.c", directory: "")
+!2 = !{i32 2, !"Debug Info Version", i32 3}

llvm/test/CodeGen/X86/statepoint-stackmap-size.ll

 ; RUN: llc  -verify-machineinstrs < %s | FileCheck %s
 
 ; Without removal of duplicate entries, the size is 62 lines
 ;      CHECK: .section  .llvm_stackmaps,{{.*$}}
 ; CHECK-NEXT:{{(.+$[[:space:]]){48}[[:space:]]}}
-;  CHECK-NOT:{{.|[[:space:]]}}
+; CHECK-SAME: .section
 
 target triple = "x86_64-pc-linux-gnu"
 
 declare void @func()
 
 define i1 @test1(ptr addrspace(1) %arg) gc "statepoint-example" {
 entry:
   %safepoint_token = call token (i64, i32, ptr, i32, i32, ...) @llvm.experimental.gc.statepoint.p0(i64 0, i32 0, ptr elementtype(void ()) @func, i32 0, i32 0, i32 0, i32 0) ["gc-live"(ptr addrspace(1) %arg)]