[lld] r319563 - Revert r318924 Skip over empty sections when checking for contiguous relro
Rafael Espindola via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 1 10:14:14 PST 2017
Author: rafael
Date: Fri Dec 1 10:14:14 2017
New Revision: 319563
URL: http://llvm.org/viewvc/llvm-project?rev=319563&view=rev
Log:
Revert r318924 Skip over empty sections when checking for contiguous relro
PR35478 https://bugs.llvm.org/show_bug.cgi?id=35478 points out a flaw
in the implementation of r318924 from D40364. The implementation
depends on the Size field being set or the SyntheticSection::empty()
being accurate. These functions are not reliable as some linker script
commands that have yet to be processed may affect the results, causing
some non-zero size sections to be reported as zero size.
I think the first step is to revert r318924 and come up with a better
solution for the underlying problem rather than trying to layer more
heuristics onto the zero sized output section.
Chances are I'll be out of office by the time anyone sees this so feel
free to commit the revert if you agree with me.
Fixes PR35478
Current thoughts on the underlying problem:
Revisiting the motivation for adding the zero size check in the first
place; it was to prevent 0 sized SyntheticSections that a user does
not have full control over from needlessly breaking the PT_GNU_RELRO,
rather than trying to accommodate arbitrarily complex linker
scripts. Looking at the code, it looks like
removeUnusedSyntheticSections() should remove zero sized synthetic
sections. It does, but it doesn't set the Parent to nullptr, this has
the side effect that Sec == InX::BssRelRo->getParent() will make the
parent OutputSection of InX::BssRelRo RelRo even if there is no
InX::BssRelRo.
I tried a quick experiment with setting the Parent to nullptr and this
flushed out a few interesting test failures, it feels like playing
Jenga with every change:
In the isRelroSection() we have to consider the case where there
is no .plt and .plt.got but there is a ifunc plt with accompanying
(ifunc .got or .plt.got)
The PPC64 has PltHeaderSize == 0. Unfortunately HeaderSize == 0 is
used to choose between the ifunc plt or normal plt. We seem to get
away with this at the moment, but tests start to fail when Parent
is set to nullptr for the .got.plt.
The InX::BssRelRo and InX::Bss never get their sizes set and they
are always removed by removeUnusedSyntheticSections(), their
purpose seems to be as some kind of proxy for add .bss or
.bss.relro InputSections into their parent OutputSections, they
therefore don't behave like other SyntheticSections anyway.
My thinking is that some work is needed to make sure that the Sec ==
SyntheticSection->getParent() does a bit more checking before
returning true, particularly for InX::BssRelRo as that has special
behaviour. I'll hope to post something for review as soon as possible.
Patch by Peter Smith!
Removed:
lld/trunk/test/ELF/relro-non-contiguous-zerosize.s
Modified:
lld/trunk/ELF/Writer.cpp
Modified: lld/trunk/ELF/Writer.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Writer.cpp?rev=319563&r1=319562&r2=319563&view=diff
==============================================================================
--- lld/trunk/ELF/Writer.cpp (original)
+++ lld/trunk/ELF/Writer.cpp Fri Dec 1 10:14:14 2017
@@ -1465,22 +1465,6 @@ static uint64_t computeFlags(uint64_t Fl
return Flags;
}
-// Prior to finalizeContents() an OutputSection containing SyntheticSections
-// may have 0 Size, but contain SyntheticSections that haven't had their size
-// calculated yet. We must use SyntheticSection->empty() for these sections.
-static bool isOutputSectionZeroSize(const OutputSection* Sec) {
- if (Sec->Size > 0)
- return false;
- for (BaseCommand *BC : Sec->SectionCommands) {
- if (auto *ISD = dyn_cast<InputSectionDescription>(BC))
- for (InputSection *IS : ISD->Sections)
- if (SyntheticSection *SS = dyn_cast<SyntheticSection>(IS))
- if (!SS->empty())
- return false;
- }
- return true;
-}
-
// Decide which program headers to create and which sections to include in each
// one.
template <class ELFT> std::vector<PhdrEntry *> Writer<ELFT>::createPhdrs() {
@@ -1546,7 +1530,7 @@ template <class ELFT> std::vector<PhdrEn
bool InRelroPhdr = false;
bool IsRelroFinished = false;
for (OutputSection *Sec : OutputSections) {
- if (!needsPtLoad(Sec) || isOutputSectionZeroSize(Sec))
+ if (!needsPtLoad(Sec))
continue;
if (isRelroSection(Sec)) {
InRelroPhdr = true;
Removed: lld/trunk/test/ELF/relro-non-contiguous-zerosize.s
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/relro-non-contiguous-zerosize.s?rev=319562&view=auto
==============================================================================
--- lld/trunk/test/ELF/relro-non-contiguous-zerosize.s (original)
+++ lld/trunk/test/ELF/relro-non-contiguous-zerosize.s (removed)
@@ -1,62 +0,0 @@
-// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/shared.s -o %t.o
-// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/copy-in-shared.s -o %t2.o
-// RUN: ld.lld -shared %t.o %t2.o -o %t.so
-
-// Check that we ignore zero sized non-relro sections that are covered by the
-// range of addresses covered by the PT_GNU_RELRO header.
-// Check that we ignore zero sized relro sections that are disjoint from the
-// range of addresses covered by the PT_GNU_RELRO header.
-// REQUIRES: x86
-
-// RUN: echo "SECTIONS { \
-// RUN: .ctors : { *(.ctors) } \
-// RUN: .large1 : { *(.large1) } \
-// RUN: .dynamic : { *(.dynamic) } \
-// RUN: .zero_size : { *(.zero_size) } \
-// RUN: .jcr : { *(.jcr) } \
-// RUN: .got.plt : { *(.got.plt) } \
-// RUN: .large2 : { *(.large2) } \
-// RUN: .data.rel.ro : { *(.data.rel.ro.*) } \
-// RUN: } " > %t.script
-// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t3.o
-// RUN: ld.lld %t3.o %t.so -o %t --script=%t.script
-// RUN: llvm-readobj -program-headers %t | FileCheck %s
-
-// CHECK: Type: PT_GNU_RELRO
-// CHECK-NEXT: Offset:
-// CHECK-NEXT: VirtualAddress:
-// CHECK-NEXT: PhysicalAddress:
-// CHECK-NEXT: FileSize:
-// CHECK-NEXT: MemSize: 4096
-
- .section .text, "ax", @progbits
- .global _start
- .global bar
- .global foo
-_start:
- callq bar
-
- // page size non-relro sections that would alter PT_GNU_RELRO header
- // MemSize if counted as part of relro.
- .section .large1, "aw", @progbits
- .space 4 * 1024
-
- .section .large2, "aw", @progbits
- .space 4 * 1024
-
- // empty relro section
- .section .ctors, "aw", @progbits
-
- // non-empty relro section
- .section .jcr, "aw", @progbits
- .quad 0
-
- // empty non-relro section
- .section .zero_size, "aw", @progbits
- .global sym
-sym:
-
- // empty relro section
- .section .data.rel.ro, "aw", @progbits
- .global sym2
-sym2:
More information about the llvm-commits
mailing list