-
Notifications
You must be signed in to change notification settings - Fork 15.4k
Parse CFI instructions to create SFrame FREs #155496
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 8 commits
6ce089d
1b1eb72
67b9205
f5b9edd
d2b2503
c1a97b0
8b54030
3415154
8dd4b52
3ea761c
c9e9ee8
be4da9e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -21,13 +21,33 @@ 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; | ||||||||
| // Unwinding fres | ||||||||
| SmallVector<SFrameFRE> FREs; | ||||||||
|
|
||||||||
| SFrameFDE(const MCDwarfFrameInfo &DF, MCSymbol *FRES) | ||||||||
| : DFrame(DF), FREStart(FRES) {} | ||||||||
|
|
@@ -53,7 +73,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 | ||||||||
|
|
@@ -76,10 +97,91 @@ 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(SFrameFRE &FRE, const MCCFIInstruction &I) { | ||||||||
| if (I.getRegister() == SPReg) { | ||||||||
| FRE.CFARegSet = true; | ||||||||
| FRE.FromFP = false; | ||||||||
| return 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."); | ||||||||
| 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. | ||||||||
| bool handleCFI(SFrameFDE &FDE, SFrameFRE &FRE, const MCCFIInstruction &CFI) { | ||||||||
| switch (CFI.getOperation()) { | ||||||||
| case MCCFIInstruction::OpDefCfaRegister: | ||||||||
| return setCFARegister(FRE, CFI); | ||||||||
| case MCCFIInstruction::OpDefCfa: | ||||||||
| case MCCFIInstruction::OpLLVMDefAspaceCfa: | ||||||||
| if (!setCFARegister(FRE, CFI)) | ||||||||
| return false; | ||||||||
| setCFAOffset(FRE, CFI.getLoc(), CFI.getOffset()); | ||||||||
| return true; | ||||||||
|
||||||||
| setCFAOffset(FRE, CFI.getLoc(), CFI.getOffset()); | |
| return true; | |
| return setCFAOffset(FRE, CFI.getLoc(), CFI.getOffset()); |
I guess the difference is that if this was the only error, the code would still go on to produce a (broken) FDE entry.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: non-default to conform with LLVM warning style guide.
Outdated
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| // TODO: Add other architectures as they gain sframe support | ||
| // REQUIRES: x86-registered-target | ||
| // 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 | ||
labath marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| f1: | ||
| .cfi_startproc simple | ||
| // CHECK: Non-default RA register {{.*}} | ||
| .cfi_return_column 0 | ||
| nop | ||
| // CHECK: {{.*}} Adjusting CFA offset without a base register.{{.*}} | ||
| .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 | ||
| 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 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. something wrong with the indentation here. Tabs?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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-NOFDES: Num FDEs: 0 | ||
| // CHECK-NOFDES: Num FREs: 0 | ||
Uh oh!
There was an error while loading. Please reload this page.