MC: force eager evaluation of relocations in `.debug_info`

[MaskRay]

Summary

Force eager evaluation of symbolic difference on debug_info which is
required to be resolved eagerly for fission as dwo sections may not
contain relocations.

Fixes: #56642

[MaskRay]

This does not fix the incompatibility with RISC-V linker relaxation. This changes DW_AT_high_pc from a length to (pre-DWARF-v4) an address.
The address needs to be relocated as well. The correct approach is to use .debug_addr indirection via DW_FORM_addrx.

This is useful with linker relaxation, though, as it can decrease the number of relocations.

https://maskray.me/blog/2021-03-14-the-dark-side-of-riscv-linker-relaxation#code-addresses I am going to revise some paragraphs about DWARF.

[MaskRay]

Note: with -mno-relax this will increase the number of relocations.

[compnerd]

In D130206#3666984, @MaskRay wrote:

This does not fix the incompatibility with RISC-V linker relaxation. This changes DW_AT_high_pc from a length to (pre-DWARF-v4) an address.

Correct, this is not to fix the incompatibility with relaxation, it is to fix the compatibility with no relaxation and fission.

The address needs to be relocated as well. The correct approach is to use .debug_addr indirection via DW_FORM_addrx.

Interesting, do we have any target currently using that form?

This is useful with linker relaxation, though, as it can decrease the number of relocations.

https://maskray.me/blog/2021-03-14-the-dark-side-of-riscv-linker-relaxation#code-addresses I am going to revise some paragraphs about DWARF.

Yes, this should be able to reduce the number of relocations at the cost of debug info size. However, given that we currently fail to emit the object file this seems like a good tradeoff.

[MaskRay]

In D130206#3669301, @compnerd wrote:

In D130206#3666984, @MaskRay wrote:

This does not fix the incompatibility with RISC-V linker relaxation. This changes DW_AT_high_pc from a length to (pre-DWARF-v4) an address.

Correct, this is not to fix the incompatibility with relaxation, it is to fix the compatibility with no relaxation and fission.

For -mno-relax code, using the length instead of the end address for DW_AT_high_pc leads to fewer relocations in the output.
MC should be able to resolve the pair of R_RISCV_ADD32/R_RISCV_SUB32 to a constant.

With this patch, there will be a relocation which cannot be resolved at assembler time, leading to larger output size.

The address needs to be relocated as well. The correct approach is to use .debug_addr indirection via DW_FORM_addrx.

Interesting, do we have any target currently using that form?

No. As you can see, for -mno-relax, DW_AT_high_pc is a constant which saves more space than using the end address (one relocation) or using a .debug_addr indirection to represent the end address (more expensive),
so it is unimplemented. I think GCC doesn't implement it as well, hence the silent broken output :)

This is useful with linker relaxation, though, as it can decrease the number of relocations.

https://maskray.me/blog/2021-03-14-the-dark-side-of-riscv-linker-relaxation#code-addresses I am going to revise some paragraphs about DWARF.

Yes, this should be able to reduce the number of relocations at the cost of debug info size. However, given that we currently fail to emit the object file this seems like a good tradeoff.

See above.

[MaskRay]

There have been some changes in the relocation area. Is this patch unneeded now?

[MaskRay]

Pretty sure unneeded after https://reviews.llvm.org/D155357 completely fixed the requiresFixups heuristic problem :)

llvm/lib/MC/MCObjectStreamer.cpp

if (!Hi->getFragment() || Hi->getFragment() != Lo->getFragment() ||
     return None;
 
   return Hi->getOffset() - Lo->getOffset();
 }
 
 void MCObjectStreamer::emitAbsoluteSymbolDiff(const MCSymbol *Hi,
                                               const MCSymbol *Lo,
                                               unsigned Size) {
-  if (!getAssembler().getContext().getTargetTriple().isRISCV())
+  if (!getAssembler().getContext().getTargetTriple().isRISCV() ||
+      getCurrentFragment()->getParent()->getName().startswith(".debug_info"))
     if (Optional<uint64_t> Diff = absoluteSymbolDiff(Hi, Lo))
       return emitIntValue(*Diff, Size);
   MCStreamer::emitAbsoluteSymbolDiff(Hi, Lo, Size);
 }
 
 void MCObjectStreamer::emitAbsoluteSymbolDiffAsULEB128(const MCSymbol *Hi,
                                                        const MCSymbol *Lo) {
-  if (!getAssembler().getContext().getTargetTriple().isRISCV())
+  if (!getAssembler().getContext().getTargetTriple().isRISCV() ||
+      getCurrentFragment()->getParent()->getName().startswith(".debug_info"))
     if (Optional<uint64_t> Diff = absoluteSymbolDiff(Hi, Lo))
       return emitULEB128IntValue(*Diff);
   MCStreamer::emitAbsoluteSymbolDiffAsULEB128(Hi, Lo);
 }
 
 void MCObjectStreamer::reset() {
   if (Assembler)
     Assembler->reset();

llvm/test/CodeGen/RISCV/fixups-diff.ll

 define i32 @main() !dbg !7 {
 entry:
   %retval = alloca i32, align 4
   store i32 0, i32* %retval, align 4
   ret i32 0
 }
 
 ; CHECK:      Section {{.*}} .rela.debug_info {
-; CHECK:        0x22 R_RISCV_ADD32 - 0x0
-; CHECK-NEXT:   0x22 R_RISCV_SUB32 - 0x0
-; CHECK:        0x2B R_RISCV_ADD32 - 0x0
-; CHECK-NEXT:   0x2B R_RISCV_SUB32 - 0x0
+; CHECK:        0x27 R_RISCV_32 - 0x0
+; CHECK:        0x31 R_RISCV_32 - 0x0
 ; CHECK:      }
 
 ; CHECK:      Section {{.*}} .rela.eh_frame {
 ; CHECK:        0x1C R_RISCV_32_PCREL - 0x0
 ; CHECK:        0x20 R_RISCV_ADD32 - 0x0
 ; CHECK-NEXT:   0x20 R_RISCV_SUB32 - 0x0
 ; CHECK:      }
 

llvm/test/DebugInfo/RISCV/debug_info.ll

+; RUN: llc -mtriple riscv64-unknown-linux-gnu -filetype obj %s -o - | llvm-dwarfdump - | FileCheck %s -check-prefix CHECK-DWARF
+; RUN: llc -mtriple riscv64-unknown-linux-gnu -filetype obj %s -o - | llvm-readobj -r - | FileCheck %s -check-prefix CHECK-OBJ
+
+source_filename = "reduced.c"
+target datalayout = "e-m:e-p:64:64-i64:64-i128:128-n64-S128"
+target triple = "riscv64-unknown-linux-gnu"
+
+define dso_local signext i32 @f() #0 !dbg !7 {
+entry:
+  ret i32 32, !dbg !13
+}
+
+attributes #0 = { noinline nounwind optnone "frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+64bit" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 15.0.0", isOptimized: false, runtimeVersion: 0, splitDebugFilename: "reduced.dwo", emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "reduced.c", directory: "/SourceCache")
+!2 = !{i32 2, !"Debug Info Version", i32 3}
+!3 = !{i32 1, !"wchar_size", i32 4}
+!4 = !{i32 1, !"target-abi", !"lp64"}
+!5 = !{i32 1, !"SmallDataLimit", i32 0}
+!6 = !{!"clang version 15.0.0"}
+!7 = distinct !DISubprogram(name: "f", scope: !8, file: !8, line: 2, type: !9, scopeLine: 2, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !12)
+!8 = !DIFile(filename: "/SourceCache/reduced.c", directory: "")
+!9 = !DISubroutineType(types: !10)
+!10 = !{!11}
+!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!12 = !{}
+!13 = !DILocation(line: 2, column: 15, scope: !7)
+
+; CHECK-DWARF:      0x0000002a:   DW_TAG_subprogram
+; CHECK-DWARF-NEXT:                 DW_AT_low_pc  (0x0000000000000000)
+; CHECK-DWARF-NEXT:                 DW_AT_high_pc (0x0000000000000008)
+
+; CHECK-OBJ: Section (5) .rela.debug_info {
+; CHECK-OBJ:       0x1A R_RISCV_32 - 0x0
+; CHECK-OBJ:       0x1E R_RISCV_64 - 0x0
+; CHECK-OBJ: }

llvm/test/DebugInfo/RISCV/dwarf-riscv-relocs.ll

 ; RUN: llc -filetype=obj -mtriple=riscv32 -mattr=+relax %s -o %t.o
 ; RUN: llvm-readobj -r %t.o | FileCheck -check-prefix=READOBJ-RELOCS %s
 ; RUN: llvm-objdump --source %t.o | FileCheck --check-prefix=OBJDUMP-SOURCE %s
 ; RUN: llvm-dwarfdump --debug-info --debug-line %t.o | \
 ; RUN:     FileCheck -check-prefix=DWARF-DUMP %s
 
 ; Check that we actually have relocations, otherwise this is kind of pointless.
 ; READOBJ-RELOCS:  Section ({{.*}}) .rela.debug_info {
-; READOBJ-RELOCS:    0x1B R_RISCV_ADD32 - 0x0
-; READOBJ-RELOCS-NEXT:    0x1B R_RISCV_SUB32 - 0x0
+; READOBJ-RELOCS:    0x11 R_RISCV_32 - 0x0
 ; READOBJ-RELOCS:  Section ({{.*}}) .rela.debug_frame {
 ; READOBJ-RELOCS:    0x20 R_RISCV_ADD32 - 0x0
 ; READOBJ-RELOCS-NEXT:    0x20 R_RISCV_SUB32 - 0x0
 ; READOBJ-RELOCS:  Section ({{.*}}) .rela.debug_line {
 ; READOBJ-RELOCS:    0x5A R_RISCV_ADD16 - 0x0
 ; READOBJ-RELOCS-NEXT:    0x5A R_RISCV_SUB16 - 0x0
 
 ; Check that we can print the source, even with relocations.