[MC] Allow .pushsection between .cfi_startproc/.cfi_endproc

[abrachet]

Summary

This follows the behavior of gnu assemblers. This is useful when
writing inline assembly.

[fmayer]

llvm/lib/MC/MCStreamer.cpp

448–449

!FrameInfoStack.empty()

[fmayer]

LGTM, but leaving MaskRay to take a look too.

[MaskRay]

In D141893#4060551, @fmayer wrote:

LGTM, but leaving MaskRay to take a look too.

Thank you! I am on a trip and will be back on 2023-01-27. I'll try to read this through as quickly as I can.

[MaskRay]

(back from a trip) Thanks!

llvm/include/llvm/MC/MCStreamer.h

217

This deserves a comment about what first/second are.

llvm/test/MC/ELF/cfi-startproc-pushsection.s

2

llvm-dwarfdump --eh-frame is slightly preferred over llvm-objdump

5

Change one .cfi_startproc to not use simple: to test more variants

[MaskRay]

[MC] Allow new .cfi_startproc in new section

The subject isn't precise. Perhaps Allow .pushsection between .cfi_startproc/.cfi_endproc ?

llvm/include/llvm/MC/MCStreamer.h

 /// There are multiple implementations of this interface: one for writing out
 /// a .s file, and implementations that write out .o files of various formats.
 ///
 class MCStreamer {
   MCContext &Context;
   std::unique_ptr<MCTargetStreamer> TargetStreamer;
 
   std::vector<MCDwarfFrameInfo> DwarfFrameInfos;
+  // This is a pair of index into DwarfFrameInfos and the MCSection associated
+  // with the frame. Note, we use an index instead of an iterator because they
+  // can be invalidated in std::vector.
+  SmallVector<std::pair<size_t, MCSection *>, 1> FrameInfoStack;
   MCDwarfFrameInfo *getCurrentDwarfFrameInfo();
 
   /// Similar to DwarfFrameInfos, but for SEH unwind info. Chained frames may
   /// refer to each other, so use std::unique_ptr to provide pointer stability.
   std::vector<std::unique_ptr<WinEH::FrameInfo>> WinFrameInfos;
 
   WinEH::FrameInfo *CurrentWinFrameInfo;
   size_t CurrentProcWinFrameInfoStartIndex;

llvm/lib/MC/MCStreamer.cpp

if (!Table.getLabel()) {
     StringRef Prefix = Context.getAsmInfo()->getPrivateGlobalPrefix();
     Table.setLabel(
         Context.getOrCreateSymbol(Prefix + "line_table_start" + Twine(CUID)));
   }
   return Table.getLabel();
 }
 
 bool MCStreamer::hasUnfinishedDwarfFrameInfo() {
-  return !DwarfFrameInfos.empty() && !DwarfFrameInfos.back().End;
+  return !FrameInfoStack.empty();
 }
 
 MCDwarfFrameInfo *MCStreamer::getCurrentDwarfFrameInfo() {
   if (!hasUnfinishedDwarfFrameInfo()) {
     getContext().reportError(getStartTokLoc(),
                              "this directive must appear between "
                              ".cfi_startproc and .cfi_endproc directives");
     return nullptr;
   }
-  return &DwarfFrameInfos.back();
+  return &DwarfFrameInfos[FrameInfoStack.back().first];
 }
 
 bool MCStreamer::emitCVFileDirective(unsigned FileNo, StringRef Filename,
                                      ArrayRef<uint8_t> Checksum,
                                      unsigned ChecksumKind) {
   return getContext().getCVContext().addFile(*this, FileNo, Filename, Checksum,
                                              ChecksumKind);
 }

 }
 
 void MCStreamer::emitConditionalAssignment(MCSymbol *Symbol,
                                            const MCExpr *Value) {}
 
 void MCStreamer::emitCFISections(bool EH, bool Debug) {}
 
 void MCStreamer::emitCFIStartProc(bool IsSimple, SMLoc Loc) {
-  if (hasUnfinishedDwarfFrameInfo())
+  if (!FrameInfoStack.empty() &&
+      getCurrentSectionOnly() == FrameInfoStack.back().second)
     return getContext().reportError(
         Loc, "starting new .cfi frame before finishing the previous one");
 
   MCDwarfFrameInfo Frame;
   Frame.IsSimple = IsSimple;
   emitCFIStartProcImpl(Frame);
 
   const MCAsmInfo* MAI = Context.getAsmInfo();
   if (MAI) {
     for (const MCCFIInstruction& Inst : MAI->getInitialFrameState()) {
       if (Inst.getOperation() == MCCFIInstruction::OpDefCfa ||
           Inst.getOperation() == MCCFIInstruction::OpDefCfaRegister ||
           Inst.getOperation() == MCCFIInstruction::OpLLVMDefAspaceCfa) {
         Frame.CurrentCfaRegister = Inst.getRegister();
       }
     }
   }
 
+  FrameInfoStack.emplace_back(DwarfFrameInfos.size(), getCurrentSectionOnly());
   DwarfFrameInfos.push_back(Frame);
 }
 
 void MCStreamer::emitCFIStartProcImpl(MCDwarfFrameInfo &Frame) {
 }
 
 void MCStreamer::emitCFIEndProc() {
   MCDwarfFrameInfo *CurFrame = getCurrentDwarfFrameInfo();
   if (!CurFrame)
     return;
   emitCFIEndProcImpl(*CurFrame);
+  FrameInfoStack.pop_back();
 }
 
 void MCStreamer::emitCFIEndProcImpl(MCDwarfFrameInfo &Frame) {
   // Put a dummy non-null value in Frame.End to mark that this frame has been
   // closed.
   Frame.End = (MCSymbol *)1;
 }
 

llvm/test/MC/ELF/cfi-startproc-pushsection.s

+// RUN: llvm-mc -filetype=obj -triple x86_64-linux %s -o - | llvm-dwarfdump --eh-frame - | FileCheck %s
+
+.section .text.a, "ax", %progbits
+.cfi_startproc
+.cfi_def_cfa %rsp, 0
+
+.pushsection .text.b, "ax", %progbits
+.cfi_startproc simple
+.cfi_def_cfa %rsp, 8
+nop
+ret
+
+.pushsection .text.c, "ax", %progbits
+.cfi_startproc simple
+.cfi_def_cfa %rsp, 16
+nop
+nop
+ret
+.cfi_endproc
+.popsection
+
+.cfi_endproc
+.popsection
+
+.pushsection .text.d, "ax", %progbits
+.cfi_startproc simple
+.cfi_def_cfa %rsp, 24
+nop
+nop
+nop
+ret
+.cfi_endproc
+.popsection
+
+ret
+.cfi_endproc
+
+// CHECK: pc=00000000...00000001
+// CHECK: RSP +0
+// CHECK: pc=00000000...00000002
+// CHECK: RSP +8
+// CHECK: pc=00000000...00000003
+// CHECK: RSP +16
+// CHECK: pc=00000000...00000004
+// CHECK: RSP +24