[ELF] Compress debug sections after assignAddresses and support custom layout

[jhenderson]

Summary

Previously, in rL320472, I moved the calculation of section offsets and sizes for compressed debug sections into maybeCompress, which happens before assignAddresses, so that the compression had the required information. However, I failed to take account of relocations that patch such sections. This had two effects:

1. A race condition existed when a debug section referred to a different debug section (see PR35788).
2. References to symbols in non-debug sections would be patched incorrectly. This is because the addresses of such symbols are not calculated until after assignAddresses (this was a partial regression caused by rL320472, but they could still have been broken before, in the event that a custom layout was used in a linker script).

assignAddresses does not need to know about the output section size of non-allocatable sections, because they do not affect the value of Dot. This means that there is no longer a reason not to support custom layout of compressed debug sections, as far as I'm aware (if anybody else knows of a reason why we cannot, then please say so!). These two points allow for delaying when maybeCompress can be called, removing the need for the loop I added to calculate the section size, and therefore the race condition. Furthermore, by delaying, we fix the issues of relocations getting incorrect symbol values, because they have now all been finalized.

[jhenderson]

Add comments in tests as requested by rafael on the mailing list.

[jhenderson]

@rafael gave a LGTM on the mailing list.

[jhenderson]

Commited in r321986.

ELF/LinkerScript.cpp

void LinkerScript::assignOffsets(OutputSection *Sec) {
 
   if (Sec->LMAExpr) {
     uint64_t D = Dot;
     Ctx->LMAOffset = [=] { return Sec->LMAExpr().getValue() - D; };
   }
 
   switchTo(Sec);
 
-  // We do not support custom layout for compressed debug sectons.
-  // At this point we already know their size and have compressed content.
-  if (Ctx->OutSec->Flags & SHF_COMPRESSED)
-    return;
-
   // The Size previously denoted how many InputSections had been added to this
   // section, and was used for sorting SHF_LINK_ORDER sections. Reset it to
   // compute the actual size value.
   Sec->Size = 0;
 
   // We visited SectionsCommands from processSectionCommands to
   // layout sections. Now, we visit SectionsCommands again to fix
   // section offsets.

ELF/OutputSections.cpp

 template <class ELFT> void OutputSection::maybeCompress() {
   typedef typename ELFT::Chdr Elf_Chdr;
 
   // Compress only DWARF debug sections.
   if (!Config->CompressDebugSections || (Flags & SHF_ALLOC) ||
       !Name.startswith(".debug_"))
     return;
 
-  // Calculate the section offsets and size pre-compression.
-  Size = 0;
-  for (BaseCommand *Cmd : SectionCommands)
-    if (auto *ISD = dyn_cast<InputSectionDescription>(Cmd))
-      for (InputSection *IS : ISD->Sections) {
-        IS->OutSecOff = alignTo(Size, IS->Alignment);
-        this->Size = IS->OutSecOff + IS->getSize();
-      }
-
   // Create a section header.
   ZDebugHeader.resize(sizeof(Elf_Chdr));
   auto *Hdr = reinterpret_cast<Elf_Chdr *>(ZDebugHeader.data());
   Hdr->ch_type = ELFCOMPRESS_ZLIB;
   Hdr->ch_size = Size;
   Hdr->ch_addralign = Alignment;
 
   // Write section contents to a temporary buffer and compress it.

ELF/Writer.cpp

template <class ELFT> void Writer<ELFT>::run() {
   // Now that we have a complete set of output sections. This function
   // completes section contents. For example, we need to add strings
   // to the string table, and add entries to .got and .plt.
   // finalizeSections does that.
   finalizeSections();
   if (errorCount())
     return;
 
+  Script->assignAddresses();
+
   // If -compressed-debug-sections is specified, we need to compress
   // .debug_* sections. Do it right now because it changes the size of
   // output sections.
   parallelForEach(OutputSections,
                   [](OutputSection *Sec) { Sec->maybeCompress<ELFT>(); });
 
-  Script->assignAddresses();
   Script->allocateHeaders(Phdrs);
 
   // Remove empty PT_LOAD to avoid causing the dynamic linker to try to mmap a
   // 0 sized region. This has to be done late since only after assignAddresses
   // we know the size of the sections.
   removeEmptyPTLoad();
 
   if (!Config->OFormatBinary)

test/ELF/Inputs/compress-debug.s

+.text
+.fill 0x44
+
+.section .debug_info,"",@progbits
+.fill 0x43

test/ELF/compress-debug-sections-reloc.s

+# REQUIRES: x86, zlib
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %S/Inputs/compress-debug.s -o %t2.o
+# RUN: ld.lld %t2.o %t.o -o %t1 --compress-debug-sections=zlib -Ttext=0
+# RUN: llvm-dwarfdump %t1 -debug-str | FileCheck %s
+# These two checks correspond to the patched values of a_sym and a_debug_sym.
+# D = 0x44 - address of .text input section for this file (the start address of
+#     .text is 0 as requested on the command line, and the size of the
+#   preceding .text in the other input file is 0x44).
+# C = 0x43 - offset of .debug_info section for this file (the size of
+#     the preceding .debug_info from the other input file is 0x43).
+# CHECK: 0x00000000: "D"
+# CHECK: 0x00000004: "C"
+
+.text
+a_sym:
+nop
+
+.section .debug_str,"",@progbits
+.long a_sym
+.long a_debug_sym
+
+.section .debug_info,"",@progbits
+a_debug_sym:
+.long 0x88776655

test/ELF/linkerscript/compress-debug-sections-custom.s

+# REQUIRES: x86, zlib
+
+# RUN: echo "SECTIONS { \
+# RUN:          .text : { . += 0x10; *(.text) } \
+# RUN:          .debug_str : { . += 0x10; *(.debug_str) } \
+# RUN:          .debug_info : { . += 0x10; *(.debug_info) } \
+# RUN:          }" > %t.script
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %S/../Inputs/compress-debug.s -o %t2.o
+# RUN: ld.lld %t2.o %t.o -o %t1 --compress-debug-sections=zlib -T %t.script
+# RUN: llvm-dwarfdump %t1 -debug-str | FileCheck %s
+# These two checks correspond to the patched values of a_sym and a_debug_sym.
+# T = 0x54 - address of .text input section for this file (the start address of
+#     .text is 0 by default, the size of the preceding .text in the other input
+#   file is 0x44, and the linker script adds an additional 0x10).
+# S = 0x53 - offset of .debug_info section for this file (the size of
+#     the preceding .debug_info from the other input file is 0x43, and the
+#   linker script adds an additional 0x10).
+# Also note that the .debug_str offsets are also offset by 0x10, as directed by
+# the linker script.
+# CHECK: 0x00000010: "T"
+# CHECK: 0x00000014: "S"
+
+.text
+a_sym:
+nop
+
+.section .debug_str,"",@progbits
+.long a_sym
+.long a_debug_sym
+
+.section .debug_info,"",@progbits
+a_debug_sym:
+.long 0x88776655