[llvm] Parse CFI instructions to create SFrame FREs (PR #155496)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 26 13:54:04 PDT 2025
https://github.com/Sterling-Augustine created https://github.com/llvm/llvm-project/pull/155496
This PR parses CFI instructions to generate FREs.
Unfortunately, actually emitting the FREs into the object file is somewhat involved and would make this PR quite a bit harder to review. And the dumper itself properly errors if the FRE count is included in the header or FDEs, but they are not actually emitted. So actually testing that the proper FREs are generated will have to wait for a subsequent PR.
For now, just check for common issues with CFI that sframe doesn't support, and that proper error handling is done.
>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/5] 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/5] 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/5] 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/5] 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/5] 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: }
More information about the llvm-commits
mailing list