[PATCH] D150004: [RISCV] .debug_line: emit relocations for assembly input files

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added reviewers: asb, compnerd, craig.topper, enh, hiraditya, kito-cheng, jrtc27.
Herald added subscribers: jobnoorman, luke, VincentWu, vkmr, frasercrmck, evandro, luismarques, apazos, sameer.abuasal, pengfei, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, shiva0217, niosHD, sabuasal, simoncook, johnrusso, rbar, arichardson, emaste.
Herald added a project: All.
MaskRay requested review of this revision.
Herald added subscribers: llvm-commits, pcwang-thead, eopXD.
Herald added a project: LLVM.

Fix https://github.com/llvm/llvm-project/issues/62535

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.

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 before
commit 57ab708bdd3231b23a8ef4978b11ff07616034a2 (2010) that seems no longer
relevant. It usually don't trigger for non-assembly input files (no
`MCAssembler`). Let's remove it.
Assembling the assembly output of X86ISelLowering.cpp with `-g` may be
1% slower, but I think the cost is fine.

For -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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150004

Files:
  llvm/lib/MC/MCObjectStreamer.cpp
  llvm/test/MC/ELF/RISCV/debug-line.s
  llvm/test/MC/ELF/RISCV/lit.local.cfg


Index: llvm/test/MC/ELF/RISCV/lit.local.cfg
===================================================================
--- /dev/null
+++ llvm/test/MC/ELF/RISCV/lit.local.cfg
@@ -0,0 +1,2 @@
+if 'RISCV' not in config.root.targets:
+    config.unsupported = True
Index: llvm/test/MC/ELF/RISCV/debug-line.s
===================================================================
--- /dev/null
+++ llvm/test/MC/ELF/RISCV/debug-line.s
@@ -0,0 +1,37 @@
+## When assembling an assembly file with linker relaxation, emit DW_LNS_fixed_advance_pc
+## with ADD16/SUB16 relocations so that .debug_line can be fixed by the linker.
+## Without linker relaxation, we can emit special opcodes to make .debug_line smaller.
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -g -mattr=-relax < %s -o %t.norelax
+# RUN: llvm-dwarfdump -debug-line -v %t.norelax | FileCheck %s
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -g -mattr=+relax < %s -o %t.relax
+# RUN: llvm-dwarfdump -debug-line -v %t.relax | FileCheck %s
+# RUN: llvm-readobj -r %t.relax | FileCheck %s --check-prefix=RELOC
+
+call foo
+call foo
+call foo
+
+# CHECK:      DW_LNE_set_address
+# CHECK-NEXT: DW_LNS_advance_line ([[#]])
+# CHECK-NEXT: DW_LNS_copy
+# CHECK-NEXT:                         is_stmt
+# CHECK-NEXT: DW_LNS_advance_line
+# CHECK-NEXT: DW_LNS_fixed_advance_pc (0x0008)
+# CHECK-NEXT: DW_LNS_copy
+# CHECK-NEXT:                           is_stmt
+# CHECK-NEXT: DW_LNS_advance_line
+# CHECK-NEXT: DW_LNS_fixed_advance_pc (0x0008)
+# CHECK-NEXT: DW_LNS_copy
+# CHECK-NEXT:                           is_stmt
+# CHECK-NEXT: DW_LNS_fixed_advance_pc (0x0008)
+# CHECK-NEXT: DW_LNE_end_sequence
+
+# RELOC:      Section ([[#]]) .rela.debug_line {
+# RELOC-NEXT:   0x2C R_RISCV_64 - 0x0
+# RELOC-NEXT:   0x3A R_RISCV_ADD16 - 0x0
+# RELOC-NEXT:   0x3A R_RISCV_SUB16 - 0x0
+# RELOC-NEXT:   0x40 R_RISCV_ADD16 - 0x0
+# RELOC-NEXT:   0x40 R_RISCV_SUB16 - 0x0
+# RELOC-NEXT:   0x44 R_RISCV_ADD16 - 0x0
+# RELOC-NEXT:   0x44 R_RISCV_SUB16 - 0x0
+# RELOC-NEXT: }
Index: llvm/lib/MC/MCObjectStreamer.cpp
===================================================================
--- llvm/lib/MC/MCObjectStreamer.cpp
+++ llvm/lib/MC/MCObjectStreamer.cpp
@@ -541,12 +541,6 @@
     return;
   }
   const MCExpr *AddrDelta = buildSymbolDiff(*this, Label, LastLabel);
-  int64_t Res;
-  if (AddrDelta->evaluateAsAbsolute(Res, getAssemblerPtr())) {
-    MCDwarfLineAddr::Emit(this, Assembler->getDWARFLinetableParams(), LineDelta,
-                          Res);
-    return;
-  }
   insert(new MCDwarfLineAddrFragment(LineDelta, *AddrDelta));
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D150004.519993.patch
Type: text/x-patch
Size: 2571 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230505/28045b1f/attachment.bin>