[llvm-objcopy] Add --localize-hidden option

[jakehehrlich]

Summary

This change adds support in llvm-objcopy for GNU objcopy's --localize-hidden option. This option changes every hidden or internal symbol into a local symbol.

[phosek]

tools/llvm-objcopy/llvm-objcopy.cpp

197

nit: This block should be probably above the RemovePred since RemovePred is related to lines 199-304.

[jhenderson]

test/tools/llvm-objcopy/localize-hidden.test

36

Why do you have so many different symbols here?

Also, you should have symbols with hidden visibility with both local and weak bindings, to show that a) local symbols remain unchanged, and b) weak symbols are changed.

Finally, a symbol with STV_PROTECTED visibility should also be included to show that it is not localized.

tools/llvm-objcopy/Object.cpp

184–188

I'd add blank lines either side of this block.

[jakehehrlich]

1. Moved RemovePred decl as recommended
2. Added spaces around a bit of code
3. Made some big changes to the test. I added weak and local hidden symbols. I changed their names because I couldn't keep track of anything. I changed a symbol to be protected. I believe it should meet the requirements originally desired.

[jhenderson]

test/tools/llvm-objcopy/localize-hidden.test

47

Could we have a separate symbol for hidden Globals, please. Since that's going to be a relatively common case, I think it deserves explicit testing.

tools/llvm-objcopy/Object.cpp

191

indexs -> indexes

[jakehehrlich]

1. Added hidden global symbol
2. Fixed "indexes" typo

[jhenderson]

LGTM.

test/tools/llvm-objcopy/localize-hidden.test

+# RUN: yaml2obj %s > %t
+# RUN: llvm-objcopy -localize-hidden %t %t2
+# RUN: llvm-readobj -relocations -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:            .data
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    Address:         0x2000
+    AddressAlign:    0x0000000000000010
+    Content:         "0000000000000000"
+  - Name:            .rel.text
+    Type:            SHT_REL
+    Info:            .text
+    Relocations:
+      - Offset: 0x1000
+        Symbol: undefGlobal
+        Type:   R_X86_64_PC32
+Symbols:
+  Local:
+    - Name:     hiddenLocal
+      Type:     STT_FUNC
+      Section:  .text
+      Value:    0x1008
+      Size:     8
+      Visibility: STV_HIDDEN
+  Weak:
+    - Name:     hiddenWeak
+      Type:     STT_FUNC
+      Section:  .text
+      Value:    0x1010
+      Size:     8
+      Visibility: STV_HIDDEN
+  Global:
+    - Name:     defaultGlobal
+      Type:     STT_FUNC
+      Size:     8
+      Section:  .text
+      Value:    0x1000
+    - Name:     undefGlobal
+      Type:     STT_FUNC
+      Size:     8
+    - Name:     internalGlobal
+      Type:     STT_OBJECT
+      Section:  .data
+      Value:    0x2002
+      Size:     2
+      Visibility: STV_INTERNAL
+    - Name:     protectedGlobal
+      Type:     STT_OBJECT
+      Section:  .data
+      Value:    0x2000
+      Size:     4
+      Visibility: STV_PROTECTED
+
+#CHECK:     Relocations [
+#CHECK-NEXT:  Section (3) .rel.text {
+#CHECK-NEXT:    0x1000 R_X86_64_PC32 undefGlobal 0x0
+#CHECK-NEXT:  }
+#CHECK-NEXT:]
+
+
+#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: hiddenLocal
+#CHECK-NEXT:    Value: 0x1008
+#CHECK-NEXT:    Size: 8
+#CHECK-NEXT:    Binding: Local
+#CHECK-NEXT:    Type: Function
+#CHECK-NEXT:    Other [
+#CHECK-NEXT:      STV_HIDDEN
+#CHECK-NEXT:    ]
+#CHECK-NEXT:    Section: .text
+#CHECK-NEXT:  }
+#CHECK-NEXT:  Symbol {
+#CHECK-NEXT:    Name: internalGlobal
+#CHECK-NEXT:    Value: 0x2002
+#CHECK-NEXT:    Size: 2
+#CHECK-NEXT:    Binding: Local
+#CHECK-NEXT:    Type: Object
+#CHECK-NEXT:    Other [
+#CHECK-NEXT:      STV_INTERNAL
+#CHECK-NEXT:    ]
+#CHECK-NEXT:    Section: .data
+#CHECK-NEXT:  }
+#CHECK-NEXT:  Symbol {
+#CHECK-NEXT:    Name: hiddenWeak
+#CHECK-NEXT:    Value: 0x1010
+#CHECK-NEXT:    Size: 8
+#CHECK-NEXT:    Binding: Local
+#CHECK-NEXT:    Type: Function
+#CHECK-NEXT:    Other [
+#CHECK-NEXT:      STV_HIDDEN
+#CHECK-NEXT:    ]
+#CHECK-NEXT:    Section: .text
+#CHECK-NEXT:  }
+#CHECK-NEXT:  Symbol {
+#CHECK-NEXT:    Name: defaultGlobal
+#CHECK-NEXT:    Value: 0x1000
+#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: undefGlobal
+#CHECK-NEXT:    Value: 0x0
+#CHECK-NEXT:    Size: 8
+#CHECK-NEXT:    Binding: Global
+#CHECK-NEXT:    Type: Function
+#CHECK-NEXT:    Other: 0
+#CHECK-NEXT:    Section:
+#CHECK-NEXT:  }
+#CHECK-NEXT:  Symbol {
+#CHECK-NEXT:    Name: protectedGlobal
+#CHECK-NEXT:    Value: 0x2000
+#CHECK-NEXT:    Size: 4
+#CHECK-NEXT:    Binding: Global
+#CHECK-NEXT:    Type: Object
+#CHECK-NEXT:    Other [
+#CHECK-NEXT:      STV_PROTECTED
+#CHECK-NEXT:    ]
+#CHECK-NEXT:    Section: .data
+#CHECK-NEXT:  }
+#CHECK-NEXT:]

tools/llvm-objcopy/Object.h

public:
   void setStrTab(StringTableSection *StrTab) { SymbolNames = StrTab; }
   void addSymbol(StringRef Name, uint8_t Bind, uint8_t Type,
                  SectionBase *DefinedIn, uint64_t Value, uint8_t Visibility,
                  uint16_t Shndx, uint64_t Sz);
   void addSymbolNames();
   const SectionBase *getStrTab() const { return SymbolNames; }
   const Symbol *getSymbolByIndex(uint32_t Index) const;
   void removeSectionReferences(const SectionBase *Sec) override;
+  void localize(std::function<bool(const Symbol &)> ToLocalize);
   void initialize(SectionTableRef SecTable) override;
   void finalize() override;
 
   static bool classof(const SectionBase *S) {
     return S->Type == ELF::SHT_SYMTAB;
   }
 };
 
public:
   uint32_t Machine;
   uint32_t Version;
   uint32_t Flags;
   bool WriteSectionHeaders = true;
 
   Object(const object::ELFObjectFile<ELFT> &Obj);
   virtual ~Object() = default;
 
-  const SymbolTableSection *getSymTab() const { return SymbolTable; }
+  SymbolTableSection *getSymTab() const { return SymbolTable; }
   const SectionBase *getSectionHeaderStrTab() const { return SectionNames; }
   void removeSections(std::function<bool(const SectionBase &)> ToRemove);
   void addSection(StringRef SecName, ArrayRef<uint8_t> Data);
   virtual size_t totalSize() const = 0;
   virtual void finalize() = 0;
   virtual void write(FileOutputBuffer &Out) const = 0;
 };
 

tools/llvm-objcopy/Object.cpp

void SymbolTableSection::removeSectionReferences(const SectionBase *Sec) {
   }
   auto Iter =
       std::remove_if(std::begin(Symbols), std::end(Symbols),
                      [=](const SymPtr &Sym) { return Sym->DefinedIn == Sec; });
   Size -= (std::end(Symbols) - Iter) * this->EntrySize;
   Symbols.erase(Iter, std::end(Symbols));
 }
 
+void SymbolTableSection::localize(
+    std::function<bool(const Symbol &)> ToLocalize) {
+  for (const auto &Sym : Symbols) {
+    if (ToLocalize(*Sym))
+      Sym->Binding = STB_LOCAL;
+  }
+
+  // Now that the local symbols aren't grouped at the start we have to reorder
+  // the symbols to respect this property.
+  std::stable_partition(
+      std::begin(Symbols), std::end(Symbols),
+      [](const SymPtr &Sym) { return Sym->Binding == STB_LOCAL; });
+
+  // Lastly we fix the symbol indexs.
+  uint32_t Index = 0;
+  for (auto &Sym : Symbols)
+    Sym->Index = Index++;
+}
+
 void SymbolTableSection::initialize(SectionTableRef SecTable) {
   Size = 0;
   setStrTab(SecTable.getSectionOfType<StringTableSection>(
       Link,
       "Symbol table has link index of " + Twine(Link) +
           " which is not a valid index",
       "Symbol table has link index of " + Twine(Link) +
           " which is not a string table"));

tools/llvm-objcopy/llvm-objcopy.cpp

static cl::opt<std::string>
     SplitDWO("split-dwo",
              cl::desc("Equivalent to extract-dwo on the input file to "
                       "<dwo-file>, then strip-dwo on the input file"),
              cl::value_desc("dwo-file"));
 static cl::list<std::string> AddSection(
     "add-section",
     cl::desc("Make a section named <section> with the contents of <file>."),
     cl::value_desc("section=file"));
+static cl::opt<bool> LocalizeHidden(
+    "localize-hidden",
+    cl::desc(
+        "Mark all symbols that have hidden or internal visibility as local"));
 
 using SectionPred = std::function<bool(const SectionBase &Sec)>;
 
 bool IsDWOSection(const SectionBase &Sec) { return Sec.Name.endswith(".dwo"); }
 
 template <class ELFT>
 bool OnlyKeepDWOPred(const Object<ELFT> &Obj, const SectionBase &Sec) {
   // We can't remove the section header string table.
template <class ELFT> void CopyBinary(const ELFObjectFile<ELFT> &ObjFile) {
   if (!OutputFormat.empty() && OutputFormat == "binary")
     Obj = llvm::make_unique<BinaryObject<ELFT>>(ObjFile);
   else
     Obj = llvm::make_unique<ELFObject<ELFT>>(ObjFile);
 
   if (!SplitDWO.empty())
     SplitDWOToFile<ELFT>(ObjFile, SplitDWO.getValue());
 
+  // Localize:
+
+  if (LocalizeHidden) {
+    Obj->getSymTab()->localize([](const Symbol &Sym) {
+      return Sym.Visibility == STV_HIDDEN || Sym.Visibility == STV_INTERNAL;
+    });
+  }
+
   SectionPred RemovePred = [](const SectionBase &) { return false; };
 
   // Removes:
 
   if (!ToRemove.empty()) {
     RemovePred = [&](const SectionBase &Sec) {
       return std::find(std::begin(ToRemove), std::end(ToRemove), Sec.Name) !=
              std::end(ToRemove);
     };
   }