[llvm-objcopy] --add-symbol: fix crash if SHT_SYMTAB does not exist
Exposed by D69041. If SHT_SYMTAB does not exist, ELFObjcopy.cpp:handleArgs will crash due
to a null pointer dereference.
for (const NewSymbolInfo &SI : Config.ELF->SymbolsToAdd) {
...
Obj.SymbolTable->addSymbol(
Fix this by creating .symtab and .strtab on demand in ELFBuilder<ELFT>::readSections,
if --add-symbol is specified.
Reviewed By: grimar
Differential Revision: https://reviews.llvm.org/D69093
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@375105 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/test/tools/llvm-objcopy/ELF/add-symbol-no-symtab.test b/test/tools/llvm-objcopy/ELF/add-symbol-no-symtab.test
new file mode 100644
index 0000000..fba904b
--- /dev/null
+++ b/test/tools/llvm-objcopy/ELF/add-symbol-no-symtab.test
@@ -0,0 +1,81 @@
+## Test --add-symbol creates .symtab if it does not exist.
+
+# RUN: yaml2obj --docnum=1 %s -o %t
+
+## 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
+
+# SEC: Index: 1
+# SEC-NEXT: Name: .strtab
+# SEC-NEXT: Type: SHT_STRTAB
+# SEC: Index: 2
+# SEC-NEXT: Name: .shstrtab
+# SEC-NEXT: Type: SHT_STRTAB
+# SEC: Index: 3
+# SEC-NEXT: Name: .symtab
+# SEC-NEXT: Type: SHT_SYMTAB
+# SEC-NOT: }
+# SEC: Link: 1
+
+# CHECK: 0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND
+# 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
+
+## Check that we create both .strtab and .symtab.
+## We 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
+
+# SEC2: Index: 1
+# SEC2-NEXT: Name: .shstrtab
+# SEC2-NEXT: Type: SHT_STRTAB
+# SEC2: Index: 2
+# SEC2-NEXT: Name: .symtab
+# SEC2-NEXT: Type: SHT_SYMTAB
+# SEC2-NOT: }
+# SEC2: Link: 1
+
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2LSB
+ Type: ET_REL
+ Machine: EM_X86_64
+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
+
+# SEC3: Index: 1
+# SEC3-NEXT: Name: .shstrtab
+# SEC3-NEXT: Type: SHT_STRTAB
+# SEC3: Name: .symtab
+# SEC3-NEXT: Type: SHT_SYMTAB
+# SEC3-NOT: }
+# SEC3: Link: 1
+
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2LSB
+ Type: ET_DYN
+ Machine: EM_X86_64
+DynamicSymbols:
+ - Name: dummy
+ Binding: STB_GLOBAL
+Symbols:
+...
diff --git a/tools/llvm-objcopy/ELF/ELFObjcopy.cpp b/tools/llvm-objcopy/ELF/ELFObjcopy.cpp
index dd6a7d7..8bf7e0f 100644
--- a/tools/llvm-objcopy/ELF/ELFObjcopy.cpp
+++ b/tools/llvm-objcopy/ELF/ELFObjcopy.cpp
@@ -263,7 +263,7 @@
static Error splitDWOToFile(const CopyConfig &Config, const Reader &Reader,
StringRef File, ElfType OutputElfType) {
- auto DWOFile = Reader.create();
+ auto DWOFile = Reader.create(false);
auto OnlyKeepDWOPred = [&DWOFile](const SectionBase &Sec) {
return onlyKeepDWOPred(*DWOFile, Sec);
};
@@ -743,7 +743,7 @@
Error executeObjcopyOnIHex(const CopyConfig &Config, MemoryBuffer &In,
Buffer &Out) {
IHexReader Reader(&In);
- std::unique_ptr<Object> Obj = Reader.create();
+ std::unique_ptr<Object> Obj = Reader.create(true);
const ElfType OutputElfType =
getOutputElfType(Config.OutputArch.getValueOr(MachineInfo()));
if (Error E = handleArgs(Config, *Obj, Reader, OutputElfType))
@@ -756,7 +756,7 @@
uint8_t NewSymbolVisibility =
Config.ELF->NewSymbolVisibility.getValueOr((uint8_t)ELF::STV_DEFAULT);
BinaryReader Reader(&In, NewSymbolVisibility);
- std::unique_ptr<Object> Obj = Reader.create();
+ std::unique_ptr<Object> Obj = Reader.create(true);
// Prefer OutputArch (-O<format>) if set, otherwise fallback to BinaryArch
// (-B<arch>).
@@ -770,7 +770,7 @@
Error executeObjcopyOnBinary(const CopyConfig &Config,
object::ELFObjectFileBase &In, Buffer &Out) {
ELFReader Reader(&In, Config.ExtractPartition);
- std::unique_ptr<Object> Obj = Reader.create();
+ std::unique_ptr<Object> Obj = Reader.create(!Config.SymbolsToAdd.empty());
// Prefer OutputArch (-O<format>) if set, otherwise infer it from the input.
const ElfType OutputElfType =
Config.OutputArch ? getOutputElfType(Config.OutputArch.getValue())
diff --git a/tools/llvm-objcopy/ELF/Object.cpp b/tools/llvm-objcopy/ELF/Object.cpp
index 0220b2b..74145da 100644
--- a/tools/llvm-objcopy/ELF/Object.cpp
+++ b/tools/llvm-objcopy/ELF/Object.cpp
@@ -1527,7 +1527,7 @@
}
}
-template <class ELFT> void ELFBuilder<ELFT>::readSections() {
+template <class ELFT> void ELFBuilder<ELFT>::readSections(bool EnsureSymtab) {
// 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.
@@ -1540,6 +1540,27 @@
if (Obj.SymbolTable) {
Obj.SymbolTable->initialize(Obj.sections());
initSymbolTable(Obj.SymbolTable);
+ } else if (EnsureSymtab) {
+ // Reuse the existing SHT_STRTAB section if 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.
+ if (Obj.SectionNames != &Sec)
+ break;
+ }
+ }
+ if (!StrTab)
+ StrTab = &Obj.addSection<StringTableSection>();
+
+ SymbolTableSection &SymTab = Obj.addSection<SymbolTableSection>();
+ SymTab.Name = ".symtab";
+ SymTab.Link = StrTab->Index;
+ SymTab.initialize(Obj.sections());
+ SymTab.addSymbol("", 0, 0, nullptr, 0, 0, 0, 0);
+ Obj.SymbolTable = &SymTab;
}
// Now that all sections and symbols have been added we can add
@@ -1578,7 +1599,7 @@
" is not a string table");
}
-template <class ELFT> void ELFBuilder<ELFT>::build() {
+template <class ELFT> void ELFBuilder<ELFT>::build(bool EnsureSymtab) {
readSectionHeaders();
findEhdrOffset();
@@ -1597,7 +1618,7 @@
Obj.Entry = Ehdr.e_entry;
Obj.Flags = Ehdr.e_flags;
- readSections();
+ readSections(EnsureSymtab);
readProgramHeaders(HeadersFile);
}
@@ -1605,7 +1626,7 @@
Reader::~Reader() {}
-std::unique_ptr<Object> BinaryReader::create() const {
+std::unique_ptr<Object> BinaryReader::create(bool /*EnsureSymtab*/) const {
return BinaryELFBuilder(MemBuf, NewSymbolVisibility).build();
}
@@ -1635,28 +1656,28 @@
return std::move(Records);
}
-std::unique_ptr<Object> IHexReader::create() const {
+std::unique_ptr<Object> IHexReader::create(bool /*EnsureSymtab*/) const {
std::vector<IHexRecord> Records = unwrapOrError(parse());
return IHexELFBuilder(Records).build();
}
-std::unique_ptr<Object> ELFReader::create() const {
+std::unique_ptr<Object> ELFReader::create(bool EnsureSymtab) const {
auto Obj = std::make_unique<Object>();
if (auto *O = dyn_cast<ELFObjectFile<ELF32LE>>(Bin)) {
ELFBuilder<ELF32LE> Builder(*O, *Obj, ExtractPartition);
- Builder.build();
+ Builder.build(EnsureSymtab);
return Obj;
} else if (auto *O = dyn_cast<ELFObjectFile<ELF64LE>>(Bin)) {
ELFBuilder<ELF64LE> Builder(*O, *Obj, ExtractPartition);
- Builder.build();
+ Builder.build(EnsureSymtab);
return Obj;
} else if (auto *O = dyn_cast<ELFObjectFile<ELF32BE>>(Bin)) {
ELFBuilder<ELF32BE> Builder(*O, *Obj, ExtractPartition);
- Builder.build();
+ Builder.build(EnsureSymtab);
return Obj;
} else if (auto *O = dyn_cast<ELFObjectFile<ELF64BE>>(Bin)) {
ELFBuilder<ELF64BE> Builder(*O, *Obj, ExtractPartition);
- Builder.build();
+ Builder.build(EnsureSymtab);
return Obj;
}
error("invalid file type");
diff --git a/tools/llvm-objcopy/ELF/Object.h b/tools/llvm-objcopy/ELF/Object.h
index d74b5f4..eeacb01 100644
--- a/tools/llvm-objcopy/ELF/Object.h
+++ b/tools/llvm-objcopy/ELF/Object.h
@@ -863,7 +863,7 @@
class Reader {
public:
virtual ~Reader();
- virtual std::unique_ptr<Object> create() const = 0;
+ virtual std::unique_ptr<Object> create(bool EnsureSymtab) const = 0;
};
using object::Binary;
@@ -926,7 +926,7 @@
void initGroupSection(GroupSection *GroupSec);
void initSymbolTable(SymbolTableSection *SymTab);
void readSectionHeaders();
- void readSections();
+ void readSections(bool EnsureSymtab);
void findEhdrOffset();
SectionBase &makeSection(const Elf_Shdr &Shdr);
@@ -936,7 +936,7 @@
: ElfFile(*ElfObj.getELFFile()), Obj(Obj),
ExtractPartition(ExtractPartition) {}
- void build();
+ void build(bool EnsureSymtab);
};
class BinaryReader : public Reader {
@@ -946,7 +946,7 @@
public:
BinaryReader(MemoryBuffer *MB, const uint8_t NewSymbolVisibility)
: MemBuf(MB), NewSymbolVisibility(NewSymbolVisibility) {}
- std::unique_ptr<Object> create() const override;
+ std::unique_ptr<Object> create(bool EnsureSymtab) const override;
};
class IHexReader : public Reader {
@@ -968,7 +968,7 @@
public:
IHexReader(MemoryBuffer *MB) : MemBuf(MB) {}
- std::unique_ptr<Object> create() const override;
+ std::unique_ptr<Object> create(bool EnsureSymtab) const override;
};
class ELFReader : public Reader {
@@ -976,7 +976,7 @@
Optional<StringRef> ExtractPartition;
public:
- std::unique_ptr<Object> create() const override;
+ std::unique_ptr<Object> create(bool EnsureSymtab) const override;
explicit ELFReader(Binary *B, Optional<StringRef> ExtractPartition)
: Bin(B), ExtractPartition(ExtractPartition) {}
};
