[all-commits] [llvm/llvm-project] dde179: [RISCV][MC] .debug_line/.debug_frame/.eh_frame: em...

[Fangrui Song via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: dde1795f14eb946fbcb1810863fa172fbdf7e044
      https://github.com/llvm/llvm-project/commit/dde1795f14eb946fbcb1810863fa172fbdf7e044
  Author: Fangrui Song <i at maskray.me>
  Date:   2023-05-15 (Mon, 15 May 2023)

  Changed paths:
    M llvm/lib/MC/MCObjectStreamer.cpp
    A llvm/test/MC/ELF/RISCV/gen-dwarf.s
    A llvm/test/MC/ELF/RISCV/lit.local.cfg

  Log Message:
  -----------
  [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
# 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.

Reviewed By: enh, barannikov88

Differential Revision: https://reviews.llvm.org/D150004