[llvm] Parse CFI instructions to create SFrame FREs (PR #155496)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 28 11:11:24 PDT 2025
https://github.com/Sterling-Augustine updated https://github.com/llvm/llvm-project/pull/155496
>From 6ce089d78dab3c49dc4c6d4168917f1c54b43331 Mon Sep 17 00:00:00 2001
From: Sterling Augustine <saugustine at google.com>
Date: Tue, 12 Aug 2025 16:52:09 -0700
Subject: [PATCH 1/7] Generate skeleton SFrame Function Description Entries.
---
llvm/lib/MC/MCSFrame.cpp | 84 +++++++++++++++++++++++++++++++++--
llvm/test/MC/ELF/cfi-sframe.s | 12 +++--
2 files changed, 90 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/MC/MCSFrame.cpp b/llvm/lib/MC/MCSFrame.cpp
index 447f22eaaf17e..043f2adf91911 100644
--- a/llvm/lib/MC/MCSFrame.cpp
+++ b/llvm/lib/MC/MCSFrame.cpp
@@ -9,6 +9,7 @@
#include "llvm/MC/MCSFrame.h"
#include "llvm/BinaryFormat/SFrame.h"
#include "llvm/MC/MCContext.h"
+#include "llvm/MC/MCAsmInfo.h"
#include "llvm/MC/MCObjectFileInfo.h"
#include "llvm/MC/MCObjectStreamer.h"
#include "llvm/MC/MCSection.h"
@@ -20,12 +21,62 @@ using namespace sframe;
namespace {
+// High-level structure to track info needed to emit a sframe_func_desc_entry
+// and its associated FREs.
+struct SFrameFDE {
+ // Reference to the original dwarf frame to avoid copying.
+ const MCDwarfFrameInfo &DFrame;
+ // Label where this FDE's FREs start.
+ MCSymbol *FREStart;
+
+ SFrameFDE(const MCDwarfFrameInfo &DF, MCSymbol *FRES)
+ : DFrame(DF), FREStart(FRES) {}
+
+ void emit(MCObjectStreamer &S, const MCSymbol* FRESubSectionStart) {
+ MCContext &C = S.getContext();
+
+ // sfde_func_start_address
+ const MCExpr *V = C.getAsmInfo()->getExprForFDESymbol(
+ &(*DFrame.Begin), C.getObjectFileInfo()->getFDEEncoding(), S);
+ S.emitValue(V, sizeof(int32_t));
+
+ // sfde_func_size
+ S.emitAbsoluteSymbolDiff(DFrame.End, DFrame.Begin, sizeof(uint32_t));
+
+ // sfde_func_start_fre_off
+ auto *F = S.getCurrentFragment();
+ const MCExpr *Diff =
+ MCBinaryExpr::createSub(MCSymbolRefExpr::create(FREStart, C),
+ MCSymbolRefExpr::create(FRESubSectionStart, C), C);
+
+ F->addFixup(MCFixup::create(F->getContents().size(), Diff,
+ MCFixup::getDataKindForSize(4)));
+ S.emitInt32(0);
+
+ // sfde_func_start_num_fres
+ S.emitInt32(0);
+
+ // sfde_func_info word
+ FDEInfo I;
+ I.setFuncInfo(0 /* No pauth key */, FDEType::PCInc, FREType::Addr1);
+ S.emitInt8(I.Info);
+
+ // sfde_func_rep_size. Not relevant in non-PCMASK fdes.
+ S.emitInt8(0);
+
+ // sfde_func_padding2
+ S.emitInt16(0);
+ }
+};
+
// Emitting these field-by-field, instead of constructing the actual structures
// lets Streamer do target endian-fixups for free.
class SFrameEmitterImpl {
MCObjectStreamer &Streamer;
+ SmallVector<SFrameFDE> FDEs;
ABI SFrameABI;
+
MCSymbol *FDESubSectionStart;
MCSymbol *FRESubSectionStart;
MCSymbol *FRESubSectionEnd;
@@ -36,16 +87,22 @@ class SFrameEmitterImpl {
.getObjectFileInfo()
->getSFrameABIArch()
.has_value());
+ FDEs.reserve(Streamer.getDwarfFrameInfos().size());
SFrameABI = *Streamer.getContext().getObjectFileInfo()->getSFrameABIArch();
+
FDESubSectionStart = Streamer.getContext().createTempSymbol();
FRESubSectionStart = Streamer.getContext().createTempSymbol();
FRESubSectionEnd = Streamer.getContext().createTempSymbol();
}
+ void BuildSFDE(const MCDwarfFrameInfo &DF) {
+ FDEs.emplace_back(DF, Streamer.getContext().createTempSymbol());
+ }
+
void emitPreamble() {
Streamer.emitInt16(Magic);
Streamer.emitInt8(static_cast<uint8_t>(Version::V2));
- Streamer.emitInt8(0);
+ Streamer.emitInt8(static_cast<uint8_t>(Flags::FDEFuncStartPCRel));
}
void emitHeader() {
@@ -59,9 +116,11 @@ class SFrameEmitterImpl {
// sfh_auxhdr_len
Streamer.emitInt8(0);
// shf_num_fdes
- Streamer.emitInt32(0);
+ Streamer.emitInt32(FDEs.size());
+
// shf_num_fres
Streamer.emitInt32(0);
+
// shf_fre_len
Streamer.emitAbsoluteSymbolDiff(FRESubSectionEnd, FRESubSectionStart,
sizeof(int32_t));
@@ -72,10 +131,17 @@ class SFrameEmitterImpl {
sizeof(uint32_t));
}
- void emitFDEs() { Streamer.emitLabel(FDESubSectionStart); }
+ void emitFDEs() {
+ Streamer.emitLabel(FDESubSectionStart);
+ for (auto &FDE : FDEs) {
+ FDE.emit(Streamer, FRESubSectionStart);
+ }
+ }
void emitFREs() {
Streamer.emitLabel(FRESubSectionStart);
+ for (auto FDE : FDEs)
+ Streamer.emitLabel(FDE.FREStart);
Streamer.emitLabel(FRESubSectionEnd);
}
};
@@ -84,7 +150,19 @@ class SFrameEmitterImpl {
void MCSFrameEmitter::emit(MCObjectStreamer &Streamer) {
MCContext &Context = Streamer.getContext();
+ // If this target doesn't support sframes, return now. Gas doesn't warn in
+ // this case, but if we want to, it should be done at option-parsing time,
+ // rather than here.
+ if (!Streamer.getContext().getObjectFileInfo()->getSFrameABIArch().has_value())
+ return;
+
SFrameEmitterImpl Emitter(Streamer);
+ ArrayRef<MCDwarfFrameInfo> FrameArray = Streamer.getDwarfFrameInfos();
+
+ // Both the header itself and the FDEs include various offsets and counts.
+ // Therefore, all of this must be precomputed.
+ for (const auto& DFrame : FrameArray)
+ Emitter.BuildSFDE(DFrame);
MCSection *Section = Context.getObjectFileInfo()->getSFrameSection();
// Not strictly necessary, but gas always aligns to 8, so match that.
diff --git a/llvm/test/MC/ELF/cfi-sframe.s b/llvm/test/MC/ELF/cfi-sframe.s
index 45fce241c8764..f6e3dfbd4548d 100644
--- a/llvm/test/MC/ELF/cfi-sframe.s
+++ b/llvm/test/MC/ELF/cfi-sframe.s
@@ -10,17 +10,23 @@ f1:
nop
.cfi_endproc
+f2:
+ .cfi_startproc
+ nop
+ nop
+ .cfi_endproc
+
// CHECK: SFrame section '.sframe' {
// CHECK-NEXT: Header {
// CHECK-NEXT: Magic: 0xDEE2
// CHECK-NEXT: Version: V2 (0x2)
-// CHECK-NEXT: Flags [ (0x0)
+// CHECK-NEXT: Flags [ (0x4)
// CHECK: ABI: AMD64EndianLittle (0x3)
// CHECK-NEXT: CFA fixed FP offset (unused): 0
// CHECK-NEXT: CFA fixed RA offset: 0
// CHECK-NEXT: Auxiliary header length: 0
-// CHECK-NEXT: Num FDEs: 0
+// CHECK-NEXT: Num FDEs: 2
// CHECK-NEXT: Num FREs: 0
// CHECK-NEXT: FRE subsection length: 0
// CHECK-NEXT: FDE subsection offset: 0
-// CHECK-NEXT: FRE subsection offset: 0
+// CHECK-NEXT: FRE subsection offset: 40
>From 67b92059f3d01a3e84478a335a177cf935b4c016 Mon Sep 17 00:00:00 2001
From: Sterling Augustine <saugustine at google.com>
Date: Mon, 18 Aug 2025 15:18:50 -0700
Subject: [PATCH 2/7] Fix formatting
---
llvm/lib/MC/MCSFrame.cpp | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/llvm/lib/MC/MCSFrame.cpp b/llvm/lib/MC/MCSFrame.cpp
index 043f2adf91911..45d407eecf04b 100644
--- a/llvm/lib/MC/MCSFrame.cpp
+++ b/llvm/lib/MC/MCSFrame.cpp
@@ -8,8 +8,8 @@
#include "llvm/MC/MCSFrame.h"
#include "llvm/BinaryFormat/SFrame.h"
-#include "llvm/MC/MCContext.h"
#include "llvm/MC/MCAsmInfo.h"
+#include "llvm/MC/MCContext.h"
#include "llvm/MC/MCObjectFileInfo.h"
#include "llvm/MC/MCObjectStreamer.h"
#include "llvm/MC/MCSection.h"
@@ -32,7 +32,7 @@ struct SFrameFDE {
SFrameFDE(const MCDwarfFrameInfo &DF, MCSymbol *FRES)
: DFrame(DF), FREStart(FRES) {}
- void emit(MCObjectStreamer &S, const MCSymbol* FRESubSectionStart) {
+ void emit(MCObjectStreamer &S, const MCSymbol *FRESubSectionStart) {
MCContext &C = S.getContext();
// sfde_func_start_address
@@ -45,9 +45,9 @@ struct SFrameFDE {
// sfde_func_start_fre_off
auto *F = S.getCurrentFragment();
- const MCExpr *Diff =
- MCBinaryExpr::createSub(MCSymbolRefExpr::create(FREStart, C),
- MCSymbolRefExpr::create(FRESubSectionStart, C), C);
+ const MCExpr *Diff = MCBinaryExpr::createSub(
+ MCSymbolRefExpr::create(FREStart, C),
+ MCSymbolRefExpr::create(FRESubSectionStart, C), C);
F->addFixup(MCFixup::create(F->getContents().size(), Diff,
MCFixup::getDataKindForSize(4)));
@@ -57,7 +57,7 @@ struct SFrameFDE {
S.emitInt32(0);
// sfde_func_info word
- FDEInfo I;
+ FDEInfo<endianness::native> I;
I.setFuncInfo(0 /* No pauth key */, FDEType::PCInc, FREType::Addr1);
S.emitInt8(I.Info);
@@ -76,7 +76,6 @@ class SFrameEmitterImpl {
MCObjectStreamer &Streamer;
SmallVector<SFrameFDE> FDEs;
ABI SFrameABI;
-
MCSymbol *FDESubSectionStart;
MCSymbol *FRESubSectionStart;
MCSymbol *FRESubSectionEnd;
@@ -89,7 +88,6 @@ class SFrameEmitterImpl {
.has_value());
FDEs.reserve(Streamer.getDwarfFrameInfos().size());
SFrameABI = *Streamer.getContext().getObjectFileInfo()->getSFrameABIArch();
-
FDESubSectionStart = Streamer.getContext().createTempSymbol();
FRESubSectionStart = Streamer.getContext().createTempSymbol();
FRESubSectionEnd = Streamer.getContext().createTempSymbol();
@@ -117,10 +115,8 @@ class SFrameEmitterImpl {
Streamer.emitInt8(0);
// shf_num_fdes
Streamer.emitInt32(FDEs.size());
-
// shf_num_fres
Streamer.emitInt32(0);
-
// shf_fre_len
Streamer.emitAbsoluteSymbolDiff(FRESubSectionEnd, FRESubSectionStart,
sizeof(int32_t));
@@ -153,7 +149,10 @@ void MCSFrameEmitter::emit(MCObjectStreamer &Streamer) {
// If this target doesn't support sframes, return now. Gas doesn't warn in
// this case, but if we want to, it should be done at option-parsing time,
// rather than here.
- if (!Streamer.getContext().getObjectFileInfo()->getSFrameABIArch().has_value())
+ if (!Streamer.getContext()
+ .getObjectFileInfo()
+ ->getSFrameABIArch()
+ .has_value())
return;
SFrameEmitterImpl Emitter(Streamer);
@@ -161,7 +160,7 @@ void MCSFrameEmitter::emit(MCObjectStreamer &Streamer) {
// Both the header itself and the FDEs include various offsets and counts.
// Therefore, all of this must be precomputed.
- for (const auto& DFrame : FrameArray)
+ for (const auto &DFrame : FrameArray)
Emitter.BuildSFDE(DFrame);
MCSection *Section = Context.getObjectFileInfo()->getSFrameSection();
>From f5b9edd923e88addd2808305d7850f4c3050c80b Mon Sep 17 00:00:00 2001
From: Sterling Augustine <saugustine at google.com>
Date: Wed, 20 Aug 2025 14:37:36 -0700
Subject: [PATCH 3/7] Generate FREs from .cfi directives.
---
llvm/lib/MC/MCSFrame.cpp | 229 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 225 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/MC/MCSFrame.cpp b/llvm/lib/MC/MCSFrame.cpp
index 45d407eecf04b..0536b6ce09e4c 100644
--- a/llvm/lib/MC/MCSFrame.cpp
+++ b/llvm/lib/MC/MCSFrame.cpp
@@ -21,6 +21,24 @@ using namespace sframe;
namespace {
+// High-level structure to track info needed to emit a
+// sframe_frame_row_entry_addrX. On disk these have both a fixed portion of type
+// sframe_frame_row_entry_addrX and trailing data of X * S bytes, where X is the
+// datum size, and S is 1, 2, or 3 depending on which of Cfa, SP, and FP are
+// being tracked.
+struct SFrameFRE {
+ // An FRE describes how to find the registers when the PC is at this
+ // Label from function start.
+ const MCSymbol *Label = nullptr;
+ size_t CfaOffset = 0;
+ size_t FPOffset = 0;
+ size_t RAOffset = 0;
+ bool FromFP = false;
+ bool CfaRegSet = false;
+
+ SFrameFRE(const MCSymbol *Start) : Label(Start) {}
+};
+
// High-level structure to track info needed to emit a sframe_func_desc_entry
// and its associated FREs.
struct SFrameFDE {
@@ -28,9 +46,13 @@ struct SFrameFDE {
const MCDwarfFrameInfo &DFrame;
// Label where this FDE's FREs start.
MCSymbol *FREStart;
+ // True when unwind info can't be described with an Sframe FDE.
+ bool Invalid;
+ // Unwinding fres
+ SmallVector<SFrameFRE> FREs;
SFrameFDE(const MCDwarfFrameInfo &DF, MCSymbol *FRES)
- : DFrame(DF), FREStart(FRES) {}
+ : DFrame(DF), FREStart(FRES), Invalid(false) {}
void emit(MCObjectStreamer &S, const MCSymbol *FRESubSectionStart) {
MCContext &C = S.getContext();
@@ -54,7 +76,7 @@ struct SFrameFDE {
S.emitInt32(0);
// sfde_func_start_num_fres
- S.emitInt32(0);
+ S.emitInt32(FREs.size());
// sfde_func_info word
FDEInfo<endianness::native> I;
@@ -76,10 +98,127 @@ class SFrameEmitterImpl {
MCObjectStreamer &Streamer;
SmallVector<SFrameFDE> FDEs;
ABI SFrameABI;
+ // Target-specific convenience variables to detect when a CFI instruction
+ // references these registers. Unlike in dwarf frame descriptions, they never
+ // escape into the sframe section itself.
+ unsigned SPReg;
+ unsigned FPReg;
+ unsigned RAReg;
MCSymbol *FDESubSectionStart;
MCSymbol *FRESubSectionStart;
MCSymbol *FRESubSectionEnd;
+
+ bool setCfaRegister(SFrameFDE &FDE, SFrameFRE &FRE, const MCCFIInstruction &I) {
+ if (I.getRegister() == SPReg) {
+ FRE.CfaRegSet = true;
+ FRE.FromFP = false;
+ return true;
+ } else if (I.getRegister() == FPReg) {
+ FRE.CfaRegSet = true;
+ FRE.FromFP = true;
+ return true;
+ }
+ Streamer.getContext().reportWarning(
+ I.getLoc(), "Canonical Frame Address not in stack- or frame-pointer. "
+ "Omitting SFrame unwind info.");
+ FDE.Invalid = true;
+ return false;
+ }
+
+ bool isCfaRegisterSet(SFrameFDE &FDE, SFrameFRE &FRE,
+ const MCCFIInstruction &I) {
+ if (FRE.CfaRegSet)
+ return true;
+
+ Streamer.getContext().reportWarning(
+ I.getLoc(), "skipping SFrame FDE; .cfi_def_cfa_offset "
+ "without CFA base register in effect");
+ FDE.Invalid = true;
+ return false;
+ }
+
+ // Add the effects of CFI to the current FDE, creating a new FRE when
+ // necessary.
+ void handleCFI(SFrameFDE &FDE, SFrameFRE &FRE, const MCCFIInstruction &CFI) {
+ // Return on error or uninteresting CFI.
+ switch (CFI.getOperation()) {
+ case MCCFIInstruction::OpDefCfaRegister:
+ if (!setCfaRegister(FDE, FRE, CFI))
+ return;
+ break;
+ case MCCFIInstruction::OpDefCfa:
+ case MCCFIInstruction::OpLLVMDefAspaceCfa:
+ if (!setCfaRegister(FDE, FRE, CFI))
+ return;
+ FRE.CfaOffset = CFI.getOffset();
+ break;
+ case MCCFIInstruction::OpOffset:
+ if (CFI.getRegister() == FPReg)
+ FRE.FPOffset = CFI.getOffset();
+ else if (CFI.getRegister() == RAReg)
+ FRE.RAOffset = CFI.getOffset();
+ else
+ return; // uninteresting register.
+ break;
+ case MCCFIInstruction::OpRelOffset:
+ if (CFI.getRegister() == FPReg)
+ FRE.FPOffset += CFI.getOffset();
+ else if (CFI.getRegister() == RAReg)
+ FRE.RAOffset += CFI.getOffset();
+ else
+ return; // uninteresting register.
+ break;
+ case MCCFIInstruction::OpDefCfaOffset:
+ if (!isCfaRegisterSet(FDE, FRE, CFI))
+ return;
+ FRE.CfaOffset = CFI.getOffset();
+ break;
+ case MCCFIInstruction::OpAdjustCfaOffset:
+ if (!isCfaRegisterSet(FDE, FRE, CFI))
+ return;
+ FRE.CfaOffset += CFI.getOffset();
+ break;
+ case MCCFIInstruction::OpRememberState:
+ if (FDE.FREs.size() == 1) {
+ // Error for gas compatibility: If the initial FRE isn't complete,
+ // then any state is incomplete. FIXME: Dwarf doesn't error here.
+ // Why should sframe?
+ Streamer.getContext().reportWarning(
+ CFI.getLoc(), "skipping SFrame FDE; .cfi_remember_state without "
+ "prior SFrame FRE state");
+ FDE.Invalid = true;
+ return;
+ }
+ FDE.SaveState.push_back(FRE);
+ return;
+ break;
+ case MCCFIInstruction::OpRestore:
+ // The first FRE generated has the original state.
+ if (CFI.getRegister() == FPReg)
+ FRE.FPOffset = FDE.FREs.front().FPOffset;
+ else if (CFI.getRegister() == RAReg)
+ FRE.RAOffset = FDE.FREs.front().RAOffset;
+ return; // Any other register is uninteresting.
+ case MCCFIInstruction::OpRestoreState:
+ // The cfi parser will have caught unbalanced directives earlier, so a
+ // mismatch here is an implementation error.
+ assert(!FDE.SaveState.empty() &&
+ "cfi_restore_state without cfi_save_state");
+ FRE = FDE.SaveState.back();
+ FDE.SaveState.pop_back();
+ return;
+ case MCCFIInstruction::OpEscape:
+ // TODO: Implement
+ break;
+ default:
+ // Instructions that don't affect the Cfa, RA, and SP can be safely
+ // ignored.
+ return;
+ }
+ }
+
+
public:
SFrameEmitterImpl(MCObjectStreamer &Streamer) : Streamer(Streamer) {
assert(Streamer.getContext()
@@ -88,13 +227,91 @@ class SFrameEmitterImpl {
.has_value());
FDEs.reserve(Streamer.getDwarfFrameInfos().size());
SFrameABI = *Streamer.getContext().getObjectFileInfo()->getSFrameABIArch();
+ switch (SFrameABI) {
+ case ABI::AArch64EndianBig:
+ case ABI::AArch64EndianLittle:
+ SPReg = 31;
+ RAReg = 29;
+ FPReg = 30;
+ break;
+ case ABI::AMD64EndianLittle:
+ SPReg = 7;
+ // RARegister untracked in this abi. Value chosen to match
+ // MCDwarfFrameInfo constructor.
+ RAReg = static_cast<unsigned>(INT_MAX);
+ FPReg = 6;
+ break;
+ }
+
FDESubSectionStart = Streamer.getContext().createTempSymbol();
FRESubSectionStart = Streamer.getContext().createTempSymbol();
FRESubSectionEnd = Streamer.getContext().createTempSymbol();
}
- void BuildSFDE(const MCDwarfFrameInfo &DF) {
+ bool atSameLocation(const MCSymbol *Left, const MCSymbol *Right) {
+ return Left != nullptr && Right != nullptr &&
+ Left->getFragment() == Right->getFragment() &&
+ Left->getOffset() == Right->getOffset();
+ }
+
+ bool equalIgnoringLocation(const SFrameFRE &Left, const SFrameFRE &Right) {
+ return Left.CfaOffset == Right.CfaOffset &&
+ Left.FPOffset == Right.FPOffset && Left.RAOffset == Right.RAOffset &&
+ Left.FromFP == Right.FromFP && Left.CfaRegSet == Right.CfaRegSet;
+ }
+
+ void buildSFDE(const MCDwarfFrameInfo &DF) {
FDEs.emplace_back(DF, Streamer.getContext().createTempSymbol());
+ // This would have been set via ".cfi_return_column", but
+ // MCObjectStreamer doesn't emit an MCCFIInstruction for that. It just
+ // sets the DF.RAReg.
+ // FIXME: This also prevents providing a proper location for the error.
+ // LLVM doesn't change the return column itself, so this was
+ // externally-generated assembly.
+ if (DF.RAReg != RAReg) {
+ Streamer.getContext().reportWarning(
+ SMLoc(),
+ "skipping SFrame FDE; non-default RA register " + Twine(DF.RAReg));
+ // Continue with the FDE to find any addtional errors. Discard it at
+ // the end.
+ FDE.Invalid = true;
+ }
+ MCSymbol *BaseLabel = DF.Begin;
+ SFrameFRE BaseFRE(BaseLabel);
+ if (!DF.IsSimple) {
+ for (const auto &CFI :
+ Streamer.getContext().getAsmInfo()->getInitialFrameState())
+ HandleCFI(FDE, BaseFRE, CFI);
+ }
+ FDE.FREs.push_back(BaseFRE);
+
+ for (const auto &CFI : DF.Instructions) {
+ // Instructions from InitialFrameState may not have a label, but if
+ // these instructions don't, then they are in dead code or otherwise
+ // unused.
+ auto *L = CFI.getLabel();
+ if (L && !L->isDefined())
+ continue;
+
+ SFrameFRE FRE = FDE.FREs.back();
+ HandleCFI(FDE, FRE, CFI);
+
+ // If nothing relevant but the location changed, don't add the FRE.
+ if (EqualIgnoringLocation(FRE, FDE.FREs.back()))
+ continue;
+
+ // If the location stayed the same, then update the current
+ // row. Otherwise, add a new one.
+ if (atSameLocation(BaseLabel, L))
+ FDE.FREs.back() = FRE;
+ else {
+ FDE.FREs.push_back(FRE);
+ FDE.FREs.back().Label = L;
+ BaseLabel = L;
+ }
+ }
+ if (FDE.Invalid)
+ FDEs.pop_back();
}
void emitPreamble() {
@@ -116,7 +333,11 @@ class SFrameEmitterImpl {
// shf_num_fdes
Streamer.emitInt32(FDEs.size());
// shf_num_fres
- Streamer.emitInt32(0);
+ uint32_t TotalFREs = 0;
+ for (auto &FDE : FDEs)
+ TotalFREs += FDE.FREs.size();
+ Streamer.emitInt32(TotalFREs);
+
// shf_fre_len
Streamer.emitAbsoluteSymbolDiff(FRESubSectionEnd, FRESubSectionStart,
sizeof(int32_t));
>From d2b2503954a3104b461f8d29b2403c047c1f220f Mon Sep 17 00:00:00 2001
From: Sterling Augustine <saugustine at google.com>
Date: Wed, 20 Aug 2025 15:21:10 -0700
Subject: [PATCH 4/7] Checkpoint
---
llvm/lib/MC/MCSFrame.cpp | 60 ++++++++++++----------------------------
1 file changed, 17 insertions(+), 43 deletions(-)
diff --git a/llvm/lib/MC/MCSFrame.cpp b/llvm/lib/MC/MCSFrame.cpp
index 0536b6ce09e4c..27ae9c2b69089 100644
--- a/llvm/lib/MC/MCSFrame.cpp
+++ b/llvm/lib/MC/MCSFrame.cpp
@@ -141,76 +141,50 @@ class SFrameEmitterImpl {
// Add the effects of CFI to the current FDE, creating a new FRE when
// necessary.
void handleCFI(SFrameFDE &FDE, SFrameFRE &FRE, const MCCFIInstruction &CFI) {
- // Return on error or uninteresting CFI.
switch (CFI.getOperation()) {
case MCCFIInstruction::OpDefCfaRegister:
- if (!setCfaRegister(FDE, FRE, CFI))
- return;
- break;
+ setCfaRegister(FDE, FRE, CFI);
+ return;
case MCCFIInstruction::OpDefCfa:
case MCCFIInstruction::OpLLVMDefAspaceCfa:
if (!setCfaRegister(FDE, FRE, CFI))
return;
FRE.CfaOffset = CFI.getOffset();
- break;
+ return;
case MCCFIInstruction::OpOffset:
if (CFI.getRegister() == FPReg)
FRE.FPOffset = CFI.getOffset();
else if (CFI.getRegister() == RAReg)
FRE.RAOffset = CFI.getOffset();
- else
- return; // uninteresting register.
- break;
+ return;
case MCCFIInstruction::OpRelOffset:
if (CFI.getRegister() == FPReg)
FRE.FPOffset += CFI.getOffset();
else if (CFI.getRegister() == RAReg)
FRE.RAOffset += CFI.getOffset();
- else
- return; // uninteresting register.
- break;
+ return;
case MCCFIInstruction::OpDefCfaOffset:
if (!isCfaRegisterSet(FDE, FRE, CFI))
return;
FRE.CfaOffset = CFI.getOffset();
- break;
+ return;
case MCCFIInstruction::OpAdjustCfaOffset:
if (!isCfaRegisterSet(FDE, FRE, CFI))
return;
FRE.CfaOffset += CFI.getOffset();
- break;
+ return;
case MCCFIInstruction::OpRememberState:
- if (FDE.FREs.size() == 1) {
- // Error for gas compatibility: If the initial FRE isn't complete,
- // then any state is incomplete. FIXME: Dwarf doesn't error here.
- // Why should sframe?
- Streamer.getContext().reportWarning(
- CFI.getLoc(), "skipping SFrame FDE; .cfi_remember_state without "
- "prior SFrame FRE state");
- FDE.Invalid = true;
- return;
- }
- FDE.SaveState.push_back(FRE);
+ // TODO: Implement
return;
- break;
case MCCFIInstruction::OpRestore:
- // The first FRE generated has the original state.
- if (CFI.getRegister() == FPReg)
- FRE.FPOffset = FDE.FREs.front().FPOffset;
- else if (CFI.getRegister() == RAReg)
- FRE.RAOffset = FDE.FREs.front().RAOffset;
- return; // Any other register is uninteresting.
+ // TODO: Implement
+ return;
case MCCFIInstruction::OpRestoreState:
- // The cfi parser will have caught unbalanced directives earlier, so a
- // mismatch here is an implementation error.
- assert(!FDE.SaveState.empty() &&
- "cfi_restore_state without cfi_save_state");
- FRE = FDE.SaveState.back();
- FDE.SaveState.pop_back();
+ // TODO: Implement
return;
case MCCFIInstruction::OpEscape:
// TODO: Implement
- break;
+ return;
default:
// Instructions that don't affect the Cfa, RA, and SP can be safely
// ignored.
@@ -261,7 +235,7 @@ class SFrameEmitterImpl {
}
void buildSFDE(const MCDwarfFrameInfo &DF) {
- FDEs.emplace_back(DF, Streamer.getContext().createTempSymbol());
+ auto &FDE = FDEs.emplace_back(DF, Streamer.getContext().createTempSymbol());
// This would have been set via ".cfi_return_column", but
// MCObjectStreamer doesn't emit an MCCFIInstruction for that. It just
// sets the DF.RAReg.
@@ -281,7 +255,7 @@ class SFrameEmitterImpl {
if (!DF.IsSimple) {
for (const auto &CFI :
Streamer.getContext().getAsmInfo()->getInitialFrameState())
- HandleCFI(FDE, BaseFRE, CFI);
+ handleCFI(FDE, BaseFRE, CFI);
}
FDE.FREs.push_back(BaseFRE);
@@ -294,10 +268,10 @@ class SFrameEmitterImpl {
continue;
SFrameFRE FRE = FDE.FREs.back();
- HandleCFI(FDE, FRE, CFI);
+ handleCFI(FDE, FRE, CFI);
// If nothing relevant but the location changed, don't add the FRE.
- if (EqualIgnoringLocation(FRE, FDE.FREs.back()))
+ if (equalIgnoringLocation(FRE, FDE.FREs.back()))
continue;
// If the location stayed the same, then update the current
@@ -382,7 +356,7 @@ void MCSFrameEmitter::emit(MCObjectStreamer &Streamer) {
// Both the header itself and the FDEs include various offsets and counts.
// Therefore, all of this must be precomputed.
for (const auto &DFrame : FrameArray)
- Emitter.BuildSFDE(DFrame);
+ Emitter.buildSFDE(DFrame);
MCSection *Section = Context.getObjectFileInfo()->getSFrameSection();
// Not strictly necessary, but gas always aligns to 8, so match that.
>From 8b54030b92e81700038848744b629488c19eeec4 Mon Sep 17 00:00:00 2001
From: Sterling Augustine <saugustine at google.com>
Date: Tue, 26 Aug 2025 13:27:20 -0700
Subject: [PATCH 5/7] Clean up some details
---
llvm/lib/MC/MCSFrame.cpp | 21 +++++++++--------
llvm/test/MC/ELF/cfi-sframe-errors.s | 35 ++++++++++++++++++++++++++++
llvm/test/MC/ELF/cfi-sframe.s | 20 ++++++++++++----
3 files changed, 61 insertions(+), 15 deletions(-)
create mode 100644 llvm/test/MC/ELF/cfi-sframe-errors.s
diff --git a/llvm/lib/MC/MCSFrame.cpp b/llvm/lib/MC/MCSFrame.cpp
index 75483af5fd905..cbaf128b6110d 100644
--- a/llvm/lib/MC/MCSFrame.cpp
+++ b/llvm/lib/MC/MCSFrame.cpp
@@ -75,8 +75,9 @@ struct SFrameFDE {
MCFixup::getDataKindForSize(4)));
S.emitInt32(0);
- // sfde_func_start_num_fres
- S.emitInt32(FREs.size());
+ // sfde_func_num_fres
+ // TODO: When we actually emit fres, replace 0 with FREs.size()
+ S.emitInt32(0);
// sfde_func_info word
FDEInfo<endianness::native> I;
@@ -121,7 +122,7 @@ class SFrameEmitterImpl {
}
Streamer.getContext().reportWarning(
I.getLoc(), "Canonical Frame Address not in stack- or frame-pointer. "
- "Omitting SFrame unwind info.");
+ "Omitting SFrame unwind info for this function.");
FDE.Invalid = true;
return false;
}
@@ -132,8 +133,8 @@ class SFrameEmitterImpl {
return true;
Streamer.getContext().reportWarning(
- I.getLoc(), "skipping SFrame FDE; .cfi_def_cfa_offset "
- "without CFA base register in effect");
+ I.getLoc(), "Adjusting CFA offset without a base register. "
+ "Omitting SFrame unwind info for this function.");
FDE.Invalid = true;
return false;
}
@@ -241,11 +242,11 @@ class SFrameEmitterImpl {
// sets the DF.RAReg.
// FIXME: This also prevents providing a proper location for the error.
// LLVM doesn't change the return column itself, so this was
- // externally-generated assembly.
+ // hand-written assembly.
if (DF.RAReg != RAReg) {
Streamer.getContext().reportWarning(
- SMLoc(),
- "skipping SFrame FDE; non-default RA register " + Twine(DF.RAReg));
+ SMLoc(), "Non-default RA register " + Twine(DF.RAReg) +
+ ". Omitting SFrame unwind info for this function.");
// Continue with the FDE to find any addtional errors. Discard it at
// the end.
FDE.Invalid = true;
@@ -308,8 +309,8 @@ class SFrameEmitterImpl {
Streamer.emitInt32(FDEs.size());
// shf_num_fres
uint32_t TotalFREs = 0;
- for (auto &FDE : FDEs)
- TotalFREs += FDE.FREs.size();
+ // for (auto &FDE : FDEs)
+ // TotalFREs += FDE.FREs.size();
Streamer.emitInt32(TotalFREs);
// shf_fre_len
diff --git a/llvm/test/MC/ELF/cfi-sframe-errors.s b/llvm/test/MC/ELF/cfi-sframe-errors.s
new file mode 100644
index 0000000000000..65904ed08d5fa
--- /dev/null
+++ b/llvm/test/MC/ELF/cfi-sframe-errors.s
@@ -0,0 +1,35 @@
+// TODO: Add other architectures as they gain sframe support
+// REQUIRES: x86-registered-target
+// RUN: llvm-mc --assemble --filetype=obj --gsframe -triple x86_64 %s -o %t.o 2>&1 | FileCheck %s
+// RUN: llvm-readelf --sframe %t.o | FileCheck --check-prefix=CHECK-NOFDES %s
+
+
+ .cfi_sections .sframe
+f1:
+ .cfi_startproc simple
+ nop
+ .cfi_def_cfa_offset 16 // No base register yet
+ nop
+ .cfi_adjust_cfa_offset 16 // No base register yet
+ nop
+ .cfi_return_column 0
+ nop
+ .cfi_endproc
+
+f2:
+ .cfi_startproc
+ nop
+ .cfi_def_cfa 0, 4
+ nop
+
+ .cfi_endproc
+
+// CHECK: Non-default RA register {{.*}}
+// asm parser doesn't give a location with .cfi_def_cfa_offset
+// CHECK: :0:{{.*}} Adjusting CFA offset without a base register.{{.*}}
+// .cfi_adjust_cfa_offset
+// CHECK: :13:{{.*}} Adjusting CFA offset without a base register. {{.*}}
+// CHECK: Canonical Frame Address not in stack- or frame-pointer. {{.*}}
+
+// CHECK-NOFDES: Num FDEs: 0
+// CHECK-NOFDES: Num FREs: 0
diff --git a/llvm/test/MC/ELF/cfi-sframe.s b/llvm/test/MC/ELF/cfi-sframe.s
index 66b9ee61bdaff..d92cf2195085c 100644
--- a/llvm/test/MC/ELF/cfi-sframe.s
+++ b/llvm/test/MC/ELF/cfi-sframe.s
@@ -5,8 +5,18 @@
.cfi_sections .sframe
f1:
- .cfi_startproc
+ .cfi_startproc // FRE 0
+ nop
+ .cfi_def_cfa_offset 16 // FRE 1
+ .cfi_def_cfa_offset 8 // location didn't change. No new FRE, but new offset.
+ nop
+ .cfi_def_cfa_offset 8 // offset didn't change. No new FRE.
nop
+ .cfi_def_cfa_offset 16 // FRE 2. new location, new offset.
+ nop
+ .cfi_register 0, 1 // Uninteresting register. No new FRE.
+ nop
+
.cfi_endproc
f2:
@@ -32,11 +42,11 @@ f2:
// CHECK: Function Index [
// CHECK-NEXT: FuncDescEntry [0] {
// CHECK-NEXT: PC {
-// CHECK-NEXT: Relocation: {{.*}}32{{.*}}
+// CHECK-NEXT: Relocation: {{.*}}PC32{{.*}}
// CHECK-NEXT: Symbol Name: .text
-// CHECK-NEXT: Start Address: 0x0
+// CHECK-NEXT: Start Address: {{.*}}
// CHECK-NEXT: }
-// CHECK-NEXT: Size: 0x1
+// CHECK-NEXT: Size: 0x5
// CHECK-NEXT: Start FRE Offset: 0x0
// CHECK-NEXT: Num FREs: 0
// CHECK-NEXT: Info {
@@ -51,7 +61,7 @@ f2:
// CHECK-NEXT: }
// CHECK-NEXT: FuncDescEntry [1] {
// CHECK-NEXT: PC {
-// CHECK-NEXT: Relocation: R_X86_64_PC32
+// CHECK-NEXT: Relocation: {{.*}}PC32{{.*}}
// CHECK-NEXT: Symbol Name: .text
// CHECK-NEXT: Start Address: {{.*}}
// CHECK-NEXT: }
>From 34151546301914ec8db815d29f943fa72929cc4d Mon Sep 17 00:00:00 2001
From: Sterling Augustine <saugustine at google.com>
Date: Wed, 27 Aug 2025 11:40:47 -0700
Subject: [PATCH 6/7] Address comments.
---
llvm/lib/MC/MCSFrame.cpp | 122 ++++++++++++---------------
llvm/test/MC/ELF/cfi-sframe-errors.s | 20 ++---
llvm/test/MC/ELF/cfi-sframe.s | 2 +-
3 files changed, 64 insertions(+), 80 deletions(-)
diff --git a/llvm/lib/MC/MCSFrame.cpp b/llvm/lib/MC/MCSFrame.cpp
index cbaf128b6110d..f6a1efa5945cd 100644
--- a/llvm/lib/MC/MCSFrame.cpp
+++ b/llvm/lib/MC/MCSFrame.cpp
@@ -24,17 +24,17 @@ namespace {
// High-level structure to track info needed to emit a
// sframe_frame_row_entry_addrX. On disk these have both a fixed portion of type
// sframe_frame_row_entry_addrX and trailing data of X * S bytes, where X is the
-// datum size, and S is 1, 2, or 3 depending on which of Cfa, SP, and FP are
+// datum size, and S is 1, 2, or 3 depending on which of CFA, SP, and FP are
// being tracked.
struct SFrameFRE {
// An FRE describes how to find the registers when the PC is at this
// Label from function start.
const MCSymbol *Label = nullptr;
- size_t CfaOffset = 0;
+ size_t CFAOffset = 0;
size_t FPOffset = 0;
size_t RAOffset = 0;
bool FromFP = false;
- bool CfaRegSet = false;
+ bool CFARegSet = false;
SFrameFRE(const MCSymbol *Start) : Label(Start) {}
};
@@ -46,13 +46,11 @@ struct SFrameFDE {
const MCDwarfFrameInfo &DFrame;
// Label where this FDE's FREs start.
MCSymbol *FREStart;
- // True when unwind info can't be described with an Sframe FDE.
- bool Invalid;
// Unwinding fres
SmallVector<SFrameFRE> FREs;
SFrameFDE(const MCDwarfFrameInfo &DF, MCSymbol *FRES)
- : DFrame(DF), FREStart(FRES), Invalid(false) {}
+ : DFrame(DF), FREStart(FRES) {}
void emit(MCObjectStreamer &S, const MCSymbol *FRESubSectionStart) {
MCContext &C = S.getContext();
@@ -109,91 +107,81 @@ class SFrameEmitterImpl {
MCSymbol *FRESubSectionStart;
MCSymbol *FRESubSectionEnd;
-
- bool setCfaRegister(SFrameFDE &FDE, SFrameFRE &FRE, const MCCFIInstruction &I) {
+ bool setCFARegister(SFrameFRE &FRE, const MCCFIInstruction &I) {
if (I.getRegister() == SPReg) {
- FRE.CfaRegSet = true;
+ FRE.CFARegSet = true;
FRE.FromFP = false;
return true;
- } else if (I.getRegister() == FPReg) {
- FRE.CfaRegSet = true;
+ }
+ if (I.getRegister() == FPReg) {
+ FRE.CFARegSet = true;
FRE.FromFP = true;
return true;
}
Streamer.getContext().reportWarning(
I.getLoc(), "Canonical Frame Address not in stack- or frame-pointer. "
"Omitting SFrame unwind info for this function.");
- FDE.Invalid = true;
return false;
}
- bool isCfaRegisterSet(SFrameFDE &FDE, SFrameFRE &FRE,
- const MCCFIInstruction &I) {
- if (FRE.CfaRegSet)
- return true;
-
- Streamer.getContext().reportWarning(
- I.getLoc(), "Adjusting CFA offset without a base register. "
- "Omitting SFrame unwind info for this function.");
- FDE.Invalid = true;
- return false;
+ bool setCFAOffset(SFrameFRE &FRE, const SMLoc &Loc, size_t Offset) {
+ if (!FRE.CFARegSet) {
+ Streamer.getContext().reportWarning(
+ Loc, "Adjusting CFA offset without a base register. "
+ "Omitting SFrame unwind info for this function.");
+ return false;
+ }
+ FRE.CFAOffset = Offset;
+ return true;
}
// Add the effects of CFI to the current FDE, creating a new FRE when
// necessary.
- void handleCFI(SFrameFDE &FDE, SFrameFRE &FRE, const MCCFIInstruction &CFI) {
+ bool handleCFI(SFrameFDE &FDE, SFrameFRE &FRE, const MCCFIInstruction &CFI) {
switch (CFI.getOperation()) {
case MCCFIInstruction::OpDefCfaRegister:
- setCfaRegister(FDE, FRE, CFI);
- return;
+ return setCFARegister(FRE, CFI);
case MCCFIInstruction::OpDefCfa:
case MCCFIInstruction::OpLLVMDefAspaceCfa:
- if (!setCfaRegister(FDE, FRE, CFI))
- return;
- FRE.CfaOffset = CFI.getOffset();
- return;
+ if (!setCFARegister(FRE, CFI))
+ return false;
+ setCFAOffset(FRE, CFI.getLoc(), CFI.getOffset());
+ return true;
case MCCFIInstruction::OpOffset:
if (CFI.getRegister() == FPReg)
FRE.FPOffset = CFI.getOffset();
else if (CFI.getRegister() == RAReg)
FRE.RAOffset = CFI.getOffset();
- return;
+ return true;
case MCCFIInstruction::OpRelOffset:
if (CFI.getRegister() == FPReg)
FRE.FPOffset += CFI.getOffset();
else if (CFI.getRegister() == RAReg)
FRE.RAOffset += CFI.getOffset();
- return;
+ return true;
case MCCFIInstruction::OpDefCfaOffset:
- if (!isCfaRegisterSet(FDE, FRE, CFI))
- return;
- FRE.CfaOffset = CFI.getOffset();
- return;
+ return setCFAOffset(FRE, CFI.getLoc(), CFI.getOffset());
case MCCFIInstruction::OpAdjustCfaOffset:
- if (!isCfaRegisterSet(FDE, FRE, CFI))
- return;
- FRE.CfaOffset += CFI.getOffset();
- return;
+ return setCFAOffset(FRE, CFI.getLoc(), FRE.CFAOffset + CFI.getOffset());
case MCCFIInstruction::OpRememberState:
- // TODO: Implement
- return;
+ // TODO: Implement. Will use FDE.
+ return true;
case MCCFIInstruction::OpRestore:
- // TODO: Implement
- return;
+ // TODO: Implement. Will use FDE.
+ return true;
case MCCFIInstruction::OpRestoreState:
- // TODO: Implement
- return;
+ // TODO: Implement. Will use FDE.
+ return true;
case MCCFIInstruction::OpEscape:
- // TODO: Implement
- return;
+ // TODO: Implement. Will use FDE.
+ return true;
default:
- // Instructions that don't affect the Cfa, RA, and SP can be safely
+ // Instructions that don't affect the CFA, RA, and SP can be safely
// ignored.
- return;
+ return true;
}
}
-
public:
SFrameEmitterImpl(MCObjectStreamer &Streamer) : Streamer(Streamer) {
assert(Streamer.getContext()
@@ -230,13 +218,14 @@ class SFrameEmitterImpl {
}
bool equalIgnoringLocation(const SFrameFRE &Left, const SFrameFRE &Right) {
- return Left.CfaOffset == Right.CfaOffset &&
+ return Left.CFAOffset == Right.CFAOffset &&
Left.FPOffset == Right.FPOffset && Left.RAOffset == Right.RAOffset &&
- Left.FromFP == Right.FromFP && Left.CfaRegSet == Right.CfaRegSet;
+ Left.FromFP == Right.FromFP && Left.CFARegSet == Right.CFARegSet;
}
void buildSFDE(const MCDwarfFrameInfo &DF) {
- auto &FDE = FDEs.emplace_back(DF, Streamer.getContext().createTempSymbol());
+ bool Valid = true;
+ SFrameFDE FDE(DF, Streamer.getContext().createTempSymbol());
// This would have been set via ".cfi_return_column", but
// MCObjectStreamer doesn't emit an MCCFIInstruction for that. It just
// sets the DF.RAReg.
@@ -245,18 +234,18 @@ class SFrameEmitterImpl {
// hand-written assembly.
if (DF.RAReg != RAReg) {
Streamer.getContext().reportWarning(
- SMLoc(), "Non-default RA register " + Twine(DF.RAReg) +
+ SMLoc(), "Non-default RA register in .cfi_return_column " +
+ Twine(DF.RAReg) +
". Omitting SFrame unwind info for this function.");
- // Continue with the FDE to find any addtional errors. Discard it at
- // the end.
- FDE.Invalid = true;
+ Valid = false;
}
- MCSymbol *BaseLabel = DF.Begin;
- SFrameFRE BaseFRE(BaseLabel);
+ MCSymbol *LastLabel = DF.Begin;
+ SFrameFRE BaseFRE(LastLabel);
if (!DF.IsSimple) {
for (const auto &CFI :
Streamer.getContext().getAsmInfo()->getInitialFrameState())
- handleCFI(FDE, BaseFRE, CFI);
+ if (!handleCFI(FDE, BaseFRE, CFI))
+ Valid = false;
}
FDE.FREs.push_back(BaseFRE);
@@ -269,7 +258,8 @@ class SFrameEmitterImpl {
continue;
SFrameFRE FRE = FDE.FREs.back();
- handleCFI(FDE, FRE, CFI);
+ if (!handleCFI(FDE, FRE, CFI))
+ Valid = false;
// If nothing relevant but the location changed, don't add the FRE.
if (equalIgnoringLocation(FRE, FDE.FREs.back()))
@@ -277,16 +267,16 @@ class SFrameEmitterImpl {
// If the location stayed the same, then update the current
// row. Otherwise, add a new one.
- if (atSameLocation(BaseLabel, L))
+ if (atSameLocation(LastLabel, L))
FDE.FREs.back() = FRE;
else {
FDE.FREs.push_back(FRE);
FDE.FREs.back().Label = L;
- BaseLabel = L;
+ LastLabel = L;
}
}
- if (FDE.Invalid)
- FDEs.pop_back();
+ if (Valid)
+ FDEs.push_back(FDE);
}
void emitPreamble() {
@@ -309,8 +299,6 @@ class SFrameEmitterImpl {
Streamer.emitInt32(FDEs.size());
// shf_num_fres
uint32_t TotalFREs = 0;
- // for (auto &FDE : FDEs)
- // TotalFREs += FDE.FREs.size();
Streamer.emitInt32(TotalFREs);
// shf_fre_len
diff --git a/llvm/test/MC/ELF/cfi-sframe-errors.s b/llvm/test/MC/ELF/cfi-sframe-errors.s
index 65904ed08d5fa..9456503097cf9 100644
--- a/llvm/test/MC/ELF/cfi-sframe-errors.s
+++ b/llvm/test/MC/ELF/cfi-sframe-errors.s
@@ -1,35 +1,31 @@
// TODO: Add other architectures as they gain sframe support
// REQUIRES: x86-registered-target
-// RUN: llvm-mc --assemble --filetype=obj --gsframe -triple x86_64 %s -o %t.o 2>&1 | FileCheck %s
+// RUN: llvm-mc --assemble --filetype=obj -triple x86_64 %s -o %t.o 2>&1 | FileCheck %s
// RUN: llvm-readelf --sframe %t.o | FileCheck --check-prefix=CHECK-NOFDES %s
.cfi_sections .sframe
f1:
.cfi_startproc simple
+// CHECK: Non-default RA register {{.*}}
+ .cfi_return_column 0
nop
- .cfi_def_cfa_offset 16 // No base register yet
- nop
- .cfi_adjust_cfa_offset 16 // No base register yet
+// CHECK: {{.*}} Adjusting CFA offset without a base register.{{.*}}
+ .cfi_def_cfa_offset 16 // no line number reported here.
nop
- .cfi_return_column 0
+// CHECK: [[@LINE+1]]:{{.*}} Adjusting CFA offset without a base register.{{.*}}
+ .cfi_adjust_cfa_offset 16
nop
.cfi_endproc
f2:
.cfi_startproc
nop
+// CHECK: Canonical Frame Address not in stack- or frame-pointer. {{.*}}
.cfi_def_cfa 0, 4
nop
.cfi_endproc
-// CHECK: Non-default RA register {{.*}}
-// asm parser doesn't give a location with .cfi_def_cfa_offset
-// CHECK: :0:{{.*}} Adjusting CFA offset without a base register.{{.*}}
-// .cfi_adjust_cfa_offset
-// CHECK: :13:{{.*}} Adjusting CFA offset without a base register. {{.*}}
-// CHECK: Canonical Frame Address not in stack- or frame-pointer. {{.*}}
-
// CHECK-NOFDES: Num FDEs: 0
// CHECK-NOFDES: Num FREs: 0
diff --git a/llvm/test/MC/ELF/cfi-sframe.s b/llvm/test/MC/ELF/cfi-sframe.s
index d92cf2195085c..ecf77bc3ea6b3 100644
--- a/llvm/test/MC/ELF/cfi-sframe.s
+++ b/llvm/test/MC/ELF/cfi-sframe.s
@@ -16,7 +16,7 @@ f1:
nop
.cfi_register 0, 1 // Uninteresting register. No new FRE.
nop
-
+
.cfi_endproc
f2:
>From 8dd4b523bab4dcb7c8235261ac99f3fc512b622f Mon Sep 17 00:00:00 2001
From: Sterling Augustine <saugustine at google.com>
Date: Thu, 28 Aug 2025 11:10:59 -0700
Subject: [PATCH 7/7] Address more comments
---
llvm/lib/MC/MCSFrame.cpp | 12 +++++++-----
llvm/test/MC/ELF/cfi-sframe-errors.s | 12 ++++++------
2 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/llvm/lib/MC/MCSFrame.cpp b/llvm/lib/MC/MCSFrame.cpp
index f6a1efa5945cd..6446e8d049452 100644
--- a/llvm/lib/MC/MCSFrame.cpp
+++ b/llvm/lib/MC/MCSFrame.cpp
@@ -145,8 +145,7 @@ class SFrameEmitterImpl {
case MCCFIInstruction::OpLLVMDefAspaceCfa:
if (!setCFARegister(FRE, CFI))
return false;
- setCFAOffset(FRE, CFI.getLoc(), CFI.getOffset());
- return true;
+ return setCFAOffset(FRE, CFI.getLoc(), CFI.getOffset());
case MCCFIInstruction::OpOffset:
if (CFI.getRegister() == FPReg)
FRE.FPOffset = CFI.getOffset();
@@ -250,9 +249,12 @@ class SFrameEmitterImpl {
FDE.FREs.push_back(BaseFRE);
for (const auto &CFI : DF.Instructions) {
- // Instructions from InitialFrameState may not have a label, but if
- // these instructions don't, then they are in dead code or otherwise
- // unused.
+ // Instructions from InitialFrameState may not have a label, but if these
+ // instructions don't, then they are in dead code or otherwise unused.
+ // TODO: This check follows MCDwarf.cpp
+ // FrameEmitterImplementation::emitCFIInstructions, but nothing in the
+ // testsuite triggers it. We should see if it can be removed in both
+ // places, or alternately, add a test to exercise it.
auto *L = CFI.getLabel();
if (L && !L->isDefined())
continue;
diff --git a/llvm/test/MC/ELF/cfi-sframe-errors.s b/llvm/test/MC/ELF/cfi-sframe-errors.s
index 9456503097cf9..d4ce17144bb64 100644
--- a/llvm/test/MC/ELF/cfi-sframe-errors.s
+++ b/llvm/test/MC/ELF/cfi-sframe-errors.s
@@ -8,21 +8,21 @@
f1:
.cfi_startproc simple
// CHECK: Non-default RA register {{.*}}
- .cfi_return_column 0
+ .cfi_return_column 0
nop
// CHECK: {{.*}} Adjusting CFA offset without a base register.{{.*}}
- .cfi_def_cfa_offset 16 // no line number reported here.
+ .cfi_def_cfa_offset 16 // no line number reported here.
nop
// CHECK: [[@LINE+1]]:{{.*}} Adjusting CFA offset without a base register.{{.*}}
- .cfi_adjust_cfa_offset 16
+ .cfi_adjust_cfa_offset 16
nop
- .cfi_endproc
+ .cfi_endproc
f2:
- .cfi_startproc
+ .cfi_startproc
nop
// CHECK: Canonical Frame Address not in stack- or frame-pointer. {{.*}}
- .cfi_def_cfa 0, 4
+ .cfi_def_cfa 0, 4
nop
.cfi_endproc
More information about the llvm-commits
mailing list