Skip to content

Commit 8c4d661

Browse files
Sterling-AugustineSterling-Augustine
authored
[Sframe] Support cfi_escape directives compatibly with gnu-gas (#161927)
Some cfi_escape directives don't affect sframe unwind info, some do. Detect those cases appropriately, following gnu-gas for most cases. Using a full-blown dwarf expression parser allows for somewhat more precise error detection than other sframe implementations. So this code is less conservative for long and more involved expressions. It could be made even more permissive.
1 parent 28e1628 commit 8c4d661

File tree

5 files changed

+236
-5
lines changed

5 files changed

+236
-5
lines changed

‎llvm/lib/MC/CMakeLists.txt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,10 @@ add_llvm_component_library(LLVMMC
7373
${LLVM_MAIN_INCLUDE_DIR}/llvm/MC
7474

7575
LINK_COMPONENTS
76+
BinaryFormat
77+
DebugInfoDWARFLowLevel
7678
Support
7779
TargetParser
78-
BinaryFormat
7980

8081
DEPENDS
8182
intrinsics_gen

‎llvm/lib/MC/MCSFrame.cpp‎

Lines changed: 151 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
#include "llvm/MC/MCSFrame.h"
1010
#include "llvm/BinaryFormat/SFrame.h"
11+
#include "llvm/DebugInfo/DWARF/LowLevel/DWARFCFIProgram.h"
12+
#include "llvm/DebugInfo/DWARF/LowLevel/DWARFDataExtractorSimple.h"
1113
#include "llvm/MC/MCAsmInfo.h"
1214
#include "llvm/MC/MCContext.h"
1315
#include "llvm/MC/MCObjectFileInfo.h"
@@ -211,8 +213,152 @@ class SFrameEmitterImpl {
211213
return true;
212214
}
213215

216+
// Technically, the escape data could be anything, but it is commonly a dwarf
217+
// CFI program. Even then, it could contain an arbitrarily complicated Dwarf
218+
// expression. Following gnu-gas, look for certain common cases that could
219+
// invalidate an FDE, emit a warning for those sequences, and don't generate
220+
// an FDE in those cases. Allow any that are known safe. It is likely that
221+
// more thorough test cases could refine this code, but it handles the most
222+
// important ones compatibly with gas.
223+
// Returns true if the CFI escape sequence is safe for sframes.
224+
bool isCFIEscapeSafe(SFrameFDE &FDE, const SFrameFRE &FRE,
225+
const MCCFIInstruction &CFI) {
226+
const MCAsmInfo *AI = Streamer.getContext().getAsmInfo();
227+
DWARFDataExtractorSimple data(CFI.getValues(), AI->isLittleEndian(),
228+
AI->getCodePointerSize());
229+
230+
// Normally, both alignment factors are extracted from the enclosing Dwarf
231+
// FDE or CIE. We don't have one here. Alignments are used for scaling
232+
// factors for ops like CFA_def_cfa_offset_sf. But this particular function
233+
// is only interested in registers.
234+
dwarf::CFIProgram P(/*CodeAlignmentFactor=*/1,
235+
/*DataAlignmentFactor=*/1,
236+
Streamer.getContext().getTargetTriple().getArch());
237+
uint64_t Offset = 0;
238+
if (P.parse(data, &Offset, CFI.getValues().size())) {
239+
// Not a parsable dwarf expression. Assume the worst.
240+
Streamer.getContext().reportWarning(
241+
CFI.getLoc(),
242+
"skipping SFrame FDE; .cfi_escape with unknown effects");
243+
return false;
244+
}
245+
246+
// This loop deals with dwarf::CFIProgram::Instructions. Everywhere else
247+
// this file deals with MCCFIInstructions.
248+
for (const dwarf::CFIProgram::Instruction &I : P) {
249+
switch (I.Opcode) {
250+
case dwarf::DW_CFA_nop:
251+
break;
252+
case dwarf::DW_CFA_val_offset: {
253+
// First argument is a register. Anything that touches CFA, FP, or RA is
254+
// a problem, but allow others through. As an even more special case,
255+
// allow SP + 0.
256+
auto Reg = I.getOperandAsUnsigned(P, 0);
257+
// The parser should have failed in this case.
258+
assert(Reg && "DW_CFA_val_offset with no register.");
259+
bool SPOk = true;
260+
if (*Reg == SPReg) {
261+
auto Opnd = I.getOperandAsSigned(P, 1);
262+
if (!Opnd || *Opnd != 0)
263+
SPOk = false;
264+
}
265+
if (!SPOk || *Reg == RAReg || *Reg == FPReg) {
266+
StringRef RN = *Reg == SPReg
267+
? "SP reg "
268+
: (*Reg == FPReg ? "FP reg " : "RA reg ");
269+
Streamer.getContext().reportWarning(
270+
CFI.getLoc(),
271+
Twine(
272+
"skipping SFrame FDE; .cfi_escape DW_CFA_val_offset with ") +
273+
RN + Twine(*Reg));
274+
return false;
275+
}
276+
} break;
277+
case dwarf::DW_CFA_expression: {
278+
// First argument is a register. Anything that touches CFA, FP, or RA is
279+
// a problem, but allow others through.
280+
auto Reg = I.getOperandAsUnsigned(P, 0);
281+
if (!Reg) {
282+
Streamer.getContext().reportWarning(
283+
CFI.getLoc(),
284+
"skipping SFrame FDE; .cfi_escape with unknown effects");
285+
return false;
286+
}
287+
if (*Reg == SPReg || *Reg == RAReg || *Reg == FPReg) {
288+
StringRef RN = *Reg == SPReg
289+
? "SP reg "
290+
: (*Reg == FPReg ? "FP reg " : "RA reg ");
291+
Streamer.getContext().reportWarning(
292+
CFI.getLoc(),
293+
Twine(
294+
"skipping SFrame FDE; .cfi_escape DW_CFA_expression with ") +
295+
RN + Twine(*Reg));
296+
return false;
297+
}
298+
} break;
299+
case dwarf::DW_CFA_GNU_args_size: {
300+
auto Size = I.getOperandAsSigned(P, 0);
301+
// Zero size doesn't affect the cfa.
302+
if (Size && *Size == 0)
303+
break;
304+
if (FRE.Info.getBaseRegister() != BaseReg::FP) {
305+
Streamer.getContext().reportWarning(
306+
CFI.getLoc(),
307+
Twine("skipping SFrame FDE; .cfi_escape DW_CFA_GNU_args_size "
308+
"with non frame-pointer CFA"));
309+
return false;
310+
}
311+
} break;
312+
// Cases that gas doesn't specially handle. TODO: Some of these could be
313+
// analyzed and handled instead of just punting. But these are uncommon,
314+
// or should be written as normal cfi directives. Some will need fixes to
315+
// the scaling factor.
316+
case dwarf::DW_CFA_advance_loc:
317+
case dwarf::DW_CFA_offset:
318+
case dwarf::DW_CFA_restore:
319+
case dwarf::DW_CFA_set_loc:
320+
case dwarf::DW_CFA_advance_loc1:
321+
case dwarf::DW_CFA_advance_loc2:
322+
case dwarf::DW_CFA_advance_loc4:
323+
case dwarf::DW_CFA_offset_extended:
324+
case dwarf::DW_CFA_restore_extended:
325+
case dwarf::DW_CFA_undefined:
326+
case dwarf::DW_CFA_same_value:
327+
case dwarf::DW_CFA_register:
328+
case dwarf::DW_CFA_remember_state:
329+
case dwarf::DW_CFA_restore_state:
330+
case dwarf::DW_CFA_def_cfa:
331+
case dwarf::DW_CFA_def_cfa_register:
332+
case dwarf::DW_CFA_def_cfa_offset:
333+
case dwarf::DW_CFA_def_cfa_expression:
334+
case dwarf::DW_CFA_offset_extended_sf:
335+
case dwarf::DW_CFA_def_cfa_sf:
336+
case dwarf::DW_CFA_def_cfa_offset_sf:
337+
case dwarf::DW_CFA_val_offset_sf:
338+
case dwarf::DW_CFA_val_expression:
339+
case dwarf::DW_CFA_MIPS_advance_loc8:
340+
case dwarf::DW_CFA_AARCH64_negate_ra_state_with_pc:
341+
case dwarf::DW_CFA_AARCH64_negate_ra_state:
342+
case dwarf::DW_CFA_LLVM_def_aspace_cfa:
343+
case dwarf::DW_CFA_LLVM_def_aspace_cfa_sf:
344+
Streamer.getContext().reportWarning(
345+
CFI.getLoc(), "skipping SFrame FDE; .cfi_escape "
346+
"CFA expression with unknown side effects");
347+
return false;
348+
default:
349+
// Dwarf expression was only partially valid, and user could have
350+
// written anything.
351+
Streamer.getContext().reportWarning(
352+
CFI.getLoc(),
353+
"skipping SFrame FDE; .cfi_escape with unknown effects");
354+
return false;
355+
}
356+
}
357+
return true;
358+
}
359+
214360
// Add the effects of CFI to the current FDE, creating a new FRE when
215-
// necessary.
361+
// necessary. Return true if the CFI is representable in the sframe format.
216362
bool handleCFI(SFrameFDE &FDE, SFrameFRE &FRE, const MCCFIInstruction &CFI) {
217363
switch (CFI.getOperation()) {
218364
case MCCFIInstruction::OpDefCfaRegister:
@@ -265,10 +411,11 @@ class SFrameEmitterImpl {
265411
FRE = FDE.SaveState.pop_back_val();
266412
return true;
267413
case MCCFIInstruction::OpEscape:
268-
// TODO: Implement. Will use FDE.
269-
return true;
414+
// This is a string of bytes that contains an arbitrary dwarf-expression
415+
// that may or may not affect unwind info.
416+
return isCFIEscapeSafe(FDE, FRE, CFI);
270417
default:
271-
// Instructions that don't affect the CFA, RA, and SP can be safely
418+
// Instructions that don't affect the CFA, RA, and FP can be safely
272419
// ignored.
273420
return true;
274421
}

‎llvm/test/MC/ELF/cfi-sframe-cfi-escape-diagnostics.s‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
# RUN: llvm-mc --filetype=obj --gsframe -triple x86_64 %s -o %t.o 2>&1 | FileCheck %s
2+
# RUN: llvm-readelf --sframe %t.o | FileCheck %s --check-prefix=NOFDES
3+
4+
## Tests that .cfi_escape sequences that are unrepresentable in sframe warn
5+
## and do not produce FDEs.
6+
7+
.align 1024
8+
cfi_escape_sp:
9+
.cfi_startproc
10+
.long 0
11+
## Setting SP via other registers makes it unrepresentable in sframe
12+
## DW_CFA_expression,reg 0x7,length 2,DW_OP_breg6,SLEB(-8)
13+
# CHECK: {{.*}}.s:[[#@LINE+1]]:9: warning: skipping SFrame FDE; .cfi_escape DW_CFA_expression with SP reg 7
14+
.cfi_escape 0x10, 0x7, 0x2, 0x76, 0x78
15+
.long 0
16+
.cfi_endproc
17+
18+
cfi_escape_args_sp:
19+
.cfi_startproc
20+
.long 0
21+
## DW_CFA_GNU_args_size is not OK if cfa is SP
22+
# CHECK: {{.*}}.s:[[#@LINE+1]]:9: warning: skipping SFrame FDE; .cfi_escape DW_CFA_GNU_args_size with non frame-pointer CFA
23+
.cfi_escape 0x2e, 0x20
24+
.cfi_endproc
25+
26+
cfi_escape_val_offset:
27+
.cfi_startproc
28+
.long 0
29+
.cfi_def_cfa_offset 16
30+
## DW_CFA_val_offset,rbp,ULEB scaled offset(16)
31+
# CHECK: {{.*}}.s:[[#@LINE+1]]:9: warning: skipping SFrame FDE; .cfi_escape DW_CFA_val_offset with FP reg 6
32+
.cfi_escape 0x14,0x6,0x2
33+
.long 0
34+
.cfi_endproc
35+
36+
# NOFDES: Num FDEs: 0

‎llvm/test/MC/ELF/cfi-sframe-cfi-escape.s‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
# RUN: llvm-mc --filetype=obj --gsframe -triple x86_64 %s -o %t.o
2+
# RUN: llvm-readelf --sframe %t.o | FileCheck %s
3+
4+
## Tests that .cfi_escape sequences that are ok to pass through work.
5+
6+
.align 1024
7+
cfi_escape_ok:
8+
.cfi_startproc
9+
.long 0
10+
.cfi_def_cfa_offset 16
11+
## Uninteresting register
12+
## DW_CFA_expression,reg 0xc,length 2,DW_OP_breg6,SLEB(-8)
13+
.cfi_escape 0x10,0xc,0x2,0x76,0x78
14+
## DW_CFA_nop
15+
.cfi_escape 0x0
16+
.cfi_escape 0x0,0x0,0x0,0x0
17+
## Uninteresting register
18+
## DW_CFA_val_offset,reg 0xc,ULEB scaled offset
19+
.cfi_escape 0x14,0xc,0x4
20+
.long 0
21+
.cfi_endproc
22+
23+
cfi_escape_gnu_args_fp:
24+
.cfi_startproc
25+
.long 0
26+
## DW_CFA_GNU_args_size is OK if arg size is zero
27+
.cfi_escape 0x2e, 0x0
28+
.long 0
29+
.cfi_def_cfa_register 6
30+
.long 0
31+
## DW_CFA_GNU_args_size is OK if cfa is FP
32+
.cfi_escape 0x2e, 0x20
33+
.cfi_endproc
34+
35+
cfi_escape_long_expr:
36+
.cfi_startproc
37+
.long 0
38+
.cfi_def_cfa_offset 16
39+
## This is a long, but valid, dwarf expression without sframe
40+
## implications. An FDE can still be created.
41+
## DW_CFA_val_offset,rcx,ULEB scaled offset(16), DW_CFA_expr,r10,length,DW_OP_deref,SLEB(-8)
42+
.cfi_escape 0x14,0x2,0x2,0x10,0xa,0x2,0x76,0x78
43+
.long 0
44+
.cfi_endproc
45+
46+
# CHECK: Num FDEs: 3

‎utils/bazel/llvm-project-overlay/llvm/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -577,6 +577,7 @@ cc_library(
577577
deps = [
578578
":BinaryFormat",
579579
":DebugInfoCodeView",
580+
":DebugInfoDWARFLowLevel",
580581
":Support",
581582
":TargetParser",
582583
":config",

0 commit comments

Comments
 (0)