[lld] r292910 - [ELF] Correct sh_info for static symbol table
Rafael Espíndola via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 24 12:08:57 PST 2017
Looks like the cleanest (no conflict) way to backport this is to port 4 patches:
r292594: Do not crash when assign common symbol's values in script
r292789: Stop handling local symbols in a special way.
r292910: Correct sh_info for static symbol table
r292940: Fixed crash after incrementing end iterator.
r292594 is probably a good idea to port anyway. I can fix the
conflicts and leave r292789 out if you want. A combined patch to 4.0
is attached.
Cheers,
Rafael
On 24 January 2017 at 14:26, Rui Ueyama <ruiu at google.com> wrote:
> But if you do, please cherry pick r292940 too because this patch introduced
> a new issue of creating an invalid iterator.
>
> On Tue, Jan 24, 2017 at 11:25 AM, Rui Ueyama <ruiu at google.com> wrote:
>>
>> That's OK.
>>
>> On Tue, Jan 24, 2017 at 10:02 AM, Rafael Avila de Espindola
>> <rafael.espindola at gmail.com> wrote:
>>>
>>>
>>> Thanks!
>>>
>>> I think we should port this to 4.0. Is that OK?
>>>
>>> Cheers,
>>> Rafael
>>>
>>> Peter Smith via llvm-commits <llvm-commits at lists.llvm.org> writes:
>>>
>>> > Author: psmith
>>> > Date: Tue Jan 24 04:43:40 2017
>>> > New Revision: 292910
>>> >
>>> > URL: http://llvm.org/viewvc/llvm-project?rev=292910&view=rev
>>> > Log:
>>> > [ELF] Correct sh_info for static symbol table
>>> >
>>> > The sh_info field of the SHT_SYMTAB section holds the index for the
>>> > first non-local symbol. When there are global symbols that are output
>>> > with STB_LOCAL binding due to having hidden visibility or matching
>>> > the local version from a version script, the calculated value of
>>> > NumLocals + 1 does not account for them. This change accounts for
>>> > global symbols being output with local binding.
>>> >
>>> > Differential Revision: https://reviews.llvm.org/D28950
>>> >
>>> > Modified:
>>> > lld/trunk/ELF/SyntheticSections.cpp
>>> > lld/trunk/test/ELF/basic-mips.s
>>> > lld/trunk/test/ELF/basic-ppc.s
>>> >
>>> > Modified: lld/trunk/ELF/SyntheticSections.cpp
>>> > URL:
>>> > http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/SyntheticSections.cpp?rev=292910&r1=292909&r2=292910&view=diff
>>> >
>>> > ==============================================================================
>>> > --- lld/trunk/ELF/SyntheticSections.cpp (original)
>>> > +++ lld/trunk/ELF/SyntheticSections.cpp Tue Jan 24 04:43:40 2017
>>> > @@ -1073,11 +1073,13 @@ template <class ELFT> void SymbolTableSe
>>> >
>>> > if (!StrTabSec.isDynamic()) {
>>> > auto GlobBegin = Symbols.begin() + NumLocals;
>>> > - std::stable_sort(GlobBegin, Symbols.end(), [](const
>>> > SymbolTableEntry &L,
>>> > - const
>>> > SymbolTableEntry &R) {
>>> > - return L.Symbol->symbol()->computeBinding() == STB_LOCAL &&
>>> > - R.Symbol->symbol()->computeBinding() != STB_LOCAL;
>>> > - });
>>> > + auto It = std::stable_partition(
>>> > + GlobBegin, Symbols.end(), [](const SymbolTableEntry &S) {
>>> > + return S.Symbol->symbol()->computeBinding() == STB_LOCAL;
>>> > + });
>>> > + // update sh_info with number of Global symbols output with
>>> > computed
>>> > + // binding of STB_LOCAL
>>> > + this->OutSec->Info = this->Info = 1 + It - Symbols.begin();
>>> > return;
>>> > }
>>> >
>>> >
>>> > Modified: lld/trunk/test/ELF/basic-mips.s
>>> > URL:
>>> > http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/basic-mips.s?rev=292910&r1=292909&r2=292910&view=diff
>>> >
>>> > ==============================================================================
>>> > --- lld/trunk/test/ELF/basic-mips.s (original)
>>> > +++ lld/trunk/test/ELF/basic-mips.s Tue Jan 24 04:43:40 2017
>>> > @@ -176,7 +176,7 @@ __start:
>>> > # CHECK-NEXT: Offset: 0x20010
>>> > # CHECK-NEXT: Size: 48
>>> > # CHECK-NEXT: Link: 10
>>> > -# CHECK-NEXT: Info: 1
>>> > +# CHECK-NEXT: Info: 2
>>> > # CHECK-NEXT: AddressAlignment: 4
>>> > # CHECK-NEXT: EntrySize: 16
>>> > # CHECK-NEXT: }
>>> >
>>> > Modified: lld/trunk/test/ELF/basic-ppc.s
>>> > URL:
>>> > http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/basic-ppc.s?rev=292910&r1=292909&r2=292910&view=diff
>>> >
>>> > ==============================================================================
>>> > --- lld/trunk/test/ELF/basic-ppc.s (original)
>>> > +++ lld/trunk/test/ELF/basic-ppc.s Tue Jan 24 04:43:40 2017
>>> > @@ -178,7 +178,7 @@
>>> > // CHECK-NEXT: Offset: 0x2038
>>> > // CHECK-NEXT: Size: 32
>>> > // CHECK-NEXT: Link: 9
>>> > -// CHECK-NEXT: Info: 1
>>> > +// CHECK-NEXT: Info: 2
>>> > // CHECK-NEXT: AddressAlignment: 4
>>> > // CHECK-NEXT: EntrySize: 16
>>> > // CHECK-NEXT: SectionData (
>>> >
>>> >
>>> > _______________________________________________
>>> > llvm-commits mailing list
>>> > llvm-commits at lists.llvm.org
>>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>
>
-------------- next part --------------
diff --git a/ELF/InputFiles.h b/ELF/InputFiles.h
index 73dda7b..9588806 100644
--- a/ELF/InputFiles.h
+++ b/ELF/InputFiles.h
@@ -180,10 +180,6 @@ public:
// R_MIPS_GPREL16 / R_MIPS_GPREL32 relocations.
uint32_t MipsGp0 = 0;
- // The number is the offset in the string table. It will be used as the
- // st_name of the symbol.
- std::vector<std::pair<const DefinedRegular<ELFT> *, unsigned>> KeptLocalSyms;
-
// Name of source file obtained from STT_FILE symbol value,
// or empty string if there is no such symbol in object file
// symbol table.
diff --git a/ELF/InputSection.cpp b/ELF/InputSection.cpp
index 3580042..6b1e928 100644
--- a/ELF/InputSection.cpp
+++ b/ELF/InputSection.cpp
@@ -246,7 +246,8 @@ void InputSection<ELFT>::copyRelocations(uint8_t *Buf, ArrayRef<RelTy> Rels) {
if (Config->Rela)
P->r_addend = getAddend<ELFT>(Rel);
P->r_offset = RelocatedSection->getOffset(Rel.r_offset);
- P->setSymbolAndType(Body.DynsymIndex, Type, Config->Mips64EL);
+ P->setSymbolAndType(In<ELFT>::SymTab->getSymbolIndex(&Body), Type,
+ Config->Mips64EL);
}
}
diff --git a/ELF/LinkerScript.cpp b/ELF/LinkerScript.cpp
index 887ca12..3cc2353 100644
--- a/ELF/LinkerScript.cpp
+++ b/ELF/LinkerScript.cpp
@@ -918,12 +918,7 @@ const OutputSectionBase *LinkerScript<ELFT>::getSymbolSection(StringRef S) {
return CurOutSec ? CurOutSec : (*OutputSections)[0];
}
- if (auto *DR = dyn_cast_or_null<DefinedRegular<ELFT>>(Sym))
- return DR->Section ? DR->Section->OutSec : nullptr;
- if (auto *DS = dyn_cast_or_null<DefinedSynthetic>(Sym))
- return DS->Section;
-
- return nullptr;
+ return SymbolTableSection<ELFT>::getOutputSection(Sym);
}
// Returns indices of ELF headers containing specific section, identified
diff --git a/ELF/SyntheticSections.cpp b/ELF/SyntheticSections.cpp
index f09b60b..5d318c7 100644
--- a/ELF/SyntheticSections.cpp
+++ b/ELF/SyntheticSections.cpp
@@ -1065,22 +1065,21 @@ template <class ELFT> void SymbolTableSection<ELFT>::finalize() {
this->OutSec->Info = this->Info = NumLocals + 1;
this->OutSec->Entsize = this->Entsize;
- if (Config->Relocatable) {
- size_t I = NumLocals;
- for (const SymbolTableEntry &S : Symbols)
- S.Symbol->DynsymIndex = ++I;
+ if (Config->Relocatable)
return;
- }
if (!StrTabSec.isDynamic()) {
- std::stable_sort(
- Symbols.begin(), Symbols.end(),
- [](const SymbolTableEntry &L, const SymbolTableEntry &R) {
- return L.Symbol->symbol()->computeBinding() == STB_LOCAL &&
- R.Symbol->symbol()->computeBinding() != STB_LOCAL;
+ auto GlobBegin = Symbols.begin() + NumLocals;
+ auto It = std::stable_partition(
+ GlobBegin, Symbols.end(), [](const SymbolTableEntry &S) {
+ return S.Symbol->symbol()->computeBinding() == STB_LOCAL;
});
+ // update sh_info with number of Global symbols output with computed
+ // binding of STB_LOCAL
+ this->OutSec->Info = this->Info = 1 + (It - Symbols.begin());
return;
}
+
if (In<ELFT>::GnuHashTab)
// NB: It also sorts Symbols to meet the GNU hash table requirements.
In<ELFT>::GnuHashTab->addSymbols(Symbols);
@@ -1094,10 +1093,23 @@ template <class ELFT> void SymbolTableSection<ELFT>::finalize() {
S.Symbol->DynsymIndex = ++I;
}
-template <class ELFT> void SymbolTableSection<ELFT>::addSymbol(SymbolBody *B) {
+template <class ELFT> void SymbolTableSection<ELFT>::addGlobal(SymbolBody *B) {
Symbols.push_back({B, StrTabSec.addString(B->getName(), false)});
}
+template <class ELFT> void SymbolTableSection<ELFT>::addLocal(SymbolBody *B) {
+ assert(!StrTabSec.isDynamic());
+ ++NumLocals;
+ Symbols.push_back({B, StrTabSec.addString(B->getName())});
+}
+
+template <class ELFT>
+size_t SymbolTableSection<ELFT>::getSymbolIndex(SymbolBody *Body) {
+ auto I = llvm::find_if(
+ Symbols, [&](const SymbolTableEntry &E) { return E.Symbol == Body; });
+ return I - Symbols.begin() + 1;
+}
+
template <class ELFT> void SymbolTableSection<ELFT>::writeTo(uint8_t *Buf) {
Buf += sizeof(Elf_Sym);
@@ -1113,26 +1125,24 @@ template <class ELFT>
void SymbolTableSection<ELFT>::writeLocalSymbols(uint8_t *&Buf) {
// Iterate over all input object files to copy their local symbols
// to the output symbol table pointed by Buf.
- for (ObjectFile<ELFT> *File : Symtab<ELFT>::X->getObjectFiles()) {
- for (const std::pair<const DefinedRegular<ELFT> *, size_t> &P :
- File->KeptLocalSyms) {
- const DefinedRegular<ELFT> &Body = *P.first;
- InputSectionBase<ELFT> *Section = Body.Section;
- auto *ESym = reinterpret_cast<Elf_Sym *>(Buf);
-
- if (!Section) {
- ESym->st_shndx = SHN_ABS;
- ESym->st_value = Body.Value;
- } else {
- const OutputSectionBase *OutSec = Section->OutSec;
- ESym->st_shndx = OutSec->SectionIndex;
- ESym->st_value = OutSec->Addr + Section->getOffset(Body);
- }
- ESym->st_name = P.second;
- ESym->st_size = Body.template getSize<ELFT>();
- ESym->setBindingAndType(STB_LOCAL, Body.Type);
- Buf += sizeof(*ESym);
+
+ for (auto I = Symbols.begin(); I != Symbols.begin() + NumLocals; ++I) {
+ const DefinedRegular<ELFT> &Body = *cast<DefinedRegular<ELFT>>(I->Symbol);
+ InputSectionBase<ELFT> *Section = Body.Section;
+ auto *ESym = reinterpret_cast<Elf_Sym *>(Buf);
+
+ if (!Section) {
+ ESym->st_shndx = SHN_ABS;
+ ESym->st_value = Body.Value;
+ } else {
+ const OutputSectionBase *OutSec = Section->OutSec;
+ ESym->st_shndx = OutSec->SectionIndex;
+ ESym->st_value = OutSec->Addr + Section->getOffset(Body);
}
+ ESym->st_name = I->StrTabOffset;
+ ESym->st_size = Body.template getSize<ELFT>();
+ ESym->setBindingAndType(STB_LOCAL, Body.Type);
+ Buf += sizeof(*ESym);
}
}
@@ -1141,7 +1151,9 @@ void SymbolTableSection<ELFT>::writeGlobalSymbols(uint8_t *Buf) {
// Write the internal symbol table contents to the output symbol table
// pointed by Buf.
auto *ESym = reinterpret_cast<Elf_Sym *>(Buf);
- for (const SymbolTableEntry &S : Symbols) {
+
+ for (auto I = Symbols.begin() + NumLocals; I != Symbols.end(); ++I) {
+ const SymbolTableEntry &S = *I;
SymbolBody *Body = S.Symbol;
size_t StrOff = S.StrTabOffset;
diff --git a/ELF/SyntheticSections.h b/ELF/SyntheticSections.h
index dfefb38..df67e07 100644
--- a/ELF/SyntheticSections.h
+++ b/ELF/SyntheticSections.h
@@ -366,23 +366,26 @@ public:
void finalize() override;
void writeTo(uint8_t *Buf) override;
size_t getSize() const override { return getNumSymbols() * sizeof(Elf_Sym); }
- void addSymbol(SymbolBody *Body);
+ void addGlobal(SymbolBody *Body);
+ void addLocal(SymbolBody *Body);
StringTableSection<ELFT> &getStrTabSec() const { return StrTabSec; }
- unsigned getNumSymbols() const { return NumLocals + Symbols.size() + 1; }
+ unsigned getNumSymbols() const { return Symbols.size() + 1; }
+ size_t getSymbolIndex(SymbolBody *Body);
ArrayRef<SymbolTableEntry> getSymbols() const { return Symbols; }
- unsigned NumLocals = 0;
- StringTableSection<ELFT> &StrTabSec;
+ static const OutputSectionBase *getOutputSection(SymbolBody *Sym);
private:
void writeLocalSymbols(uint8_t *&Buf);
void writeGlobalSymbols(uint8_t *Buf);
- const OutputSectionBase *getOutputSection(SymbolBody *Sym);
-
// A vector of symbols and their string table offsets.
std::vector<SymbolTableEntry> Symbols;
+
+ StringTableSection<ELFT> &StrTabSec;
+
+ unsigned NumLocals = 0;
};
// Outputs GNU Hash section. For detailed explanation see:
diff --git a/ELF/Writer.cpp b/ELF/Writer.cpp
index 4f2f919..fbb05ef 100644
--- a/ELF/Writer.cpp
+++ b/ELF/Writer.cpp
@@ -455,11 +455,7 @@ template <class ELFT> void Writer<ELFT>::copyLocalSymbols() {
InputSectionBase<ELFT> *Sec = DR->Section;
if (!shouldKeepInSymtab<ELFT>(Sec, B->getName(), *B))
continue;
- ++In<ELFT>::SymTab->NumLocals;
- if (Config->Relocatable)
- B->DynsymIndex = In<ELFT>::SymTab->NumLocals;
- F->KeptLocalSyms.push_back(std::make_pair(
- DR, In<ELFT>::SymTab->StrTabSec.addString(B->getName())));
+ In<ELFT>::SymTab->addLocal(B);
}
}
}
@@ -1024,10 +1020,10 @@ template <class ELFT> void Writer<ELFT>::finalizeSections() {
if (!includeInSymtab<ELFT>(*Body))
continue;
if (In<ELFT>::SymTab)
- In<ELFT>::SymTab->addSymbol(Body);
+ In<ELFT>::SymTab->addGlobal(Body);
if (In<ELFT>::DynSymTab && S->includeInDynsym()) {
- In<ELFT>::DynSymTab->addSymbol(Body);
+ In<ELFT>::DynSymTab->addGlobal(Body);
if (auto *SS = dyn_cast<SharedSymbol<ELFT>>(Body))
if (SS->file()->isNeeded())
In<ELFT>::VerNeed->addSymbol(SS);
diff --git a/test/ELF/basic-mips.s b/test/ELF/basic-mips.s
index 67b58f8..57181ca 100644
--- a/test/ELF/basic-mips.s
+++ b/test/ELF/basic-mips.s
@@ -176,7 +176,7 @@ __start:
# CHECK-NEXT: Offset: 0x20010
# CHECK-NEXT: Size: 48
# CHECK-NEXT: Link: 10
-# CHECK-NEXT: Info: 1
+# CHECK-NEXT: Info: 2
# CHECK-NEXT: AddressAlignment: 4
# CHECK-NEXT: EntrySize: 16
# CHECK-NEXT: }
diff --git a/test/ELF/basic-ppc.s b/test/ELF/basic-ppc.s
index e08c7a3..aae81fe 100644
--- a/test/ELF/basic-ppc.s
+++ b/test/ELF/basic-ppc.s
@@ -178,7 +178,7 @@
// CHECK-NEXT: Offset: 0x2038
// CHECK-NEXT: Size: 32
// CHECK-NEXT: Link: 9
-// CHECK-NEXT: Info: 1
+// CHECK-NEXT: Info: 2
// CHECK-NEXT: AddressAlignment: 4
// CHECK-NEXT: EntrySize: 16
// CHECK-NEXT: SectionData (
diff --git a/test/ELF/linkerscript/common-assign.s b/test/ELF/linkerscript/common-assign.s
new file mode 100644
index 0000000..4244ae3
--- /dev/null
+++ b/test/ELF/linkerscript/common-assign.s
@@ -0,0 +1,48 @@
+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+# RUN: echo "SECTIONS { . = SIZEOF_HEADERS; pfoo = foo; pbar = bar; }" > %t.script
+# RUN: ld.lld -o %t1 --script %t.script %t
+# RUN: llvm-readobj -symbols %t1 | FileCheck %s
+
+# CHECK: Symbol {
+# CHECK: Name: bar
+# CHECK-NEXT: Value: 0x134
+# CHECK-NEXT: Size: 4
+# CHECK-NEXT: Binding: Global
+# CHECK-NEXT: Type: Object
+# CHECK-NEXT: Other: 0
+# CHECK-NEXT: Section: .bss
+# CHECK-NEXT: }
+# CHECK-NEXT: Symbol {
+# CHECK-NEXT: Name: foo
+# CHECK-NEXT: Value: 0x138
+# CHECK-NEXT: Size: 4
+# CHECK-NEXT: Binding: Global
+# CHECK-NEXT: Type: Object
+# CHECK-NEXT: Other: 0
+# CHECK-NEXT: Section: .bss
+# CHECK-NEXT: }
+# CHECK-NEXT: Symbol {
+# CHECK-NEXT: Name: pfoo
+# CHECK-NEXT: Value: 0x138
+# CHECK-NEXT: Size: 0
+# CHECK-NEXT: Binding: Global
+# CHECK-NEXT: Type: None
+# CHECK-NEXT: Other: 0
+# CHECK-NEXT: Section: .bss
+# CHECK-NEXT: }
+# CHECK-NEXT: Symbol {
+# CHECK-NEXT: Name: pbar
+# CHECK-NEXT: Value: 0x134
+# CHECK-NEXT: Size: 0
+# CHECK-NEXT: Binding: Global
+# CHECK-NEXT: Type: None
+# CHECK-NEXT: Other: 0
+# CHECK-NEXT: Section: .bss
+# CHECK-NEXT: }
+# CHECK-NEXT: ]
+
+.comm foo,4,4
+.comm bar,4,4
+movl $1, foo(%rip)
+movl $2, bar(%rip)
More information about the llvm-commits
mailing list