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 # C to object file: correct DW_LNS_fixed_advance_pc clang --target=riscv64 -g -c x.c llvm-dwarfdump --debug-line -v x.o | grep \ DW_LNS_fixed_advance_pc -q # Assembly to object file with synthesized line number information: incorrect special opcodes 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 # Assembly with .loc to object file: incorrect special opcodes 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
57ab708bdd3231b23a8ef4978b11ff07616034a2 (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 bbea64250f65480d787e1c5ff45c4de3ec2dcda8 for some of the related code.
The code structure here makes it seem like evaluateAsAbsolute should be returning false in this case. Removing the fastpath here certainly hides this bug, but do we have others lurking due to incorrect return results from evaluateAsAbsolute?
Not my area, so totally possible I'm off base here.