Eliminate .{,z}debug_gnu_pub{names,types} sections as early as possible.

[ruiu]

Summary

Previously, if --gdb-index is not given, we copy .debug_gnu_pub{names,types}
sections to an output file, which is just a waste of disk space.
If the input sections are compressed, doing it also wastes CPU and memory
because, in order to copy them, we need to uncompress them first.

[dblaikie]

It's possible another tool (gdb itself, for example) could read these sections and construct an index themselves - just later on, but still more efficient than parsing all the DWARF, etc. So I'm not sure if this should be the default or only behavior (not saying it shouldn't be - I'm genuinely unsure).

Did you find this behavior to be a problem in some real-world case? Presumably the user would be better off turning off pubnames to reduce object size, rather than producing them and not using them?

[ruiu]

What I found is lld consumes a lot of memory when we feed object files containing .zdebug_gnu_pub{names,types} sections because it uncompresses them in memory. I don't know how realistic that scenario is, but that's at least I tried.

Currently, if no --gdb-index is given, lld just concatenates .{,z}debug_gnu_pub{names,types} by section name because that's the default behavior of the linker. Can external tools consume such concatenated sections?

[ruiu]

What I found is lld consumes a lot of memory when we feed object files containing .zdebug_gnu_pub{names,types} sections

... even if --gdb-index is not given

[dblaikie]

In D52241#1238637, @ruiu wrote:

What I found is lld consumes a lot of memory when we feed object files containing .zdebug_gnu_pub{names,types} sections because it uncompresses them in memory. I don't know how realistic that scenario is, but that's at least I tried.

Currently, if no --gdb-index is given, lld just concatenates .{,z}debug_gnu_pub{names,types} by section name because that's the default behavior of the linker. Can external tools consume such concatenated sections?

Yep - like most DWARF sections (I think maybe only the apple accelerator tables missed this), they're designed to be concatenated safely - they include a relocation to the CU they index in their header. So each chunk can be appropriately attributed.

[ruiu]

Ping.

[MaskRay]

The gdb command save gdb-index can produce symbol-file.gdb-index, but it seems it does not need .debug_gnu_pub{names,types}

https://sourceware.org/git/?p=binutils-gdb.git;a=blob;hb=7235dd9f9092d719121a635f73ae2c72102f0263;f=gdb/dwarf-index-write.c#l1715

In the binutils-gdb repo:

% rg -l debug_gnu_pubnames
gold/ChangeLog-0815
binutils/dwarf.c
binutils/ChangeLog-2013
gold/testsuite/dwp_test_1.s
gold/testsuite/dwp_test_1b.s
gold/testsuite/dwp_test_main.s
gold/testsuite/dwp_test_2.s
binutils/doc/debug.options.texi
debug/binutils/doc/objdump.1
debug/binutils/doc/readelf.1
gdb/testsuite/gdb.dwarf2/fission-loclists-pie.S
gdb/testsuite/gdb.dwarf2/fission-base.S
gdb/testsuite/gdb.dwarf2/fission-loclists.S

The three gdb tests mention .debug_gnu_pubnames just because they are created by -gsplit-dwarf, not because that gdb itself does anything with them.

This looks fine. For extra safeguard you may ask if .debug_gnu_pubnames is useful on the gdb mailing list [e-mail]

[ruiu]

I'll submit this patch soon if there's no further comment.

[dblaikie]

In D52241#1251854, @MaskRay wrote:

The gdb command save gdb-index can produce symbol-file.gdb-index, but it seems it does not need .debug_gnu_pub{names,types}

Doesn't need it - but the data can be used (not sure if GDB does use it, admittedly - but the data is meaningful even without a linker index builder) so it seems a bit weird to me to have the linker silently strip it out.

In D52241#1254358, @ruiu wrote:

I'll submit this patch soon if there's no further comment.

Still feels strange to me to strip a perfectly usable section silently like this. If the user doesn't want these sections they can pass -gno-pubnames to the compiler and avoid generating them in the first place.

[ruiu]

I submitted https://reviews.llvm.org/rL343979 which also reduce memory consumption for .zdebug_gnu_pub{names,types} sections, so I don't think we need this anymore.

lld/ELF/InputFiles.cpp

InputSectionBase *ObjFile<ELFT>::createInputSection(const Elf_Shdr &Sec) {
   // The linkonce feature is a sort of proto-comdat. Some glibc i386 object
   // files contain definitions of symbol "__x86.get_pc_thunk.bx" in linkonce
   // sections. Drop those sections to avoid duplicate symbol errors.
   // FIXME: This is glibc PR20543, we should remove this hack once that has been
   // fixed for a while.
   if (Name.startswith(".gnu.linkonce."))
     return &InputSection::Discarded;
 
+  // We create a .gdb_index section from .debug_gnu_pub{names,types}.
+  // If --gdb-index is not given, they are useless. So eliminate them early.
+  if (!Config->GdbIndex &&
+      (Name == ".debug_gnu_pubnames" || Name == ".zdebug_gnu_pubnames" ||
+       Name == ".debug_gnu_pubtypes" || Name == ".zdebug_gnu_pubtypes"))
+    return &InputSection::Discarded;
+
   // If we are creating a new .build-id section, strip existing .build-id
   // sections so that the output won't have more than one .build-id.
   // This is not usually a problem because input object files normally don't
   // have .build-id sections, but you can create such files by
   // "ld.{bfd,gold,lld} -r --build-id", and we want to guard against it.
   if (Name == ".note.gnu.build-id" && Config->BuildId != BuildIdKind::None)
     return &InputSection::Discarded;
 

lld/test/ELF/debug-gnu-pubnames.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 
 # RUN: ld.lld %t.o -o %t1.exe
 # RUN: llvm-readobj -sections %t1.exe | FileCheck %s
-# CHECK: .debug_gnu_pubnames
-# CHECK: .debug_gnu_pubtypes
 
 # RUN: ld.lld -gdb-index %t.o -o %t2.exe
-# RUN: llvm-readobj -sections %t2.exe | FileCheck %s --check-prefix=GDB
-# GDB-NOT: .debug_gnu_pubnames
-# GDB-NOT: .debug_gnu_pubtypes
+# RUN: llvm-readobj -sections %t2.exe | FileCheck %s
+
+# CHECK-NOT: .debug_gnu_pubnames
+# CHECK-NOT: .debug_gnu_pubtypes
 
 .section .debug_gnu_pubnames,"",@progbits
 .long 0
 
 .section .debug_gnu_pubtypes,"",@progbits
 .long 0