[llvm] 27cb352 - [llvm-objcopy] --add-symbol: address post-commit reviews of D69093
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 31 09:12:25 PDT 2019
Author: Fangrui Song
Date: 2019-10-31T09:12:06-07:00
New Revision: 27cb352fd27668519f25ab8d5717173fc3ff2235
URL: https://github.com/llvm/llvm-project/commit/27cb352fd27668519f25ab8d5717173fc3ff2235
DIFF: https://github.com/llvm/llvm-project/commit/27cb352fd27668519f25ab8d5717173fc3ff2235.diff
LOG: [llvm-objcopy] --add-symbol: address post-commit reviews of D69093
* Improve comments.
* Reorder the assignment to Obj.SectionNames before the symbol table
creation code. Add a test.
Reviewed By: grimar
Differential Revision: https://reviews.llvm.org/D69526
Added:
Modified:
llvm/test/tools/llvm-objcopy/ELF/add-symbol-no-symtab.test
llvm/tools/llvm-objcopy/ELF/Object.cpp
Removed:
################################################################################
diff --git a/llvm/test/tools/llvm-objcopy/ELF/add-symbol-no-symtab.test b/llvm/test/tools/llvm-objcopy/ELF/add-symbol-no-symtab.test
index fba904b91a65..b482634b41e1 100644
--- a/llvm/test/tools/llvm-objcopy/ELF/add-symbol-no-symtab.test
+++ b/llvm/test/tools/llvm-objcopy/ELF/add-symbol-no-symtab.test
@@ -4,10 +4,10 @@
## If a non-SHF_ALLOC SHT_STRTAB exists but SHT_SYMTAB does not, create .symtab
## and set its sh_link to the SHT_STRTAB.
-# RUN: llvm-objcopy --remove-section=.symtab %t %t1
-# RUN: llvm-objcopy --add-symbol='abs1=1' %t1 %t2
-# RUN: llvm-readobj -S %t2 | FileCheck --check-prefix=SEC %s
-# RUN: llvm-readelf -s %t2 | FileCheck %s
+# RUN: llvm-objcopy --remove-section=.symtab %t %t.no.symtab
+# RUN: llvm-objcopy --add-symbol='abs1=1' %t.no.symtab %t.add.both
+# RUN: llvm-readobj -S %t.add.both | FileCheck --check-prefix=SEC %s
+# RUN: llvm-readelf -s %t.add.both | FileCheck %s
# SEC: Index: 1
# SEC-NEXT: Name: .strtab
@@ -25,16 +25,15 @@
# CHECK-NEXT: 1: 0000000000000001 0 NOTYPE GLOBAL DEFAULT ABS abs1
## Don't create .symtab if --add-symbol is not specified.
-# RUN: llvm-objcopy --remove-section=.symtab --remove-section=.strtab %t %t1
-# RUN: llvm-objcopy %t1 %t2
-# RUN: llvm-readobj -S %t2 | FileCheck --implicit-check-not=.symtab /dev/null
+# RUN: llvm-objcopy --remove-section=.symtab --remove-section=.strtab %t %t.no
+# RUN: llvm-objcopy %t.no %t.nop
+# RUN: llvm-readobj -S %t.nop | FileCheck --implicit-check-not=.symtab --implicit-check-not=.strtab /dev/null
-## Check that we create both .strtab and .symtab.
-## We reuse the existing .shstrtab (section names) for symbol names.
+## Reuse the existing .shstrtab (section names) for symbol names.
## This may look strange but it works.
-# RUN: llvm-objcopy --add-symbol='abs1=1' %t1 %t2
-# RUN: llvm-readobj -S %t2 | FileCheck --check-prefix=SEC2 %s
-# RUN: llvm-readelf -s %t2 | FileCheck %s
+# RUN: llvm-objcopy --add-symbol='abs1=1' %t.no %t.add.symtab
+# RUN: llvm-readobj -S %t.add.symtab | FileCheck --check-prefix=SEC2 %s --implicit-check-not=.strtab
+# RUN: llvm-readelf -s %t.add.symtab | FileCheck %s
# SEC2: Index: 1
# SEC2-NEXT: Name: .shstrtab
@@ -51,14 +50,41 @@ FileHeader:
Data: ELFDATA2LSB
Type: ET_REL
Machine: EM_X86_64
-Symbols:
+Sections:
+ - Name: .strtab
+ Type: SHT_STRTAB
+ - Name: .shstrtab
+ Type: SHT_STRTAB
+Symbols: []
+...
+
+## Check that we prefer the string table that is not the section header string table.
+# RUN: yaml2obj --docnum=2 %s -o %t2
+# RUN: llvm-objcopy --remove-section=.symtab --remove-section=.strtab %t2 %t2.no
+# RUN: llvm-objcopy --add-symbol='abs1=1' %t2.no %t2.add.symtab
+# RUN: llvm-readobj -S %t2.add.symtab | FileCheck --check-prefix=SEC2 %s
+# RUN: llvm-readelf -s %t2.add.symtab | FileCheck %s
+
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2LSB
+ Type: ET_REL
+ Machine: EM_X86_64
+Sections:
+ # .shstrtab is reordered before .strtab
+ - Name: .shstrtab
+ Type: SHT_STRTAB
+ - Name: .strtab
+ Type: SHT_STRTAB
+Symbols: []
...
## Check the created .symtab does not link to .dynstr (SHF_ALLOC).
-# RUN: yaml2obj --docnum=2 %s -o %t
-# RUN: llvm-objcopy --remove-section=.symtab --remove-section=.strtab %t %t1
-# RUN: llvm-objcopy --add-symbol='abs1=1' %t1 %t2
-# RUN: llvm-readobj -S %t2 | FileCheck --check-prefix=SEC3 %s
+# RUN: yaml2obj --docnum=3 %s -o %t3
+# RUN: llvm-objcopy --remove-section=.symtab --remove-section=.strtab %t3 %t3.no
+# RUN: llvm-objcopy --add-symbol='abs1=1' %t3.no %t3.not.dynstr
+# RUN: llvm-readobj -S %t3.not.dynstr | FileCheck --check-prefix=SEC3 %s
# SEC3: Index: 1
# SEC3-NEXT: Name: .shstrtab
@@ -77,5 +103,5 @@ FileHeader:
DynamicSymbols:
- Name: dummy
Binding: STB_GLOBAL
-Symbols:
+Symbols: []
...
diff --git a/llvm/tools/llvm-objcopy/ELF/Object.cpp b/llvm/tools/llvm-objcopy/ELF/Object.cpp
index 0fb42e0305f8..bbf7ff3ae327 100644
--- a/llvm/tools/llvm-objcopy/ELF/Object.cpp
+++ b/llvm/tools/llvm-objcopy/ELF/Object.cpp
@@ -1539,6 +1539,21 @@ template <class ELFT> void ELFBuilder<ELFT>::readSectionHeaders() {
}
template <class ELFT> void ELFBuilder<ELFT>::readSections(bool EnsureSymtab) {
+ uint32_t ShstrIndex = ElfFile.getHeader()->e_shstrndx;
+ if (ShstrIndex == SHN_XINDEX)
+ ShstrIndex = unwrapOrError(ElfFile.getSection(0))->sh_link;
+
+ if (ShstrIndex == SHN_UNDEF)
+ Obj.HadShdrs = false;
+ else
+ Obj.SectionNames =
+ Obj.sections().template getSectionOfType<StringTableSection>(
+ ShstrIndex,
+ "e_shstrndx field value " + Twine(ShstrIndex) + " in elf header " +
+ " is invalid",
+ "e_shstrndx field value " + Twine(ShstrIndex) + " in elf header " +
+ " does not reference a string table");
+
// If a section index table exists we'll need to initialize it before we
// initialize the symbol table because the symbol table might need to
// reference it.
@@ -1552,13 +1567,14 @@ template <class ELFT> void ELFBuilder<ELFT>::readSections(bool EnsureSymtab) {
Obj.SymbolTable->initialize(Obj.sections());
initSymbolTable(Obj.SymbolTable);
} else if (EnsureSymtab) {
- // Reuse the existing SHT_STRTAB section if exists.
+ // Reuse an existing SHT_STRTAB section if it exists.
StringTableSection *StrTab = nullptr;
for (auto &Sec : Obj.sections()) {
if (Sec.Type == ELF::SHT_STRTAB && !(Sec.Flags & SHF_ALLOC)) {
StrTab = static_cast<StringTableSection *>(&Sec);
- // Prefer .strtab to .shstrtab.
+ // Prefer a string table that is not the section header string table, if
+ // such a table exists.
if (Obj.SectionNames != &Sec)
break;
}
@@ -1593,21 +1609,6 @@ template <class ELFT> void ELFBuilder<ELFT>::readSections(bool EnsureSymtab) {
initGroupSection(GroupSec);
}
}
-
- uint32_t ShstrIndex = ElfFile.getHeader()->e_shstrndx;
- if (ShstrIndex == SHN_XINDEX)
- ShstrIndex = unwrapOrError(ElfFile.getSection(0))->sh_link;
-
- if (ShstrIndex == SHN_UNDEF)
- Obj.HadShdrs = false;
- else
- Obj.SectionNames =
- Obj.sections().template getSectionOfType<StringTableSection>(
- ShstrIndex,
- "e_shstrndx field value " + Twine(ShstrIndex) + " in elf header " +
- " is invalid",
- "e_shstrndx field value " + Twine(ShstrIndex) + " in elf header " +
- " is not a string table");
}
template <class ELFT> void ELFBuilder<ELFT>::build(bool EnsureSymtab) {
More information about the llvm-commits
mailing list