From: Fangrui Song Date: Thu, 17 Oct 2019 11:21:54 +0000 (+0000) Subject: [llvm-objcopy] --add-symbol: fix crash if SHT_SYMTAB does not exist X-Git-Tag: llvmorg-11-init~6286 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=9dce25a9fa953cec1b89009226cdc463166a7ad4;p=platform%2Fupstream%2Fllvm.git [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::readSections, if --add-symbol is specified. Reviewed By: grimar Differential Revision: https://reviews.llvm.org/D69093 llvm-svn: 375105 --- 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 new file mode 100644 index 0000000..fba904b --- /dev/null +++ b/llvm/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/llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp b/llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp index dd6a7d7..8bf7e0f 100644 --- a/llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp +++ b/llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp @@ -263,7 +263,7 @@ static Error linkToBuildIdDir(const CopyConfig &Config, StringRef ToLink, 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 @@ static Error writeOutput(const CopyConfig &Config, Object &Obj, Buffer &Out, Error executeObjcopyOnIHex(const CopyConfig &Config, MemoryBuffer &In, Buffer &Out) { IHexReader Reader(&In); - std::unique_ptr Obj = Reader.create(); + std::unique_ptr Obj = Reader.create(true); const ElfType OutputElfType = getOutputElfType(Config.OutputArch.getValueOr(MachineInfo())); if (Error E = handleArgs(Config, *Obj, Reader, OutputElfType)) @@ -756,7 +756,7 @@ Error executeObjcopyOnRawBinary(const CopyConfig &Config, MemoryBuffer &In, uint8_t NewSymbolVisibility = Config.ELF->NewSymbolVisibility.getValueOr((uint8_t)ELF::STV_DEFAULT); BinaryReader Reader(&In, NewSymbolVisibility); - std::unique_ptr Obj = Reader.create(); + std::unique_ptr Obj = Reader.create(true); // Prefer OutputArch (-O) if set, otherwise fallback to BinaryArch // (-B). @@ -770,7 +770,7 @@ Error executeObjcopyOnRawBinary(const CopyConfig &Config, MemoryBuffer &In, Error executeObjcopyOnBinary(const CopyConfig &Config, object::ELFObjectFileBase &In, Buffer &Out) { ELFReader Reader(&In, Config.ExtractPartition); - std::unique_ptr Obj = Reader.create(); + std::unique_ptr Obj = Reader.create(!Config.SymbolsToAdd.empty()); // Prefer OutputArch (-O) if set, otherwise infer it from the input. const ElfType OutputElfType = Config.OutputArch ? getOutputElfType(Config.OutputArch.getValue()) diff --git a/llvm/tools/llvm-objcopy/ELF/Object.cpp b/llvm/tools/llvm-objcopy/ELF/Object.cpp index 0220b2b..74145da 100644 --- a/llvm/tools/llvm-objcopy/ELF/Object.cpp +++ b/llvm/tools/llvm-objcopy/ELF/Object.cpp @@ -1527,7 +1527,7 @@ template void ELFBuilder::readSectionHeaders() { } } -template void ELFBuilder::readSections() { +template void ELFBuilder::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 @@ template void ELFBuilder::readSections() { 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(&Sec); + + // Prefer .strtab to .shstrtab. + if (Obj.SectionNames != &Sec) + break; + } + } + if (!StrTab) + StrTab = &Obj.addSection(); + + SymbolTableSection &SymTab = Obj.addSection(); + 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 @@ template void ELFBuilder::readSections() { " is not a string table"); } -template void ELFBuilder::build() { +template void ELFBuilder::build(bool EnsureSymtab) { readSectionHeaders(); findEhdrOffset(); @@ -1597,7 +1618,7 @@ template void ELFBuilder::build() { Obj.Entry = Ehdr.e_entry; Obj.Flags = Ehdr.e_flags; - readSections(); + readSections(EnsureSymtab); readProgramHeaders(HeadersFile); } @@ -1605,7 +1626,7 @@ Writer::~Writer() {} Reader::~Reader() {} -std::unique_ptr BinaryReader::create() const { +std::unique_ptr BinaryReader::create(bool /*EnsureSymtab*/) const { return BinaryELFBuilder(MemBuf, NewSymbolVisibility).build(); } @@ -1635,28 +1656,28 @@ Expected> IHexReader::parse() const { return std::move(Records); } -std::unique_ptr IHexReader::create() const { +std::unique_ptr IHexReader::create(bool /*EnsureSymtab*/) const { std::vector Records = unwrapOrError(parse()); return IHexELFBuilder(Records).build(); } -std::unique_ptr ELFReader::create() const { +std::unique_ptr ELFReader::create(bool EnsureSymtab) const { auto Obj = std::make_unique(); if (auto *O = dyn_cast>(Bin)) { ELFBuilder Builder(*O, *Obj, ExtractPartition); - Builder.build(); + Builder.build(EnsureSymtab); return Obj; } else if (auto *O = dyn_cast>(Bin)) { ELFBuilder Builder(*O, *Obj, ExtractPartition); - Builder.build(); + Builder.build(EnsureSymtab); return Obj; } else if (auto *O = dyn_cast>(Bin)) { ELFBuilder Builder(*O, *Obj, ExtractPartition); - Builder.build(); + Builder.build(EnsureSymtab); return Obj; } else if (auto *O = dyn_cast>(Bin)) { ELFBuilder Builder(*O, *Obj, ExtractPartition); - Builder.build(); + Builder.build(EnsureSymtab); return Obj; } error("invalid file type"); diff --git a/llvm/tools/llvm-objcopy/ELF/Object.h b/llvm/tools/llvm-objcopy/ELF/Object.h index d74b5f4..eeacb01 100644 --- a/llvm/tools/llvm-objcopy/ELF/Object.h +++ b/llvm/tools/llvm-objcopy/ELF/Object.h @@ -863,7 +863,7 @@ public: class Reader { public: virtual ~Reader(); - virtual std::unique_ptr create() const = 0; + virtual std::unique_ptr create(bool EnsureSymtab) const = 0; }; using object::Binary; @@ -926,7 +926,7 @@ private: 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 @@ public: : ElfFile(*ElfObj.getELFFile()), Obj(Obj), ExtractPartition(ExtractPartition) {} - void build(); + void build(bool EnsureSymtab); }; class BinaryReader : public Reader { @@ -946,7 +946,7 @@ class BinaryReader : public Reader { public: BinaryReader(MemoryBuffer *MB, const uint8_t NewSymbolVisibility) : MemBuf(MB), NewSymbolVisibility(NewSymbolVisibility) {} - std::unique_ptr create() const override; + std::unique_ptr create(bool EnsureSymtab) const override; }; class IHexReader : public Reader { @@ -968,7 +968,7 @@ class IHexReader : public Reader { public: IHexReader(MemoryBuffer *MB) : MemBuf(MB) {} - std::unique_ptr create() const override; + std::unique_ptr create(bool EnsureSymtab) const override; }; class ELFReader : public Reader { @@ -976,7 +976,7 @@ class ELFReader : public Reader { Optional ExtractPartition; public: - std::unique_ptr create() const override; + std::unique_ptr create(bool EnsureSymtab) const override; explicit ELFReader(Binary *B, Optional ExtractPartition) : Bin(B), ExtractPartition(ExtractPartition) {} };