maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

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!

llvm-svn: 319563
This commit is contained in:
Rafael Espindola 2017-12-01 18:14:14 +00:00
parent 57783730fd
commit b70824c1b8
2 changed files with 1 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
lld/ELF/Writer.cpp
View File

@ -1465,22 +1465,6 @@ static uint64_t computeFlags(uint64_t Flags) {
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<PhdrEntry *> Writer<ELFT>::createPhdrs() {
bool InRelroPhdr = false;
bool IsRelroFinished = false;
for (OutputSection *Sec : OutputSections) {
if (!needsPtLoad(Sec) || isOutputSectionZeroSize(Sec))
if (!needsPtLoad(Sec))
continue;
if (isRelroSection(Sec)) {
InRelroPhdr = true;

62
lld/test/ELF/relro-non-contiguous-zerosize.s
View File

@ -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: