Skip to content

Conversation

@Sterling-Augustine
Copy link
Contributor

This reverts commit f9d0bd0.

@llvmbot
Copy link
Member

llvmbot commented Jul 23, 2025

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-debuginfo

Author: None (Sterling-Augustine)

Changes

This reverts commit f9d0bd0.


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

13 Files Affected:

  • (modified) llvm/include/llvm/MC/MCObjectStreamer.h (+1-2)
  • (modified) llvm/include/llvm/MC/MCStreamer.h (+1-1)
  • (modified) llvm/include/llvm/MC/MCTargetOptions.h (-3)
  • (modified) llvm/include/llvm/MC/MCTargetOptionsCommandFlags.h (-2)
  • (modified) llvm/lib/CodeGen/AsmPrinter/ARMException.cpp (+1-1)
  • (modified) llvm/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp (+2-4)
  • (modified) llvm/lib/MC/MCAsmStreamer.cpp (+6-15)
  • (modified) llvm/lib/MC/MCObjectStreamer.cpp (+2-3)
  • (modified) llvm/lib/MC/MCParser/AsmParser.cpp (+3-6)
  • (modified) llvm/lib/MC/MCStreamer.cpp (+1-1)
  • (modified) llvm/lib/MC/MCTargetOptionsCommandFlags.cpp (-7)
  • (modified) llvm/test/MC/ELF/AArch64/cfi.s (+1-3)
  • (modified) llvm/test/MC/ELF/cfi.s (+1-3)
diff --git a/llvm/include/llvm/MC/MCObjectStreamer.h b/llvm/include/llvm/MC/MCObjectStreamer.h
index aea93e9dc7e4f..2ceeba22abccd 100644
--- a/llvm/include/llvm/MC/MCObjectStreamer.h
+++ b/llvm/include/llvm/MC/MCObjectStreamer.h
@@ -40,7 +40,6 @@ class MCObjectStreamer : public MCStreamer {
   std::unique_ptr<MCAssembler> Assembler;
   bool EmitEHFrame;
   bool EmitDebugFrame;
-  bool EmitSFrame;
 
   struct PendingAssignment {
     MCSymbol *Symbol;
@@ -71,7 +70,7 @@ class MCObjectStreamer : public MCStreamer {
 
   void emitFrames(MCAsmBackend *MAB);
   MCSymbol *emitCFILabel() override;
-  void emitCFISections(bool EH, bool Debug, bool SFrame) override;
+  void emitCFISections(bool EH, bool Debug) override;
 
 public:
   void visitUsedSymbol(const MCSymbol &Sym) override;
diff --git a/llvm/include/llvm/MC/MCStreamer.h b/llvm/include/llvm/MC/MCStreamer.h
index dfaf3487e11a7..4bfc8f921ef11 100644
--- a/llvm/include/llvm/MC/MCStreamer.h
+++ b/llvm/include/llvm/MC/MCStreamer.h
@@ -986,7 +986,7 @@ class LLVM_ABI MCStreamer {
                                                const MCSymbol *Lo);
 
   virtual MCSymbol *getDwarfLineTableSymbol(unsigned CUID);
-  virtual void emitCFISections(bool EH, bool Debug, bool SFrame);
+  virtual void emitCFISections(bool EH, bool Debug);
   void emitCFIStartProc(bool IsSimple, SMLoc Loc = SMLoc());
   void emitCFIEndProc();
   virtual void emitCFIDefCfa(int64_t Register, int64_t Offset, SMLoc Loc = {});
diff --git a/llvm/include/llvm/MC/MCTargetOptions.h b/llvm/include/llvm/MC/MCTargetOptions.h
index 235d58d585b40..d95adf92b9a83 100644
--- a/llvm/include/llvm/MC/MCTargetOptions.h
+++ b/llvm/include/llvm/MC/MCTargetOptions.h
@@ -102,9 +102,6 @@ class MCTargetOptions {
   // functions on Darwins.
   bool EmitCompactUnwindNonCanonical : 1;
 
-  // Whether to emit SFrame unwind sections.
-  bool EmitSFrameUnwind : 1;
-
   // Whether or not to use full register names on PowerPC.
   bool PPCUseFullRegisterNames : 1;
 
diff --git a/llvm/include/llvm/MC/MCTargetOptionsCommandFlags.h b/llvm/include/llvm/MC/MCTargetOptionsCommandFlags.h
index adfdccdb5ab77..b057effd88128 100644
--- a/llvm/include/llvm/MC/MCTargetOptionsCommandFlags.h
+++ b/llvm/include/llvm/MC/MCTargetOptionsCommandFlags.h
@@ -40,8 +40,6 @@ LLVM_ABI EmitDwarfUnwindType getEmitDwarfUnwind();
 
 LLVM_ABI bool getEmitCompactUnwindNonCanonical();
 
-LLVM_ABI bool getEmitSFrameUnwind();
-
 LLVM_ABI bool getShowMCInst();
 
 LLVM_ABI bool getFatalWarnings();
diff --git a/llvm/lib/CodeGen/AsmPrinter/ARMException.cpp b/llvm/lib/CodeGen/AsmPrinter/ARMException.cpp
index 51342c6b91345..de6ebcf0c3419 100644
--- a/llvm/lib/CodeGen/AsmPrinter/ARMException.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/ARMException.cpp
@@ -39,7 +39,7 @@ void ARMException::beginFunction(const MachineFunction *MF) {
   if (CFISecType == AsmPrinter::CFISection::Debug) {
     if (!hasEmittedCFISections) {
       if (Asm->getModuleCFISectionType() == AsmPrinter::CFISection::Debug)
-        Asm->OutStreamer->emitCFISections(false, true, false);
+        Asm->OutStreamer->emitCFISections(false, true);
       hasEmittedCFISections = true;
     }
 
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp
index 6b8d08cb201d6..4fac4bbc98477 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp
@@ -109,11 +109,9 @@ void DwarfCFIException::beginBasicBlockSection(const MachineBasicBlock &MBB) {
     // chose not to be verbose in that case. And with `ForceDwarfFrameSection`,
     // we should always emit .debug_frame.
     if (CFISecType == AsmPrinter::CFISection::Debug ||
-        Asm->TM.Options.ForceDwarfFrameSection ||
-        Asm->TM.Options.MCOptions.EmitSFrameUnwind)
+        Asm->TM.Options.ForceDwarfFrameSection)
       Asm->OutStreamer->emitCFISections(
-          CFISecType == AsmPrinter::CFISection::EH, true,
-          Asm->TM.Options.MCOptions.EmitSFrameUnwind);
+          CFISecType == AsmPrinter::CFISection::EH, true);
     hasEmittedCFISections = true;
   }
 
diff --git a/llvm/lib/MC/MCAsmStreamer.cpp b/llvm/lib/MC/MCAsmStreamer.cpp
index 7119ef4a2bbcd..67c53e01a6111 100644
--- a/llvm/lib/MC/MCAsmStreamer.cpp
+++ b/llvm/lib/MC/MCAsmStreamer.cpp
@@ -345,7 +345,7 @@ class MCAsmStreamer final : public MCStreamer {
   void emitIdent(StringRef IdentString) override;
   void emitCFIBKeyFrame() override;
   void emitCFIMTETaggedFrame() override;
-  void emitCFISections(bool EH, bool Debug, bool SFrame) override;
+  void emitCFISections(bool EH, bool Debug) override;
   void emitCFIDefCfa(int64_t Register, int64_t Offset, SMLoc Loc) override;
   void emitCFIDefCfaOffset(int64_t Offset, SMLoc Loc) override;
   void emitCFIDefCfaRegister(int64_t Register, SMLoc Loc) override;
@@ -1906,24 +1906,15 @@ void MCAsmStreamer::emitIdent(StringRef IdentString) {
   EmitEOL();
 }
 
-void MCAsmStreamer::emitCFISections(bool EH, bool Debug, bool SFrame) {
-  MCStreamer::emitCFISections(EH, Debug, SFrame);
+void MCAsmStreamer::emitCFISections(bool EH, bool Debug) {
+  MCStreamer::emitCFISections(EH, Debug);
   OS << "\t.cfi_sections ";
-  bool C = false;
   if (EH) {
     OS << ".eh_frame";
-    C = true;
-  }
-  if (Debug) {
-    if (C)
-      OS << ", ";
+    if (Debug)
+      OS << ", .debug_frame";
+  } else if (Debug) {
     OS << ".debug_frame";
-    C = true;
-  }
-  if (SFrame) {
-    if (C)
-      OS << ", ";
-    OS << ".sframe";
   }
 
   EmitEOL();
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index d7f4be1bfb573..42f4cf49d7f38 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -130,11 +130,10 @@ void MCObjectStreamer::visitUsedSymbol(const MCSymbol &Sym) {
   Assembler->registerSymbol(Sym);
 }
 
-void MCObjectStreamer::emitCFISections(bool EH, bool Debug, bool SFrame) {
-  MCStreamer::emitCFISections(EH, Debug, SFrame);
+void MCObjectStreamer::emitCFISections(bool EH, bool Debug) {
+  MCStreamer::emitCFISections(EH, Debug);
   EmitEHFrame = EH;
   EmitDebugFrame = Debug;
-  EmitSFrame = SFrame;
 }
 
 void MCObjectStreamer::emitValueImpl(const MCExpr *Value, unsigned Size,
diff --git a/llvm/lib/MC/MCParser/AsmParser.cpp b/llvm/lib/MC/MCParser/AsmParser.cpp
index eda5e8c37f7b8..d0b6ea4cfd562 100644
--- a/llvm/lib/MC/MCParser/AsmParser.cpp
+++ b/llvm/lib/MC/MCParser/AsmParser.cpp
@@ -4093,30 +4093,27 @@ bool AsmParser::parseDirectiveCVFPOData() {
 }
 
 /// parseDirectiveCFISections
-/// ::= .cfi_sections section [, section][, section]
+/// ::= .cfi_sections section [, section]
 bool AsmParser::parseDirectiveCFISections() {
   StringRef Name;
   bool EH = false;
   bool Debug = false;
-  bool SFrame = false;
 
   if (!parseOptionalToken(AsmToken::EndOfStatement)) {
     for (;;) {
       if (parseIdentifier(Name))
-        return TokError("expected .eh_frame, .debug_frame, or .sframe");
+        return TokError("expected .eh_frame or .debug_frame");
       if (Name == ".eh_frame")
         EH = true;
       else if (Name == ".debug_frame")
         Debug = true;
-      else if (Name == ".sframe")
-        SFrame = true;
       if (parseOptionalToken(AsmToken::EndOfStatement))
         break;
       if (parseComma())
         return true;
     }
   }
-  getStreamer().emitCFISections(EH, Debug, SFrame);
+  getStreamer().emitCFISections(EH, Debug);
   return false;
 }
 
diff --git a/llvm/lib/MC/MCStreamer.cpp b/llvm/lib/MC/MCStreamer.cpp
index e14a32f5dc0ce..30198c97d8ab9 100644
--- a/llvm/lib/MC/MCStreamer.cpp
+++ b/llvm/lib/MC/MCStreamer.cpp
@@ -415,7 +415,7 @@ void MCStreamer::emitLabel(MCSymbol *Symbol, SMLoc Loc) {
 void MCStreamer::emitConditionalAssignment(MCSymbol *Symbol,
                                            const MCExpr *Value) {}
 
-void MCStreamer::emitCFISections(bool EH, bool Debug, bool SFrame) {}
+void MCStreamer::emitCFISections(bool EH, bool Debug) {}
 
 void MCStreamer::emitCFIStartProc(bool IsSimple, SMLoc Loc) {
   if (!FrameInfoStack.empty() &&
diff --git a/llvm/lib/MC/MCTargetOptionsCommandFlags.cpp b/llvm/lib/MC/MCTargetOptionsCommandFlags.cpp
index ff95ff78fd53a..2adc29172f9dd 100644
--- a/llvm/lib/MC/MCTargetOptionsCommandFlags.cpp
+++ b/llvm/lib/MC/MCTargetOptionsCommandFlags.cpp
@@ -41,7 +41,6 @@ MCOPT(int, DwarfVersion)
 MCOPT(bool, Dwarf64)
 MCOPT(EmitDwarfUnwindType, EmitDwarfUnwind)
 MCOPT(bool, EmitCompactUnwindNonCanonical)
-MCOPT(bool, EmitSFrameUnwind)
 MCOPT(bool, ShowMCInst)
 MCOPT(bool, FatalWarnings)
 MCOPT(bool, NoWarn)
@@ -106,11 +105,6 @@ llvm::mc::RegisterMCTargetOptionsFlags::RegisterMCTargetOptionsFlags() {
           false)); // By default, use DWARF for non-canonical personalities.
   MCBINDOPT(EmitCompactUnwindNonCanonical);
 
-  static cl::opt<bool> EmitSFrameUnwind(
-      "gsframe", cl::desc("Whether to emit .sframe unwind sections."),
-      cl::init(false));
-  MCBINDOPT(EmitSFrameUnwind);
-
   static cl::opt<bool> ShowMCInst(
       "asm-show-inst",
       cl::desc("Emit internal instruction representation to assembly file"));
@@ -194,7 +188,6 @@ MCTargetOptions llvm::mc::InitMCTargetOptionsFromFlags() {
   Options.X86Sse2Avx = getX86Sse2Avx();
   Options.EmitDwarfUnwind = getEmitDwarfUnwind();
   Options.EmitCompactUnwindNonCanonical = getEmitCompactUnwindNonCanonical();
-  Options.EmitSFrameUnwind = getEmitSFrameUnwind();
   Options.AsSecureLogFile = getAsSecureLogFile();
 
   return Options;
diff --git a/llvm/test/MC/ELF/AArch64/cfi.s b/llvm/test/MC/ELF/AArch64/cfi.s
index 7047f92bab8fe..6bdf03cc7bb85 100644
--- a/llvm/test/MC/ELF/AArch64/cfi.s
+++ b/llvm/test/MC/ELF/AArch64/cfi.s
@@ -557,14 +557,12 @@ f37:
 // CHECK-NEXT:  }
 
 .ifdef ERR
-// ERR: [[#@LINE+1]]:15: error: expected .eh_frame, .debug_frame, or .sframe
+// ERR: [[#@LINE+1]]:15: error: expected .eh_frame or .debug_frame
 .cfi_sections $
 // ERR: [[#@LINE+1]]:28: error: expected comma
 .cfi_sections .debug_frame $
 // ERR: [[#@LINE+1]]:39: error: expected comma
 .cfi_sections .debug_frame, .eh_frame $
-// ERR: [[#@LINE+1]]:48: error: expected comma
-.cfi_sections .debug_frame, .eh_frame, .sframe $
 
 // ERR: [[#@LINE+1]]:16: error: unexpected token
 .cfi_startproc $
diff --git a/llvm/test/MC/ELF/cfi.s b/llvm/test/MC/ELF/cfi.s
index b7f937104dbf2..3bd16aec4613e 100644
--- a/llvm/test/MC/ELF/cfi.s
+++ b/llvm/test/MC/ELF/cfi.s
@@ -445,14 +445,12 @@ f37:
 // CHECK:        }
 
 .ifdef ERR
-// ERR: [[#@LINE+1]]:15: error: expected .eh_frame, .debug_frame, or .sframe
+// ERR: [[#@LINE+1]]:15: error: expected .eh_frame or .debug_frame
 .cfi_sections $
 // ERR: [[#@LINE+1]]:28: error: expected comma
 .cfi_sections .debug_frame $
 // ERR: [[#@LINE+1]]:39: error: expected comma
 .cfi_sections .debug_frame, .eh_frame $
-// ERR: [[#@LINE+1]]:48: error: expected comma
-.cfi_sections .debug_frame, .eh_frame, .sframe $
 
 // ERR: [[#@LINE+1]]:16: error: unexpected token
 .cfi_startproc $

@github-actions
Copy link

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Developer Policy and LLVM Discourse for more information.

@Sterling-Augustine
Copy link
Contributor Author

I cannot reproduce the failure reported by the original change. Please advise on how to do that.

ninja -v check-clang-driver passes just fine.

@Sterling-Augustine Sterling-Augustine merged commit ad36e42 into main Jul 23, 2025
13 checks passed
@Sterling-Augustine Sterling-Augustine deleted the revert-149935-sframe_syntax branch July 23, 2025 21:32
@dwblaikie
Copy link
Collaborator

per https://llvm.org/docs/CodeReview.html#code-review-workflow - please label PRs that aren't intended to be gated on precommit review (ie: are intended to/do pass the "trivial changes" bar) with the skip-precommit-approval label when creating them.

Sterling-Augustine added a commit that referenced this pull request Jul 24, 2025
#150509)

This reverts commit ad36e42, fixing a
single uninitialized bit (which cannot be detected with Address
Sanitizer).

This PR adds support for the llvm-mc command-line flag "--gsframe" and
adds ".sframe" to the legal values passed ".cfi_section". It plumbs the
option through the cfi handling code a fair amount. Code to support
actual section generation follows in a future PR.

These options match the gnu-assembler's support syntax for sframes, on
both the command line and in assembly files.

First in a series of changes that will allow llvm-mc to produce sframe
.cfi sections. For more information about sframes, see
https://sourceware.org/binutils/docs-2.44/sframe-spec.html

and the llvm-RFC here:
https://discourse.llvm.org/t/rfc-adding-sframe-support-to-llvm/86900
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Jul 28, 2025
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Jul 28, 2025
…0316) (llvm#150509)

This reverts commit ad36e42, fixing a
single uninitialized bit (which cannot be detected with Address
Sanitizer).

This PR adds support for the llvm-mc command-line flag "--gsframe" and
adds ".sframe" to the legal values passed ".cfi_section". It plumbs the
option through the cfi handling code a fair amount. Code to support
actual section generation follows in a future PR.

These options match the gnu-assembler's support syntax for sframes, on
both the command line and in assembly files.

First in a series of changes that will allow llvm-mc to produce sframe
.cfi sections. For more information about sframes, see
https://sourceware.org/binutils/docs-2.44/sframe-spec.html

and the llvm-RFC here:
https://discourse.llvm.org/t/rfc-adding-sframe-support-to-llvm/86900
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants