[lld] r321986 - [ELF] Compress debug sections after assignAddresses and support custom layout
Hans Wennborg via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 16 07:47:51 PST 2018
Merged in r322565.
Thanks,
Hans
On Tue, Jan 16, 2018 at 4:41 PM, Rafael Avila de Espindola
<rafael.espindola at gmail.com> wrote:
> LGTM to the branch.
>
> Cheers,
> Rafael
>
> Hans Wennborg <hans at chromium.org> writes:
>
>> Rafael, is this OK for the branch?
>>
>> On Mon, Jan 8, 2018 at 3:37 PM, James Henderson
>> <jh7370.2008 at my.bristol.ac.uk> wrote:
>>> Hi Hans,
>>>
>>> Could this commit be merged into the 6.0 release branch, as it fixes
>>> PR35788.
>>>
>>> Regards,
>>>
>>> James Henderson
>>>
>>> On 8 January 2018 at 10:17, James Henderson via llvm-commits
>>> <llvm-commits at lists.llvm.org> wrote:
>>>>
>>>> Author: jhenderson
>>>> Date: Mon Jan 8 02:17:03 2018
>>>> New Revision: 321986
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=321986&view=rev
>>>> Log:
>>>> [ELF] Compress debug sections after assignAddresses and support custom
>>>> layout
>>>>
>>>> Previously, in r320472, 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 r320472, 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. These two points allow for
>>>> delaying when maybeCompress can be called, removing the need for the loop
>>>> I
>>>> previously 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.
>>>>
>>>> Added:
>>>> lld/trunk/test/ELF/Inputs/compress-debug.s
>>>> lld/trunk/test/ELF/compress-debug-sections-reloc.s
>>>> lld/trunk/test/ELF/linkerscript/compress-debug-sections-custom.s
>>>> Modified:
>>>> lld/trunk/ELF/LinkerScript.cpp
>>>> lld/trunk/ELF/OutputSections.cpp
>>>> lld/trunk/ELF/Writer.cpp
>>>>
>>>> Modified: lld/trunk/ELF/LinkerScript.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/LinkerScript.cpp?rev=321986&r1=321985&r2=321986&view=diff
>>>>
>>>> ==============================================================================
>>>> --- lld/trunk/ELF/LinkerScript.cpp (original)
>>>> +++ lld/trunk/ELF/LinkerScript.cpp Mon Jan 8 02:17:03 2018
>>>> @@ -669,11 +669,6 @@ void LinkerScript::assignOffsets(OutputS
>>>>
>>>> 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.
>>>>
>>>> Modified: lld/trunk/ELF/OutputSections.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/OutputSections.cpp?rev=321986&r1=321985&r2=321986&view=diff
>>>>
>>>> ==============================================================================
>>>> --- lld/trunk/ELF/OutputSections.cpp (original)
>>>> +++ lld/trunk/ELF/OutputSections.cpp Mon Jan 8 02:17:03 2018
>>>> @@ -183,15 +183,6 @@ template <class ELFT> void OutputSection
>>>> !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());
>>>>
>>>> Modified: lld/trunk/ELF/Writer.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Writer.cpp?rev=321986&r1=321985&r2=321986&view=diff
>>>>
>>>> ==============================================================================
>>>> --- lld/trunk/ELF/Writer.cpp (original)
>>>> +++ lld/trunk/ELF/Writer.cpp Mon Jan 8 02:17:03 2018
>>>> @@ -427,13 +427,14 @@ template <class ELFT> void Writer<ELFT>:
>>>> 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
>>>>
>>>> Added: lld/trunk/test/ELF/Inputs/compress-debug.s
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/Inputs/compress-debug.s?rev=321986&view=auto
>>>>
>>>> ==============================================================================
>>>> --- lld/trunk/test/ELF/Inputs/compress-debug.s (added)
>>>> +++ lld/trunk/test/ELF/Inputs/compress-debug.s Mon Jan 8 02:17:03 2018
>>>> @@ -0,0 +1,5 @@
>>>> +.text
>>>> +.fill 0x44
>>>> +
>>>> +.section .debug_info,"", at progbits
>>>> +.fill 0x43
>>>>
>>>> Added: lld/trunk/test/ELF/compress-debug-sections-reloc.s
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/compress-debug-sections-reloc.s?rev=321986&view=auto
>>>>
>>>> ==============================================================================
>>>> --- lld/trunk/test/ELF/compress-debug-sections-reloc.s (added)
>>>> +++ lld/trunk/test/ELF/compress-debug-sections-reloc.s Mon Jan 8 02:17:03
>>>> 2018
>>>> @@ -0,0 +1,26 @@
>>>> +# 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,"", at progbits
>>>> +.long a_sym
>>>> +.long a_debug_sym
>>>> +
>>>> +.section .debug_info,"", at progbits
>>>> +a_debug_sym:
>>>> +.long 0x88776655
>>>>
>>>> Added: lld/trunk/test/ELF/linkerscript/compress-debug-sections-custom.s
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/linkerscript/compress-debug-sections-custom.s?rev=321986&view=auto
>>>>
>>>> ==============================================================================
>>>> --- lld/trunk/test/ELF/linkerscript/compress-debug-sections-custom.s
>>>> (added)
>>>> +++ lld/trunk/test/ELF/linkerscript/compress-debug-sections-custom.s Mon
>>>> Jan 8 02:17:03 2018
>>>> @@ -0,0 +1,35 @@
>>>> +# 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,"", at progbits
>>>> +.long a_sym
>>>> +.long a_debug_sym
>>>> +
>>>> +.section .debug_info,"", at progbits
>>>> +a_debug_sym:
>>>> +.long 0x88776655
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>>
More information about the llvm-commits
mailing list