Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MC] Support .cfi_label #97922

Merged
merged 3 commits into from
Jul 7, 2024
Merged

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Jul 6, 2024

GNU assembler 2.26 introduced the .cfi_label directive. It does not
expand to any CFI instructions, but defines a label in
.eh_frame/.debug_frame, which can be used by runtime patching code to
locate the FDE. .cfi_label is not allowed for CIE's initial
instructions, and can therefore be used to force the next instruction to
be placed in a FDE instead of a CIE.

In glibc since 2018, sysdeps/riscv/start.S utilizes .cfi_label to force
DW_CFA_undefined to be placed in a FDE. arc/csky/loongarch ports have
copied this use.

.cfi_startproc
// DW_CFA_undefined is allowed for CIE's initial instructions.
// Without .cfi_label, gas would place DW_CFA_undefined in a CIE.
.cfi_label .Ldummy
.cfi_undefined ra
.cfi_endproc

No CFI instruction is associated with .cfi_label, so the case MCCFIInstruction::OpLabel: code in BOLT is unreachable and onlt to make
-Wswitch happy.

Close #97222

Created using spr 1.3.5-bogner
@llvmbot llvmbot added debuginfo mc Machine (object) code labels Jul 6, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 6, 2024

@llvm/pr-subscribers-bolt
@llvm/pr-subscribers-debuginfo

@llvm/pr-subscribers-mc

Author: Fangrui Song (MaskRay)

Changes

GNU assembler 2.26 introduced the .cfi_label directive. It does not
expand to any CFI instructions, but defines a label in
.eh_frame/.debug_frame, which can be used by runtime patching code to
locate the FDE. .cfi_label is not allowed for CIE's initial
instructions, and can therefore be used to force the next instruction to
be placed in a FDE instead of a CIE.

In glibc since 2018, sysdeps/riscv/start.S utilizes .cfi_label to force
DW_CFA_undefined to be placed in a FDE. arc/csky/loongarch ports have
copied this use.

.cfi_startproc
// DW_CFA_undefined is allowed for CIE's initial instructions.
// Without .cfi_label, gas would place DW_CFA_undefined in a CIE.
.cfi_label .Ldummy
.cfi_undefined ra
.cfi_endproc

Full diff: https://github.com/llvm/llvm-project/pull/97922.diff

8 Files Affected:

  • (modified) llvm/include/llvm/MC/MCDwarf.h (+19-1)
  • (modified) llvm/include/llvm/MC/MCStreamer.h (+1)
  • (modified) llvm/lib/CodeGen/CFIInstrInserter.cpp (+1)
  • (modified) llvm/lib/MC/MCAsmStreamer.cpp (+7)
  • (modified) llvm/lib/MC/MCDwarf.cpp (+3)
  • (modified) llvm/lib/MC/MCParser/AsmParser.cpp (+18)
  • (modified) llvm/lib/MC/MCStreamer.cpp (+7)
  • (added) llvm/test/MC/ELF/cfi-label.s (+61)
diff --git a/llvm/include/llvm/MC/MCDwarf.h b/llvm/include/llvm/MC/MCDwarf.h
index 5052c6756c32b..d0e45ab59a92e 100644
--- a/llvm/include/llvm/MC/MCDwarf.h
+++ b/llvm/include/llvm/MC/MCDwarf.h
@@ -500,7 +500,8 @@ class MCCFIInstruction {
     OpRegister,
     OpWindowSave,
     OpNegateRAState,
-    OpGnuArgsSize
+    OpGnuArgsSize,
+    OpLabel,
   };
 
 private:
@@ -519,6 +520,7 @@ class MCCFIInstruction {
       unsigned Register;
       unsigned Register2;
     } RR;
+    MCSymbol *CfiLabel;
   } U;
   OpType Operation;
   SMLoc Loc;
@@ -544,6 +546,12 @@ class MCCFIInstruction {
     U.RIA = {R, O, AS};
   }
 
+  MCCFIInstruction(OpType Op, MCSymbol *L, MCSymbol *CfiLabel, SMLoc Loc)
+      : Label(L), Operation(Op), Loc(Loc) {
+    assert(Op == OpLabel);
+    U.CfiLabel = CfiLabel;
+  }
+
 public:
   /// .cfi_def_cfa defines a rule for computing CFA as: take address from
   /// Register and add Offset to it.
@@ -664,6 +672,11 @@ class MCCFIInstruction {
     return MCCFIInstruction(OpGnuArgsSize, L, 0, Size, Loc);
   }
 
+  static MCCFIInstruction createLabel(MCSymbol *L, MCSymbol *CfiLabel,
+                                      SMLoc Loc) {
+    return MCCFIInstruction(OpLabel, L, CfiLabel, Loc);
+  }
+
   OpType getOperation() const { return Operation; }
   MCSymbol *getLabel() const { return Label; }
 
@@ -698,6 +711,11 @@ class MCCFIInstruction {
     return U.RI.Offset;
   }
 
+  MCSymbol *getCfiLabel() const {
+    assert(Operation == OpLabel);
+    return U.CfiLabel;
+  }
+
   StringRef getValues() const {
     assert(Operation == OpEscape);
     return StringRef(&Values[0], Values.size());
diff --git a/llvm/include/llvm/MC/MCStreamer.h b/llvm/include/llvm/MC/MCStreamer.h
index 1d3057a140d8a..78aa12062102c 100644
--- a/llvm/include/llvm/MC/MCStreamer.h
+++ b/llvm/include/llvm/MC/MCStreamer.h
@@ -1019,6 +1019,7 @@ class MCStreamer {
                                SMLoc Loc = {});
   virtual void emitCFIWindowSave(SMLoc Loc = {});
   virtual void emitCFINegateRAState(SMLoc Loc = {});
+  virtual void emitCFILabelDirective(SMLoc Loc, StringRef Name);
 
   virtual void emitWinCFIStartProc(const MCSymbol *Symbol, SMLoc Loc = SMLoc());
   virtual void emitWinCFIEndProc(SMLoc Loc = SMLoc());
diff --git a/llvm/lib/CodeGen/CFIInstrInserter.cpp b/llvm/lib/CodeGen/CFIInstrInserter.cpp
index 87b062a16df1d..1ff01ad34b30e 100644
--- a/llvm/lib/CodeGen/CFIInstrInserter.cpp
+++ b/llvm/lib/CodeGen/CFIInstrInserter.cpp
@@ -248,6 +248,7 @@ void CFIInstrInserter::calculateOutgoingCFAInfo(MBBCFAInfo &MBBInfo) {
       case MCCFIInstruction::OpWindowSave:
       case MCCFIInstruction::OpNegateRAState:
       case MCCFIInstruction::OpGnuArgsSize:
+      case MCCFIInstruction::OpLabel:
         break;
       }
       if (CSRReg || CSROffset) {
diff --git a/llvm/lib/MC/MCAsmStreamer.cpp b/llvm/lib/MC/MCAsmStreamer.cpp
index 0050923f3cd9d..45c32f13e759b 100644
--- a/llvm/lib/MC/MCAsmStreamer.cpp
+++ b/llvm/lib/MC/MCAsmStreamer.cpp
@@ -356,6 +356,7 @@ class MCAsmStreamer final : public MCStreamer {
   void emitCFIWindowSave(SMLoc Loc) override;
   void emitCFINegateRAState(SMLoc Loc) override;
   void emitCFIReturnColumn(int64_t Register) override;
+  void emitCFILabelDirective(SMLoc Loc, StringRef Name) override;
 
   void emitWinCFIStartProc(const MCSymbol *Symbol, SMLoc Loc) override;
   void emitWinCFIEndProc(SMLoc Loc) override;
@@ -2126,6 +2127,12 @@ void MCAsmStreamer::emitCFIReturnColumn(int64_t Register) {
   EmitEOL();
 }
 
+void MCAsmStreamer::emitCFILabelDirective(SMLoc Loc, StringRef Name) {
+  MCStreamer::emitCFILabelDirective(Loc, Name);
+  OS << "\t.cfi_label " << Name;
+  EmitEOL();
+}
+
 void MCAsmStreamer::emitCFIBKeyFrame() {
   MCStreamer::emitCFIBKeyFrame();
   OS << "\t.cfi_b_key_frame";
diff --git a/llvm/lib/MC/MCDwarf.cpp b/llvm/lib/MC/MCDwarf.cpp
index 321a66ee5abc4..efafd555c5c5c 100644
--- a/llvm/lib/MC/MCDwarf.cpp
+++ b/llvm/lib/MC/MCDwarf.cpp
@@ -1465,6 +1465,9 @@ void FrameEmitterImpl::emitCFIInstruction(const MCCFIInstruction &Instr) {
   case MCCFIInstruction::OpEscape:
     Streamer.emitBytes(Instr.getValues());
     return;
+  case MCCFIInstruction::OpLabel:
+    Streamer.emitLabel(Instr.getCfiLabel(), Instr.getLoc());
+    return;
   }
   llvm_unreachable("Unhandled case in switch");
 }
diff --git a/llvm/lib/MC/MCParser/AsmParser.cpp b/llvm/lib/MC/MCParser/AsmParser.cpp
index 707edb0481a61..f3caa90eedfb1 100644
--- a/llvm/lib/MC/MCParser/AsmParser.cpp
+++ b/llvm/lib/MC/MCParser/AsmParser.cpp
@@ -520,6 +520,7 @@ class AsmParser : public MCAsmParser {
     DK_CFI_UNDEFINED,
     DK_CFI_REGISTER,
     DK_CFI_WINDOW_SAVE,
+    DK_CFI_LABEL,
     DK_CFI_B_KEY_FRAME,
     DK_MACROS_ON,
     DK_MACROS_OFF,
@@ -622,6 +623,7 @@ class AsmParser : public MCAsmParser {
   bool parseDirectiveCFIReturnColumn(SMLoc DirectiveLoc);
   bool parseDirectiveCFISignalFrame(SMLoc DirectiveLoc);
   bool parseDirectiveCFIUndefined(SMLoc DirectiveLoc);
+  bool parseDirectiveCFILabel(SMLoc DirectiveLoc);
 
   // macro directives
   bool parseDirectivePurgeMacro(SMLoc DirectiveLoc);
@@ -2224,6 +2226,8 @@ bool AsmParser::parseStatement(ParseStatementInfo &Info,
       return parseDirectiveCFIRegister(IDLoc);
     case DK_CFI_WINDOW_SAVE:
       return parseDirectiveCFIWindowSave(IDLoc);
+    case DK_CFI_LABEL:
+      return parseDirectiveCFILabel(IDLoc);
     case DK_MACROS_ON:
     case DK_MACROS_OFF:
       return parseDirectiveMacrosOnOff(IDVal);
@@ -4488,6 +4492,19 @@ bool AsmParser::parseDirectiveCFIUndefined(SMLoc DirectiveLoc) {
   return false;
 }
 
+/// parseDirectiveCFILabel
+/// ::= .cfi_label label
+bool AsmParser::parseDirectiveCFILabel(SMLoc Loc) {
+  StringRef Name;
+  Loc = Lexer.getLoc();
+  if (parseIdentifier(Name))
+    return TokError("expected identifier");
+  if (parseEOL())
+    return true;
+  getStreamer().emitCFILabelDirective(Loc, Name);
+  return false;
+}
+
 /// parseDirectiveAltmacro
 /// ::= .altmacro
 /// ::= .noaltmacro
@@ -5560,6 +5577,7 @@ void AsmParser::initializeDirectiveKindMap() {
   DirectiveKindMap[".cfi_undefined"] = DK_CFI_UNDEFINED;
   DirectiveKindMap[".cfi_register"] = DK_CFI_REGISTER;
   DirectiveKindMap[".cfi_window_save"] = DK_CFI_WINDOW_SAVE;
+  DirectiveKindMap[".cfi_label"] = DK_CFI_LABEL;
   DirectiveKindMap[".cfi_b_key_frame"] = DK_CFI_B_KEY_FRAME;
   DirectiveKindMap[".cfi_mte_tagged_frame"] = DK_CFI_MTE_TAGGED_FRAME;
   DirectiveKindMap[".macros_on"] = DK_MACROS_ON;
diff --git a/llvm/lib/MC/MCStreamer.cpp b/llvm/lib/MC/MCStreamer.cpp
index a3f67941c1015..1594bd3231abe 100644
--- a/llvm/lib/MC/MCStreamer.cpp
+++ b/llvm/lib/MC/MCStreamer.cpp
@@ -689,6 +689,13 @@ void MCStreamer::emitCFIReturnColumn(int64_t Register) {
   CurFrame->RAReg = Register;
 }
 
+void MCStreamer::emitCFILabelDirective(SMLoc Loc, StringRef Name) {
+  MCSymbol *Label = emitCFILabel();
+  MCSymbol *Sym = getContext().getOrCreateSymbol(Name);
+  if (MCDwarfFrameInfo *F = getCurrentDwarfFrameInfo())
+    F->Instructions.push_back(MCCFIInstruction::createLabel(Label, Sym, Loc));
+}
+
 WinEH::FrameInfo *MCStreamer::EnsureValidWinFrameInfo(SMLoc Loc) {
   const MCAsmInfo *MAI = Context.getAsmInfo();
   if (!MAI->usesWindowsCFI()) {
diff --git a/llvm/test/MC/ELF/cfi-label.s b/llvm/test/MC/ELF/cfi-label.s
new file mode 100644
index 0000000000000..e0df9b8a1b253
--- /dev/null
+++ b/llvm/test/MC/ELF/cfi-label.s
@@ -0,0 +1,61 @@
+# RUN: llvm-mc -triple x86_64 %s | FileCheck %s --check-prefix=ASM
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t
+# RUN: llvm-readelf -sX %t | FileCheck %s --check-prefix=SYMTAB
+# RUN: llvm-dwarfdump --eh-frame %t | FileCheck %s
+
+# RUN: not llvm-mc -filetype=obj -triple=x86_64 --defsym ERR=1 %s -o /dev/null 2>&1 | \
+# RUN:   FileCheck %s --check-prefix=ERR --implicit-check-not=error:
+
+# ASM:      nop
+# ASM-NEXT: .cfi_label cfi1
+# ASM-NEXT: .cfi_escape 0x00
+# ASM:      .globl cfi2
+# ASM-NEXT: .cfi_label cfi2
+# ASM:      nop
+# ASM-NEXT: .cfi_label .Lcfi3
+
+# SYMTAB:      000000000000002b     0 NOTYPE  LOCAL  DEFAULT     3 (.eh_frame) cfi1
+# SYMTAB:      000000000000002d     0 NOTYPE  GLOBAL DEFAULT     3 (.eh_frame) cfi2
+# SYMTAB-NOT:  {{.}}
+
+# CHECK:       DW_CFA_remember_state:
+# CHECK-NEXT:  DW_CFA_advance_loc: 1 to 0x1
+# CHECK-NEXT:  DW_CFA_nop:
+# CHECK-NEXT:  DW_CFA_advance_loc: 1 to 0x2
+# CHECK-NEXT:  DW_CFA_nop:
+# CHECK-NEXT:  DW_CFA_nop:
+# CHECK-NEXT:  DW_CFA_advance_loc: 1 to 0x3
+# CHECK-NEXT:  DW_CFA_nop:
+# CHECK-NEXT:  DW_CFA_nop:
+# CHECK-NEXT:  DW_CFA_nop:
+# CHECK-NEXT:  DW_CFA_restore_state:
+
+.globl foo
+foo:
+.cfi_startproc
+.cfi_remember_state
+nop
+.cfi_label cfi1
+.cfi_escape 0
+nop
+.globl cfi2
+.cfi_label cfi2
+.cfi_escape 0, 0
+nop
+.cfi_label .Lcfi3
+.cfi_escape 0, 0, 0
+.cfi_restore_state
+ret
+
+# ERR: [[#@LINE+10]]:1: error: this directive must appear between .cfi_startproc and .cfi_endproc directives
+.ifdef ERR
+# ERR: [[#@LINE+1]]:12: error: symbol 'foo' is already defined
+.cfi_label foo
+# ERR: [[#@LINE+1]]:12: error: symbol '.Lcfi3' is already defined
+.cfi_label .Lcfi3
+.endif
+.cfi_endproc
+
+.ifdef ERR
+.cfi_label after_endproc
+.endif

Copy link
Collaborator

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Created using spr 1.3.5-bogner
@MaskRay MaskRay merged commit 2718654 into main Jul 7, 2024
3 of 5 checks passed
@MaskRay MaskRay deleted the users/MaskRay/spr/mc-support-cfi_label branch July 7, 2024 19:41
searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Jul 9, 2024
GNU assembler 2.26 introduced the .cfi_label directive. It does not
expand to any CFI instructions, but defines a label in
.eh_frame/.debug_frame, which can be used by runtime patching code to
locate the FDE. .cfi_label is not allowed for CIE's initial
instructions, and can therefore be used to force the next instruction to
be placed in a FDE instead of a CIE.

In glibc since 2018, sysdeps/riscv/start.S utilizes .cfi_label to force
DW_CFA_undefined to be placed in a FDE. arc/csky/loongarch ports have
copied this use.
```
.cfi_startproc
// DW_CFA_undefined is allowed for CIE's initial instructions.
// Without .cfi_label, gas would place DW_CFA_undefined in a CIE.
.cfi_label .Ldummy
.cfi_undefined ra
.cfi_endproc
```

No CFI instruction is associated with .cfi_label, so the `case
MCCFIInstruction::OpLabel:` code in BOLT is unreachable and onlt to make
-Wswitch happy.

Close llvm#97222

Pull Request: llvm#97922

Change-Id: Ic236d33bf52ac5631bd202c2896d23463c8e099b
alx32 added a commit that referenced this pull request Sep 20, 2024
As discussed in [the
RFC](https://discourse.llvm.org/t/rfc-extending-llvm-mc-loc-directive-with-labeling-support/79608)
we need a way to create labels in the assembler-generated line section
in order to support the future addition of the
[DW_AT_LLVM_stmt_sequence](https://discourse.llvm.org/t/rfc-new-dwarf-attribute-for-symbolication-of-merged-functions/79434)
attribute.

We have a similar precedent for such behavior with the
[.cfi_label](#97922)
instruction - so we add the `.loc_label THE_LABEL_NAME` instruction
which:
- Terminates the current line sequence in the line section
- Creates a new label with the specified label name in the `.debug_line`
section
xgupta pushed a commit to xgupta/llvm-project that referenced this pull request Oct 4, 2024
As discussed in [the
RFC](https://discourse.llvm.org/t/rfc-extending-llvm-mc-loc-directive-with-labeling-support/79608)
we need a way to create labels in the assembler-generated line section
in order to support the future addition of the
[DW_AT_LLVM_stmt_sequence](https://discourse.llvm.org/t/rfc-new-dwarf-attribute-for-symbolication-of-merged-functions/79434)
attribute.

We have a similar precedent for such behavior with the
[.cfi_label](llvm#97922)
instruction - so we add the `.loc_label THE_LABEL_NAME` instruction
which:
- Terminates the current line sequence in the line section
- Creates a new label with the specified label name in the `.debug_line`
section
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BOLT debuginfo mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LLVM's MCParser does not understand .cfi_label directives
3 participants