[llvm] r332925 - Revert "[llvm-objcopy] Add --strip-unneeded option"

Paul Semel via llvm-commits llvm-commits at lists.llvm.org
Mon May 21 18:04:36 PDT 2018


Author: paulsemel
Date: Mon May 21 18:04:36 2018
New Revision: 332925

URL: http://llvm.org/viewvc/llvm-project?rev=332925&view=rev
Log:
Revert "[llvm-objcopy] Add --strip-unneeded option"

There is a use after free I didn't see. Need to investigate.

This reverts commit f7624abeb1f0d012309baf2e78cf2499fbfe5e5f.

Removed:
    llvm/trunk/test/tools/llvm-objcopy/strip-unneeded.test
Modified:
    llvm/trunk/tools/llvm-objcopy/ObjcopyOpts.td
    llvm/trunk/tools/llvm-objcopy/Object.cpp
    llvm/trunk/tools/llvm-objcopy/Object.h
    llvm/trunk/tools/llvm-objcopy/llvm-objcopy.cpp

Removed: llvm/trunk/test/tools/llvm-objcopy/strip-unneeded.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objcopy/strip-unneeded.test?rev=332924&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-objcopy/strip-unneeded.test (original)
+++ llvm/trunk/test/tools/llvm-objcopy/strip-unneeded.test (removed)
@@ -1,136 +0,0 @@
-# RUN: yaml2obj %s > %t
-# RUN: llvm-objcopy --strip-unneeded %t %t2
-# RUN: llvm-readobj -symbols %t2 | FileCheck %s
-
-!ELF
-FileHeader:
-  Class:           ELFCLASS64
-  Data:            ELFDATA2LSB
-  Type:            ET_REL
-  Machine:         EM_X86_64
-Sections:
-  - Name:            .text
-    Type:            SHT_PROGBITS
-    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
-    Address:         0x1000
-    AddressAlign:    0x0000000000000010
-    Size:            64
-  - Name:            .group
-    Type:            SHT_GROUP
-    Link:            .symtab
-    AddressAlign:    0x0000000000000004
-    Info:            barfoo
-    Members:
-      - SectionOrType:   GRP_COMDAT
-      - SectionOrType:   .text
-  - Name:            .rel.text
-    Type:            SHT_REL
-    Link:            .symtab
-    Info:            .text
-    Relocations:
-      - Offset: 0x1000
-        Symbol: foo
-        Type:   R_X86_64_PC32
-Symbols:
-  Local:
-    - Name:     foo
-      Type:     STT_FUNC
-      Section:  .text
-      Value:    0x1000
-      Size:     8
-    - Name:     bar
-      Type:     STT_FUNC
-      Size:     8
-      Section:  .text
-      Value:    0x1008
-    - Name:     barfoo
-      Type:     STT_FUNC
-      Size:     8
-      Section:  .text
-      Value:    0x1010
-    - Name:     fileSymbol
-      Type:     STT_FILE
-    - Name:     sectionSymbol
-      Type:     STT_SECTION
-  Weak:
-    - Name:     baz
-      Type:     STT_FUNC
-      Size:     8
-      Section:  .text
-      Value:    0x1018
-    - Name:     foobaz
-      Type:     STT_FUNC
-  Global:
-    - Name:     foobar
-      Type:     STT_FUNC
-    - Name:     barbaz
-      Type:     STT_FUNC
-      Size:     8
-      Section:  .text
-      Value:    0x1020
-
-#CHECK: Symbols [
-#CHECK-NEXT:  Symbol {
-#CHECK-NEXT:    Name:
-#CHECK-NEXT:    Value: 0x0
-#CHECK-NEXT:    Size: 0
-#CHECK-NEXT:    Binding: Local
-#CHECK-NEXT:    Type: None
-#CHECK-NEXT:    Other: 0
-#CHECK-NEXT:    Section: Undefined
-#CHECK-NEXT:  }
-#CHECK-NEXT:  Symbol {
-#CHECK-NEXT:    Name: foo
-#CHECK-NEXT:    Value: 0x1000
-#CHECK-NEXT:    Size: 8
-#CHECK-NEXT:    Binding: Local
-#CHECK-NEXT:    Type: Function
-#CHECK-NEXT:    Other: 0
-#CHECK-NEXT:    Section: .text
-#CHECK-NEXT:  }
-#CHECK-NEXT:  Symbol {
-#CHECK-NEXT:    Name: barfoo
-#CHECK-NEXT:    Value: 0x1010
-#CHECK-NEXT:    Size: 8
-#CHECK-NEXT:    Binding: Local
-#CHECK-NEXT:    Type: Function
-#CHECK-NEXT:    Other: 0
-#CHECK-NEXT:    Section: .text
-#CHECK-NEXT:  }
-#CHECK-NEXT:  Symbol {
-#CHECK-NEXT:    Name: fileSymbol
-#CHECK-NEXT:    Value: 0x0
-#CHECK-NEXT:    Size: 0
-#CHECK-NEXT:    Binding: Local
-#CHECK-NEXT:    Type: File
-#CHECK-NEXT:    Other: 0
-#CHECK-NEXT:    Section: Undefined
-#CHECK-NEXT:  }
-#CHECK-NEXT:  Symbol {
-#CHECK-NEXT:    Name: sectionSymbol
-#CHECK-NEXT:    Value: 0x0
-#CHECK-NEXT:    Size: 0
-#CHECK-NEXT:    Binding: Local
-#CHECK-NEXT:    Type: Section
-#CHECK-NEXT:    Other: 0
-#CHECK-NEXT:    Section: Undefined
-#CHECK-NEXT:  }
-#CHECK-NEXT:  Symbol {
-#CHECK-NEXT:    Name: barbaz
-#CHECK-NEXT:    Value: 0x1020
-#CHECK-NEXT:    Size: 8
-#CHECK-NEXT:    Binding: Global
-#CHECK-NEXT:    Type: Function
-#CHECK-NEXT:    Other: 0
-#CHECK-NEXT:    Section: .text
-#CHECK-NEXT:  }
-#CHECK-NEXT:  Symbol {
-#CHECK-NEXT:    Name: baz
-#CHECK-NEXT:    Value: 0x1018
-#CHECK-NEXT:    Size: 8
-#CHECK-NEXT:    Binding: Weak
-#CHECK-NEXT:    Type: Function
-#CHECK-NEXT:    Other: 0
-#CHECK-NEXT:    Section: .text
-#CHECK-NEXT:  }
-#CHECK-NEXT:]

Modified: llvm/trunk/tools/llvm-objcopy/ObjcopyOpts.td
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objcopy/ObjcopyOpts.td?rev=332925&r1=332924&r2=332925&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-objcopy/ObjcopyOpts.td (original)
+++ llvm/trunk/tools/llvm-objcopy/ObjcopyOpts.td Mon May 21 18:04:36 2018
@@ -90,5 +90,3 @@ def K : JoinedOrSeparate<["-"], "K">,
         Alias<keep_symbol>;
 def only_keep_debug : Flag<["-", "--"], "only-keep-debug">,
                           HelpText<"Currently ignored. Only for compaitability with GNU objcopy.">;
-def strip_unneeded : Flag<["-", "--"], "strip-unneeded">,
-                      HelpText<"Remove all symbols not needed by relocations">;

Modified: llvm/trunk/tools/llvm-objcopy/Object.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objcopy/Object.cpp?rev=332925&r1=332924&r2=332925&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-objcopy/Object.cpp (original)
+++ llvm/trunk/tools/llvm-objcopy/Object.cpp Mon May 21 18:04:36 2018
@@ -185,7 +185,6 @@ void SymbolTableSection::addSymbol(Strin
   Sym.Visibility = Visibility;
   Sym.Size = Sz;
   Sym.Index = Symbols.size();
-  Sym.ReferenceCount = 0;
   Symbols.emplace_back(llvm::make_unique<Symbol>(Sym));
   Size += this->EntrySize;
 }
@@ -256,11 +255,6 @@ const Symbol *SymbolTableSection::getSym
   return Symbols[Index].get();
 }
 
-Symbol *SymbolTableSection::getSymbolByIndex(uint32_t Index) {
-  return const_cast<Symbol *>(
-      static_cast<const SymbolTableSection *>(this)->getSymbolByIndex(Index));
-}
-
 template <class ELFT>
 void ELFSectionWriter<ELFT>::visit(const SymbolTableSection &Sec) {
   uint8_t *Buf = Out.getBufferStart();
@@ -350,12 +344,6 @@ void RelocationSection::accept(SectionVi
   Visitor.visit(*this);
 }
 
-RelocationSection::~RelocationSection() {
-  for (auto &Rel : Relocations) {
-    --Rel.RelocSymbol->ReferenceCount;
-  }
-}
-
 void RelocationSection::removeSymbols(
     function_ref<bool(const Symbol &)> ToRemove) {
   for (const Relocation &Reloc : Relocations)
@@ -659,9 +647,7 @@ void initRelocations(RelocationSection *
     ToAdd.Offset = Rel.r_offset;
     getAddend(ToAdd.Addend, Rel);
     ToAdd.Type = Rel.getType(false);
-    Symbol *Sym = SymbolTable->getSymbolByIndex(Rel.getSymbol(false));
-    ++Sym->ReferenceCount;
-    ToAdd.RelocSymbol = Sym;
+    ToAdd.RelocSymbol = SymbolTable->getSymbolByIndex(Rel.getSymbol(false));
     Relocs->addRelocation(ToAdd);
   }
 }

Modified: llvm/trunk/tools/llvm-objcopy/Object.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objcopy/Object.h?rev=332925&r1=332924&r2=332925&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-objcopy/Object.h (original)
+++ llvm/trunk/tools/llvm-objcopy/Object.h Mon May 21 18:04:36 2018
@@ -344,7 +344,6 @@ struct Symbol {
   uint8_t Type;
   uint64_t Value;
   uint8_t Visibility;
-  uint32_t ReferenceCount;
 
   uint16_t getShndx() const;
 };
@@ -368,7 +367,6 @@ public:
   void addSymbolNames();
   const SectionBase *getStrTab() const { return SymbolNames; }
   const Symbol *getSymbolByIndex(uint32_t Index) const;
-  Symbol *getSymbolByIndex(uint32_t Index);
   void updateSymbols(function_ref<void(Symbol &)> Callable);
 
   void removeSectionReferences(const SectionBase *Sec) override;
@@ -383,7 +381,7 @@ public:
 };
 
 struct Relocation {
-  Symbol *RelocSymbol = nullptr;
+  const Symbol *RelocSymbol = nullptr;
   uint64_t Offset;
   uint64_t Addend;
   uint32_t Type;
@@ -434,7 +432,6 @@ class RelocationSection
   std::vector<Relocation> Relocations;
 
 public:
-  ~RelocationSection();
   void addRelocation(Relocation Rel) { Relocations.push_back(Rel); }
   void accept(SectionVisitor &Visitor) const override;
   void removeSymbols(function_ref<bool(const Symbol &)> ToRemove) override;
@@ -452,7 +449,7 @@ public:
 class GroupSection : public SectionBase {
   MAKE_SEC_WRITER_FRIEND
   const SymbolTableSection *SymTab = nullptr;
-  Symbol *Sym = nullptr;
+  const Symbol *Sym = nullptr;
   ELF::Elf32_Word FlagWord;
   SmallVector<SectionBase *, 3> GroupMembers;
 
@@ -462,16 +459,9 @@ public:
   ArrayRef<uint8_t> Contents;
 
   explicit GroupSection(ArrayRef<uint8_t> Data) : Contents(Data) {}
-  ~GroupSection() {
-    if (Sym)
-      --Sym->ReferenceCount;
-  }
 
   void setSymTab(const SymbolTableSection *SymTabSec) { SymTab = SymTabSec; }
-  void setSymbol(Symbol *S) {
-    Sym = S;
-    ++Sym->ReferenceCount;
-  }
+  void setSymbol(const Symbol *S) { Sym = S; }
   void setFlagWord(ELF::Elf32_Word W) { FlagWord = W; }
   void addMember(SectionBase *Sec) { GroupMembers.push_back(Sec); }
 

Modified: llvm/trunk/tools/llvm-objcopy/llvm-objcopy.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objcopy/llvm-objcopy.cpp?rev=332925&r1=332924&r2=332925&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-objcopy/llvm-objcopy.cpp (original)
+++ llvm/trunk/tools/llvm-objcopy/llvm-objcopy.cpp Mon May 21 18:04:36 2018
@@ -156,7 +156,6 @@ struct CopyConfig {
   bool StripSections = false;
   bool StripNonAlloc = false;
   bool StripDWO = false;
-  bool StripUnneeded = false;
   bool ExtractDWO = false;
   bool LocalizeHidden = false;
   bool Weaken = false;
@@ -390,13 +389,6 @@ void HandleArgs(const CopyConfig &Config
         return true;
       }
 
-      // TODO: We might handle the 'null symbol' in a different way
-      // by probably handling it the same way as we handle 'null section' ?
-      if (Config.StripUnneeded && !Sym.ReferenceCount && Sym.Index != 0 &&
-          (Sym.Binding == STB_LOCAL || Sym.getShndx() == SHN_UNDEF) &&
-          Sym.Type != STT_FILE && Sym.Type != STT_SECTION)
-        return true;
-
       return false;
     });
   }
@@ -487,7 +479,6 @@ CopyConfig ParseObjcopyOptions(ArrayRef<
   Config.StripDWO = InputArgs.hasArg(OBJCOPY_strip_dwo);
   Config.StripSections = InputArgs.hasArg(OBJCOPY_strip_sections);
   Config.StripNonAlloc = InputArgs.hasArg(OBJCOPY_strip_non_alloc);
-  Config.StripUnneeded = InputArgs.hasArg(OBJCOPY_strip_unneeded);
   Config.ExtractDWO = InputArgs.hasArg(OBJCOPY_extract_dwo);
   Config.LocalizeHidden = InputArgs.hasArg(OBJCOPY_localize_hidden);
   Config.Weaken = InputArgs.hasArg(OBJCOPY_weaken);




More information about the llvm-commits mailing list