Skip to content

Commit d213f6b

Browse files
MaskRayluismarques
authored andcommitted
[RISCV][MC] .debug_line/.debug_frame/.eh_frame: emit relocations for assembly input files with relaxation
When assembling `.debug_line` for both explicitly specified and synthesized `.loc` directives. the integrated assembler may incorrectly omit relocations for -mrelax. For an assembly file, we have a `MCAssembler` object and `evaluateAsAbsolute` will incorrectly fold `AddrDelta` to a constant (which is not true in the presence of linker relaxation). `MCDwarfLineAddr::Emit` will emit a special opcode, which does not take into account of linker relaxation. This is a sufficiently complex function that I think should be called in any "fast paths" for linker relaxation aware assembling. The following script demonstrates the bugs. ``` cat > x.c <<eof void f(); void _start() { f(); f(); f(); } eof clang --target=riscv64 -g -c x.c llvm-dwarfdump --debug-line -v x.o | grep \ DW_LNS_fixed_advance_pc -q clang --target=riscv64 -S x.c && clang --target=riscv64 -g -c x.s llvm-dwarfdump --debug-line -v x.o | grep \ DW_LNS_fixed_advance_pc -q; test $? -eq 1 clang --target=riscv64 -S -g x.c && clang --target=riscv64 -c x.s llvm-dwarfdump --debug-line -v x.o | grep \ DW_LNS_fixed_advance_pc -q; test $? -eq 1 ``` The `MCDwarfLineAddr::Emit` code path is an old optimization in commit 57ab708 (2010) that seems no longer relevant. It don't trigger for direct machine code emission (label differences are not foldable without a `MCAssembler`). MCDwarfLineAddr::Emit does complex operations that are repeated in MCAssembler::relaxDwarfLineAddr, which an intricate RISCV override. Let's remove the "fast path". Assembling the assembly output of X86ISelLowering.cpp with `-g` may be 2% slower, but I think the cost is fine. There are opportunities to make the "slow path" faster, e.g. * Optimizing the current new MC*Fragment pattern that allocates new fragments on the heap. * Reducing the number of relaxation times for .debug_line and .debug_frame, as well as possibly other sections using LEB128. For instance, LEB128 can have a one-byte estimate to avoid the second relaxation iteration. For assembly input with -mno-relax, in theory we can prefer special opcodes to DW_LNS_fixed_advance_pc to decrease the size of .debug_line, but such a change may be overkill and unnecessarily diverge from -mrelax behaviors and GCC. --- For .debug_frame/.eh_frame, MCDwarf currently emits DW_CFA_advance_loc without relocations. Remove the special case to enable relocations. Similar to .debug_line, even without the bug fix, the MCDwarfFrameEmitter::encodeAdvanceLoc special case is a sufficiently complex code path that should be avoided. --- When there are more than one section, we generate .debug_rnglists for DWARF v5. We currently emit DW_RLE_start_length using ULEB128, which is incorrect. The new test gen-dwarf.s adds a TODO. --- About other `evaluateAsAbsolute` uses. `MCObjectStreamer::emit[SU]LEB128Value` have similar code to MCDwarfLineAddr. They are fine to keep as we don't have LEB128 relocations to correctly represent link-time non-constants anyway. --- In the future, we should investigate ending a MCFragment for a relaxable instruction, to further clean up the assembler support for linker relaxation and fix `evaluateAsAbsolute`. See bbea642 for some of the related code. Reviewed By: enh, barannikov88 Differential Revision: https://reviews.llvm.org/D150004
1 parent 45dd593 commit d213f6b

File tree

3 files changed

+99
-11
lines changed

3 files changed

+99
-11
lines changed

llvm/lib/MC/MCObjectStreamer.cpp

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -542,12 +542,6 @@ void MCObjectStreamer::emitDwarfAdvanceLineAddr(int64_t LineDelta,
542542
return;
543543
}
544544
const MCExpr *AddrDelta = buildSymbolDiff(*this, Label, LastLabel);
545-
int64_t Res;
546-
if (AddrDelta->evaluateAsAbsolute(Res, getAssemblerPtr())) {
547-
MCDwarfLineAddr::Emit(this, Assembler->getDWARFLinetableParams(), LineDelta,
548-
Res);
549-
return;
550-
}
551545
insert(new MCDwarfLineAddrFragment(LineDelta, *AddrDelta));
552546
}
553547

@@ -572,11 +566,6 @@ void MCObjectStreamer::emitDwarfLineEndEntry(MCSection *Section,
572566
void MCObjectStreamer::emitDwarfAdvanceFrameAddr(const MCSymbol *LastLabel,
573567
const MCSymbol *Label) {
574568
const MCExpr *AddrDelta = buildSymbolDiff(*this, Label, LastLabel);
575-
int64_t Res;
576-
if (AddrDelta->evaluateAsAbsolute(Res, getAssemblerPtr())) {
577-
MCDwarfFrameEmitter::EmitAdvanceLoc(*this, Res);
578-
return;
579-
}
580569
insert(new MCDwarfCallFrameFragment(*AddrDelta));
581570
}
582571

llvm/test/MC/ELF/RISCV/gen-dwarf.s

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
## Linker relaxation imposes restrictions on .eh_frame/.debug_frame, .debug_line,
2+
## and LEB128 uses.
3+
4+
## CFI instructions can be preceded by relaxable instructions. We must use
5+
## DW_CFA_advance_loc* opcodes with relocations.
6+
7+
## For .debug_line, emit DW_LNS_fixed_advance_pc with ADD16/SUB16 relocations so
8+
## that .debug_line can be fixed by the linker. Without linker relaxation, we can
9+
## emit special opcodes to make .debug_line smaller, but we don't do this for
10+
## consistency.
11+
12+
# RUN: llvm-mc -filetype=obj -triple=riscv64 -g -dwarf-version=5 -mattr=+relax < %s -o %t
13+
# RUN: llvm-dwarfdump -eh-frame -debug-line -debug-rnglists -v %t | FileCheck %s
14+
# RUN: llvm-readobj -r %t | FileCheck %s --check-prefix=RELOC
15+
16+
# CHECK: FDE
17+
# CHECK-NEXT: Format: DWARF32
18+
# CHECK-NEXT: DW_CFA_advance_loc: 16
19+
# CHECK-NEXT: DW_CFA_def_cfa_offset: +32
20+
# CHECK-NEXT: DW_CFA_advance_loc: 4
21+
# CHECK-NEXT: DW_CFA_offset: X1 -8
22+
# CHECK-NEXT: DW_CFA_nop:
23+
24+
# CHECK: DW_LNE_set_address
25+
# CHECK-NEXT: DW_LNS_advance_line ([[#]])
26+
# CHECK-NEXT: DW_LNS_copy
27+
# CHECK-NEXT: is_stmt
28+
# CHECK-NEXT: DW_LNS_advance_line
29+
# CHECK-NEXT: DW_LNS_fixed_advance_pc (0x0004)
30+
# CHECK-NEXT: DW_LNS_copy
31+
# CHECK-NEXT: is_stmt
32+
# CHECK-NEXT: DW_LNS_advance_line
33+
# CHECK-NEXT: DW_LNS_fixed_advance_pc (0x0004)
34+
# CHECK-NEXT: DW_LNS_copy
35+
36+
# CHECK: 0x00000000: range list header: length = 0x0000001d, format = DWARF32, version = 0x0005
37+
# CHECK-NEXT: ranges:
38+
# CHECK-NEXT: 0x0000000c: [DW_RLE_start_length]: 0x0000000000000000, 0x0000000000000034
39+
# CHECK-NEXT: 0x00000016: [DW_RLE_start_length]: 0x0000000000000000, 0x0000000000000004
40+
# CHECK-NEXT: 0x00000020: [DW_RLE_end_of_list ]
41+
42+
# RELOC: Section ([[#]]) .rela.eh_frame {
43+
# RELOC-NEXT: 0x1C R_RISCV_32_PCREL - 0x0
44+
# RELOC-NEXT: 0x20 R_RISCV_ADD32 - 0x0
45+
# RELOC-NEXT: 0x20 R_RISCV_SUB32 - 0x0
46+
# RELOC-NEXT: 0x25 R_RISCV_SET6 - 0x0
47+
# RELOC-NEXT: 0x25 R_RISCV_SUB6 - 0x0
48+
# RELOC-NEXT: 0x28 R_RISCV_SET6 - 0x0
49+
# RELOC-NEXT: 0x28 R_RISCV_SUB6 - 0x0
50+
# RELOC-NEXT: 0x34 R_RISCV_32_PCREL - 0x0
51+
# RELOC-NEXT: 0x38 R_RISCV_ADD32 - 0x0
52+
# RELOC-NEXT: 0x38 R_RISCV_SUB32 - 0x0
53+
# RELOC-NEXT: }
54+
55+
## TODO A section needs two relocations.
56+
# RELOC: Section ([[#]]) .rela.debug_rnglists {
57+
# RELOC-NEXT: 0xD R_RISCV_64 .text.foo 0x0
58+
# RELOC-NEXT: 0x17 R_RISCV_64 .text.bar 0x0
59+
# RELOC-NEXT: }
60+
61+
# RELOC: Section ([[#]]) .rela.debug_line {
62+
# RELOC: R_RISCV_ADD16 - 0x0
63+
# RELOC-NEXT: R_RISCV_SUB16 - 0x0
64+
# RELOC-NEXT: R_RISCV_ADD16 - 0x0
65+
# RELOC-NEXT: R_RISCV_SUB16 - 0x0
66+
# RELOC-NEXT: R_RISCV_ADD16 - 0x0
67+
# RELOC-NEXT: R_RISCV_SUB16 - 0x0
68+
# RELOC: }
69+
70+
.section .text.foo,"ax"
71+
.globl foo
72+
foo:
73+
.cfi_startproc
74+
.Lpcrel_hi0:
75+
auipc a1, %pcrel_hi(g)
76+
lw a1, %pcrel_lo(.Lpcrel_hi0)(a1)
77+
bge a1, a0, .LBB0_2
78+
addi sp, sp, -32
79+
.cfi_def_cfa_offset 32
80+
sd ra, 24(sp)
81+
.cfi_offset ra, -8
82+
addi a0, sp, 8
83+
call ext@plt
84+
ld ra, 24(sp)
85+
addi sp, sp, 32
86+
ret
87+
.LBB0_2:
88+
li a0, 0
89+
ret
90+
.cfi_endproc
91+
.size foo, .-foo
92+
93+
.section .text.bar,"ax"
94+
bar:
95+
.cfi_startproc
96+
nop
97+
.cfi_endproc

llvm/test/MC/ELF/RISCV/lit.local.cfg

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
if 'RISCV' not in config.root.targets:
2+
config.unsupported = True

0 commit comments

Comments
 (0)