Skip to content
Merged
208 changes: 202 additions & 6 deletions llvm/lib/MC/MCSFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,38 @@ 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 {
// Reference to the original dwarf frame to avoid copying.
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();
Expand All @@ -53,7 +75,8 @@ struct SFrameFDE {
MCFixup::getDataKindForSize(4)));
S.emitInt32(0);

// sfde_func_start_num_fres
// sfde_func_num_fres
// TODO: When we actually emit fres, replace 0 with FREs.size()
S.emitInt32(0);

// sfde_func_info word
Expand All @@ -76,10 +99,101 @@ 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 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;
}

// Add the effects of CFI to the current FDE, creating a new FRE when
// necessary.
void handleCFI(SFrameFDE &FDE, SFrameFRE &FRE, const MCCFIInstruction &CFI) {
switch (CFI.getOperation()) {
case MCCFIInstruction::OpDefCfaRegister:
setCfaRegister(FDE, FRE, CFI);
return;
case MCCFIInstruction::OpDefCfa:
case MCCFIInstruction::OpLLVMDefAspaceCfa:
if (!setCfaRegister(FDE, FRE, CFI))
return;
FRE.CfaOffset = CFI.getOffset();
return;
case MCCFIInstruction::OpOffset:
if (CFI.getRegister() == FPReg)
FRE.FPOffset = CFI.getOffset();
else if (CFI.getRegister() == RAReg)
FRE.RAOffset = CFI.getOffset();
return;
case MCCFIInstruction::OpRelOffset:
if (CFI.getRegister() == FPReg)
FRE.FPOffset += CFI.getOffset();
else if (CFI.getRegister() == RAReg)
FRE.RAOffset += CFI.getOffset();
return;
case MCCFIInstruction::OpDefCfaOffset:
if (!isCfaRegisterSet(FDE, FRE, CFI))
return;
FRE.CfaOffset = CFI.getOffset();
return;
case MCCFIInstruction::OpAdjustCfaOffset:
if (!isCfaRegisterSet(FDE, FRE, CFI))
return;
FRE.CfaOffset += CFI.getOffset();
return;
case MCCFIInstruction::OpRememberState:
// TODO: Implement
return;
case MCCFIInstruction::OpRestore:
// TODO: Implement
return;
case MCCFIInstruction::OpRestoreState:
// TODO: Implement
return;
case MCCFIInstruction::OpEscape:
// TODO: Implement
return;
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()
Expand All @@ -88,13 +202,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) {
FDEs.emplace_back(DF, Streamer.getContext().createTempSymbol());
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) {
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.
// FIXME: This also prevents providing a proper location for the error.
// LLVM doesn't change the return column itself, so this was
// hand-written assembly.
if (DF.RAReg != RAReg) {
Streamer.getContext().reportWarning(
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;
}
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;
Copy link
Collaborator

Choose a reason for hiding this comment

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

When does this happen? Can we have a test case for these undead CFI insns?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is copied from the corresponding eh_frame code, which was added in 2010 (commit 85d9198), and does not include a test for that case.

Nothing in the testsuite triggers it today, and I haven't been able to reproduce it myself in either location. So it's possible that it isn't necessary today.

On the other hand, there are only seven (non-error checking) tests for .cfi_startproc simple in the codebase. So we are in solidly under-tested waters. The alternative here would be to warn and skip with some sort of "missing label" failure. But I'm wary of erroring on invalid input where eh_frame doesn't.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd be tempted to add an assert and see what/if anything breaks there, but I guess following eh_frame makes sense as well. It would be nice to make a note of why you're doing that, to help someone looking at this in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add a TODO here and when this is in a state to run against some much bigger and a wider variety test cases, will see if it can be removed.


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() {
Expand All @@ -116,7 +308,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));
Expand Down Expand Up @@ -161,7 +357,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.
Expand Down
35 changes: 35 additions & 0 deletions llvm/test/MC/ELF/cfi-sframe-errors.s
Original file line number Diff line number Diff line change
@@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

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

something wrong with the indentation here. Tabs?

Copy link
Collaborator

Choose a reason for hiding this comment

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

There's still something wrong with the indentation on this line (.cfi_endproc), at least when I view it in github UI. (I would expect it to start at the same column as the other .cfi directives.


// 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. {{.*}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will make it hard to update the file. Please move the checks next to the code which generates those diagnostics. That makes it easier to see what's being checked, and you can use [[@LINE-1]] to match the line numbers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

// CHECK: Canonical Frame Address not in stack- or frame-pointer. {{.*}}

// CHECK-NOFDES: Num FDEs: 0
// CHECK-NOFDES: Num FREs: 0
20 changes: 15 additions & 5 deletions llvm/test/MC/ELF/cfi-sframe.s
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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 {
Expand All @@ -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: }
Expand Down
Loading