<div dir="ltr"><div><div><div>Hi Hans,<br><br></div>Could this commit be merged into the 6.0 release branch, as it fixes PR35788.<br><br></div>Regards,<br><br></div>James Henderson<br><div><div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On 8 January 2018 at 10:17, James Henderson via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: jhenderson<br>
Date: Mon Jan  8 02:17:03 2018<br>
New Revision: 321986<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=321986&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=321986&view=rev</a><br>
Log:<br>
[ELF] Compress debug sections after assignAddresses and support custom layout<br>
<br>
Previously, in r320472, I moved the calculation of section offsets and sizes<br>
for compressed debug sections into maybeCompress, which happens before<br>
assignAddresses, so that the compression had the required information. However,<br>
I failed to take account of relocations that patch such sections. This had two<br>
effects:<br>
<br>
1. A race condition existed when a debug section referred to a different debug<br>
section (see PR35788).<br>
2. References to symbols in non-debug sections would be patched incorrectly.<br>
This is because the addresses of such symbols are not calculated until after<br>
assignAddresses (this was a partial regression caused by r320472, but they<br>
could still have been broken before, in the event that a custom layout was used<br>
in a linker script).<br>
<br>
assignAddresses does not need to know about the output section size of<br>
non-allocatable sections, because they do not affect the value of Dot. This<br>
means that there is no longer a reason not to support custom layout of<br>
compressed debug sections, as far as I'm aware. These two points allow for<br>
delaying when maybeCompress can be called, removing the need for the loop I<br>
previously added to calculate the section size, and therefore the race<br>
condition. Furthermore, by delaying, we fix the issues of relocations getting<br>
incorrect symbol values, because they have now all been finalized.<br>
<br>
Added:<br>
    lld/trunk/test/ELF/Inputs/comp<wbr>ress-debug.s<br>
    lld/trunk/test/ELF/compress-de<wbr>bug-sections-reloc.s<br>
    lld/trunk/test/ELF/linkerscrip<wbr>t/compress-debug-sections-<wbr>custom.s<br>
Modified:<br>
    lld/trunk/ELF/LinkerScript.cpp<br>
    lld/trunk/ELF/OutputSections.c<wbr>pp<br>
    lld/trunk/ELF/Writer.cpp<br>
<br>
Modified: lld/trunk/ELF/LinkerScript.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/LinkerScript.cpp?rev=321986&r1=321985&r2=321986&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/lld/trunk/ELF/LinkerScri<wbr>pt.cpp?rev=321986&r1=321985&<wbr>r2=321986&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- lld/trunk/ELF/LinkerScript.cpp (original)<br>
+++ lld/trunk/ELF/LinkerScript.cpp Mon Jan  8 02:17:03 2018<br>
@@ -669,11 +669,6 @@ void LinkerScript::assignOffsets(Ou<wbr>tputS<br>
<br>
   switchTo(Sec);<br>
<br>
-  // We do not support custom layout for compressed debug sectons.<br>
-  // At this point we already know their size and have compressed content.<br>
-  if (Ctx->OutSec->Flags & SHF_COMPRESSED)<br>
-    return;<br>
-<br>
   // The Size previously denoted how many InputSections had been added to this<br>
   // section, and was used for sorting SHF_LINK_ORDER sections. Reset it to<br>
   // compute the actual size value.<br>
<br>
Modified: lld/trunk/ELF/OutputSections.c<wbr>pp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/OutputSections.cpp?rev=321986&r1=321985&r2=321986&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/lld/trunk/ELF/OutputSect<wbr>ions.cpp?rev=321986&r1=321985&<wbr>r2=321986&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- lld/trunk/ELF/OutputSections.c<wbr>pp (original)<br>
+++ lld/trunk/ELF/OutputSections.c<wbr>pp Mon Jan  8 02:17:03 2018<br>
@@ -183,15 +183,6 @@ template <class ELFT> void OutputSection<br>
       !Name.startswith(".debug_"))<br>
     return;<br>
<br>
-  // Calculate the section offsets and size pre-compression.<br>
-  Size = 0;<br>
-  for (BaseCommand *Cmd : SectionCommands)<br>
-    if (auto *ISD = dyn_cast<InputSectionDescripti<wbr>on>(Cmd))<br>
-      for (InputSection *IS : ISD->Sections) {<br>
-        IS->OutSecOff = alignTo(Size, IS->Alignment);<br>
-        this->Size = IS->OutSecOff + IS->getSize();<br>
-      }<br>
-<br>
   // Create a section header.<br>
   ZDebugHeader.resize(sizeof(El<wbr>f_Chdr));<br>
   auto *Hdr = reinterpret_cast<Elf_Chdr *>(ZDebugHeader.data());<br>
<br>
Modified: lld/trunk/ELF/Writer.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Writer.cpp?rev=321986&r1=321985&r2=321986&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/lld/trunk/ELF/Writer.cpp<wbr>?rev=321986&r1=321985&r2=32198<wbr>6&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- lld/trunk/ELF/Writer.cpp (original)<br>
+++ lld/trunk/ELF/Writer.cpp Mon Jan  8 02:17:03 2018<br>
@@ -427,13 +427,14 @@ template <class ELFT> void Writer<ELFT>:<br>
   if (errorCount())<br>
     return;<br>
<br>
+  Script->assignAddresses();<br>
+<br>
   // If -compressed-debug-sections is specified, we need to compress<br>
   // .debug_* sections. Do it right now because it changes the size of<br>
   // output sections.<br>
   parallelForEach(OutputSection<wbr>s,<br>
                   [](OutputSection *Sec) { Sec->maybeCompress<ELFT>(); });<br>
<br>
-  Script->assignAddresses();<br>
   Script->allocateHeaders(<wbr>Phdrs);<br>
<br>
   // Remove empty PT_LOAD to avoid causing the dynamic linker to try to mmap a<br>
<br>
Added: lld/trunk/test/ELF/Inputs/comp<wbr>ress-debug.s<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/Inputs/compress-debug.s?rev=321986&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/lld/trunk/test/ELF/Input<wbr>s/compress-debug.s?rev=321986&<wbr>view=auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- lld/trunk/test/ELF/Inputs/comp<wbr>ress-debug.s (added)<br>
+++ lld/trunk/test/ELF/Inputs/comp<wbr>ress-debug.s Mon Jan  8 02:17:03 2018<br>
@@ -0,0 +1,5 @@<br>
+.text<br>
+.fill 0x44<br>
+<br>
+.section .debug_info,"",@progbits<br>
+.fill 0x43<br>
<br>
Added: lld/trunk/test/ELF/compress-de<wbr>bug-sections-reloc.s<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/compress-debug-sections-reloc.s?rev=321986&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/lld/trunk/test/ELF/compr<wbr>ess-debug-sections-reloc.s?<wbr>rev=321986&view=auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- lld/trunk/test/ELF/compress-de<wbr>bug-sections-reloc.s (added)<br>
+++ lld/trunk/test/ELF/compress-de<wbr>bug-sections-reloc.s Mon Jan  8 02:17:03 2018<br>
@@ -0,0 +1,26 @@<br>
+# REQUIRES: x86, zlib<br>
+<br>
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o<br>
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %S/Inputs/compress-debug.s -o %t2.o<br>
+# RUN: ld.lld %t2.o %t.o -o %t1 --compress-debug-sections=zlib -Ttext=0<br>
+# RUN: llvm-dwarfdump %t1 -debug-str | FileCheck %s<br>
+# These two checks correspond to the patched values of a_sym and a_debug_sym.<br>
+# D = 0x44 - address of .text input section for this file (the start address of<br>
+#     .text is 0 as requested on the command line, and the size of the<br>
+#        preceding .text in the other input file is 0x44).<br>
+# C = 0x43 - offset of .debug_info section for this file (the size of<br>
+#     the preceding .debug_info from the other input file is 0x43).<br>
+# CHECK: 0x00000000: "D"<br>
+# CHECK: 0x00000004: "C"<br>
+<br>
+.text<br>
+a_sym:<br>
+nop<br>
+<br>
+.section .debug_str,"",@progbits<br>
+.long a_sym<br>
+.long a_debug_sym<br>
+<br>
+.section .debug_info,"",@progbits<br>
+a_debug_sym:<br>
+.long 0x88776655<br>
<br>
Added: lld/trunk/test/ELF/linkerscrip<wbr>t/compress-debug-sections-<wbr>custom.s<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/linkerscript/compress-debug-sections-custom.s?rev=321986&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/lld/trunk/test/ELF/linke<wbr>rscript/compress-debug-section<wbr>s-custom.s?rev=321986&view=<wbr>auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- lld/trunk/test/ELF/linkerscrip<wbr>t/compress-debug-sections-<wbr>custom.s (added)<br>
+++ lld/trunk/test/ELF/linkerscrip<wbr>t/compress-debug-sections-<wbr>custom.s Mon Jan  8 02:17:03 2018<br>
@@ -0,0 +1,35 @@<br>
+# REQUIRES: x86, zlib<br>
+<br>
+# RUN: echo "SECTIONS { \<br>
+# RUN:          .text : { . += 0x10; *(.text) } \<br>
+# RUN:          .debug_str : { . += 0x10; *(.debug_str) } \<br>
+# RUN:          .debug_info : { . += 0x10; *(.debug_info) } \<br>
+# RUN:          }" > %t.script<br>
+<br>
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o<br>
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %S/../Inputs/compress-debug.s -o %t2.o<br>
+# RUN: ld.lld %t2.o %t.o -o %t1 --compress-debug-sections=zlib -T %t.script<br>
+# RUN: llvm-dwarfdump %t1 -debug-str | FileCheck %s<br>
+# These two checks correspond to the patched values of a_sym and a_debug_sym.<br>
+# T = 0x54 - address of .text input section for this file (the start address of<br>
+#     .text is 0 by default, the size of the preceding .text in the other input<br>
+#        file is 0x44, and the linker script adds an additional 0x10).<br>
+# S = 0x53 - offset of .debug_info section for this file (the size of<br>
+#     the preceding .debug_info from the other input file is 0x43, and the<br>
+#        linker script adds an additional 0x10).<br>
+# Also note that the .debug_str offsets are also offset by 0x10, as directed by<br>
+# the linker script.<br>
+# CHECK: 0x00000010: "T"<br>
+# CHECK: 0x00000014: "S"<br>
+<br>
+.text<br>
+a_sym:<br>
+nop<br>
+<br>
+.section .debug_str,"",@progbits<br>
+.long a_sym<br>
+.long a_debug_sym<br>
+<br>
+.section .debug_info,"",@progbits<br>
+a_debug_sym:<br>
+.long 0x88776655<br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div></div></div></div></div>