[llvm-objcopy] Add --prefix-symbols option

[paulsemel]

tools/llvm-objcopy/llvm-objcopy.cpp

215

This might not be the correct way to retain the new symbols names (and surely not the correct place to have it anyway), but I couldn't find a smarter way to do it..

[jhenderson]

test/tools/llvm-objcopy/prefix-symbols.test

23–25

This is a little weird. Section symbols don't usually have a name, since they effectively have the section's name. Does yaml2obj allow you to create a section symbol for a specific section (and thus end up with st_name == 0)?

tools/llvm-objcopy/ObjcopyOpts.td

107

to start -> to the start

tools/llvm-objcopy/llvm-objcopy.cpp

215

The lazy answer would be to just give ownership of symbol names to the symbol class. I don't know if that's the right thing to do, but it feels a bit better than this to me.

370

There's not much point in newing a std::string into existence. Just create a vector of strings.

[paulsemel]

test/tools/llvm-objcopy/prefix-symbols.test

23–25

Does it really matter ? We want to show that the name is unchanged, and indeed yaml2obj is puting a name for this section symbols, so..

tools/llvm-objcopy/llvm-objcopy.cpp

215

Well, not really, as a symbol shouldn't have anything else than a StringRef (as it is to be immutable). The fact is that we need to have a "new" string in memory, to be referenced by this symbol..

370

No, with vector of strings, the address can change (using vectors, we can't ensure the address of elements will remain the same)

[paulsemel]

Changed option name (to start -> to the start)

[jhenderson]

tools/llvm-objcopy/llvm-objcopy.cpp

215

But, symbol names are not immutable (as demonstrated by the fact that we have this option). I don't see what StringRef gives us here, apart from a bit of efficiency for cases when we don't need to change the symbol name.

370

Fair (although pre-allocating the vector capacity would solve this issue too). But this becomes moot if we give ownership of names to the symbols...

[paulsemel]

Give ownership of Symbols names to symbol itself.

[jakehehrlich]

Nice. We need the std::string change for names for several other changes. LGTM.

[jhenderson]

test/tools/llvm-objcopy/prefix-symbols.test

33

You can avoid having two near-identical blocks by using multiple check-prefixes:

# RUN: llvm-readobj -symbols %t2 | FileCheck %s --check-prefixes=COMMON,BASIC
...
# RUN: llvm-readobj -symbols %t3 | FileCheck %s --check-prefixes=COMMON,REDEF

...

# COMMON: Symbols [
# COMMON-NEXT: Symbol {
# ...
# COMMON-NEXT: Symbol {
# BASIC-NEXT: Name: prefixbar
# REDEF-NEXT: Name: prefixbaz
# COMMON-NEXT: Value: 0x0
...

tools/llvm-objcopy/llvm-objcopy.cpp

369

Nit: Use .str() on the result of this expression, so as to do the concatenation as a Twine:

Sym.Name = (Config.SymbolsPrefix + Sym.Name).str();

test/tools/llvm-objcopy/prefix-symbols.test

+# RUN: yaml2obj %s > %t
+# RUN: llvm-objcopy --prefix-symbols prefix %t %t2
+# RUN: llvm-readobj -symbols %t2 | FileCheck %s
+# RUN: llvm-objcopy --redefine-sym bar=baz --prefix-symbols prefix %t %t3
+# RUN: llvm-readobj -symbols %t3 | FileCheck %s --check-prefix=REDEF
+
+!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
+Symbols:
+  Local:
+    - Name:     foo
+      Type:     STT_SECTION
+      Section:  .text
+    - Name:     bar
+      Type:     STT_FILE
+      Section:  .text
+  Global:
+    - Name:     foobar
+      Type:     STT_FUNC
+      Section:  .text
+
+# 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: 0x0
+# CHECK-NEXT:    Size: 0
+# CHECK-NEXT:    Binding: Local
+# CHECK-NEXT:    Type: Section
+# CHECK-NEXT:    Other: 0
+# CHECK-NEXT:    Section: .text
+# CHECK-NEXT:  }
+# CHECK-NEXT:  Symbol {
+# CHECK-NEXT:    Name: prefixbar
+# CHECK-NEXT:    Value: 0x0
+# CHECK-NEXT:    Size: 0
+# CHECK-NEXT:    Binding: Local
+# CHECK-NEXT:    Type: File
+# CHECK-NEXT:    Other: 0
+# CHECK-NEXT:    Section: .text
+# CHECK-NEXT:  }
+# CHECK-NEXT:  Symbol {
+# CHECK-NEXT:    Name: prefixfoobar
+# CHECK-NEXT:    Value: 0x0
+# CHECK-NEXT:    Size: 0
+# CHECK-NEXT:    Binding: Global
+# CHECK-NEXT:    Type: Function
+# CHECK-NEXT:    Other: 0
+# CHECK-NEXT:    Section: .text
+# CHECK-NEXT:  }
+# CHECK-NEXT:]
+
+# REDEF: Symbols [
+# REDEF-NEXT:  Symbol {
+# REDEF-NEXT:    Name:
+# REDEF-NEXT:    Value: 0x0
+# REDEF-NEXT:    Size: 0
+# REDEF-NEXT:    Binding: Local
+# REDEF-NEXT:    Type: None
+# REDEF-NEXT:    Other: 0
+# REDEF-NEXT:    Section: Undefined
+# REDEF-NEXT:  }
+# REDEF-NEXT:  Symbol {
+# REDEF-NEXT:    Name: foo
+# REDEF-NEXT:    Value: 0x0
+# REDEF-NEXT:    Size: 0
+# REDEF-NEXT:    Binding: Local
+# REDEF-NEXT:    Type: Section
+# REDEF-NEXT:    Other: 0
+# REDEF-NEXT:    Section: .text
+# REDEF-NEXT:  }
+# REDEF-NEXT:  Symbol {
+# REDEF-NEXT:    Name: prefixbaz
+# REDEF-NEXT:    Value: 0x0
+# REDEF-NEXT:    Size: 0
+# REDEF-NEXT:    Binding: Local
+# REDEF-NEXT:    Type: File
+# REDEF-NEXT:    Other: 0
+# REDEF-NEXT:    Section: .text
+# REDEF-NEXT:  }
+# REDEF-NEXT:  Symbol {
+# REDEF-NEXT:    Name: prefixfoobar
+# REDEF-NEXT:    Value: 0x0
+# REDEF-NEXT:    Size: 0
+# REDEF-NEXT:    Binding: Global
+# REDEF-NEXT:    Type: Function
+# REDEF-NEXT:    Other: 0
+# REDEF-NEXT:    Section: .text
+# REDEF-NEXT:  }
+# REDEF-NEXT:]

tools/llvm-objcopy/ObjcopyOpts.td

 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">;
 def keep_file_symbols : Flag<["-", "--"], "keep-file-symbols">,
                       HelpText<"Do not remove file symbols">;
+defm prefix_symbols : Eq<"prefix-symbols">,
+                       MetaVarName<"prefix">,
+                       HelpText<"Add <prefix> to the start of every symbol name">;

tools/llvm-objcopy/Object.h

enum SymbolShndxType {
   SYMBOL_XINDEX = ELF::SHN_XINDEX,
 };
 
 struct Symbol {
   uint8_t Binding;
   SectionBase *DefinedIn = nullptr;
   SymbolShndxType ShndxType;
   uint32_t Index;
-  StringRef Name;
+  std::string Name;
   uint32_t NameIndex;
   uint64_t Size;
   uint8_t Type;
   uint64_t Value;
   uint8_t Visibility;
   bool Referenced = false;
 
   uint16_t getShndx() const;

tools/llvm-objcopy/llvm-objcopy.cpp

struct CopyConfig {
   StringRef OutputFilename;
   StringRef InputFilename;
   StringRef OutputFormat;
   StringRef InputFormat;
   StringRef BinaryArch;
 
   StringRef SplitDWO;
   StringRef AddGnuDebugLink;
+  StringRef SymbolsPrefix;
   std::vector<StringRef> ToRemove;
   std::vector<StringRef> Keep;
   std::vector<StringRef> OnlyKeep;
   std::vector<StringRef> AddSection;
   std::vector<StringRef> SymbolsToLocalize;
   std::vector<StringRef> SymbolsToGlobalize;
   std::vector<StringRef> SymbolsToWeaken;
   std::vector<StringRef> SymbolsToRemove;
LLVM_ATTRIBUTE_NORETURN void reportError(StringRef File, Error E) {
   OS.flush();
   errs() << ToolName << ": '" << File << "': " << Buf;
   exit(1);
 }
 
 } // end namespace objcopy
 } // end namespace llvm
 
 static SectionFlag ParseSectionRenameFlag(StringRef SectionName) {
   return llvm::StringSwitch<SectionFlag>(SectionName)
       .Case("alloc", SectionFlag::SecAlloc)
       .Case("load", SectionFlag::SecLoad)
       .Case("noload", SectionFlag::SecNoload)
       .Case("readonly", SectionFlag::SecReadonly)
       .Case("debug", SectionFlag::SecDebug)
       .Case("code", SectionFlag::SecCode)
       .Case("data", SectionFlag::SecData)
Obj.SymbolTable->updateSymbols([&](Symbol &Sym) {
 
       if (Config.Weaken && Sym.Binding == STB_GLOBAL &&
           Sym.getShndx() != SHN_UNDEF)
         Sym.Binding = STB_WEAK;
 
       const auto I = Config.SymbolsToRename.find(Sym.Name);
       if (I != Config.SymbolsToRename.end())
         Sym.Name = I->getValue();
+
+      if (!Config.SymbolsPrefix.empty() && Sym.Type != STT_SECTION)
+        Sym.Name = Config.SymbolsPrefix.str() + Sym.Name;
     });
 
     // The purpose of this loop is to mark symbols referenced by sections
     // (like GroupSection or RelocationSection). This way, we know which
     // symbols are still 'needed' and wich are not.
     if (Config.StripUnneeded) {
       for (auto &Section : Obj.sections())
         Section.markSymbols();
     }
static CopyConfig ParseObjcopyOptions(ArrayRef<const char *> ArgsArr) {
   Config.InputFilename = Positional[0];
   Config.OutputFilename = Positional[Positional.size() == 1 ? 0 : 1];
   Config.InputFormat = InputArgs.getLastArgValue(OBJCOPY_input_target);
   Config.OutputFormat = InputArgs.getLastArgValue(OBJCOPY_output_target);
   Config.BinaryArch = InputArgs.getLastArgValue(OBJCOPY_binary_architecture);
 
   Config.SplitDWO = InputArgs.getLastArgValue(OBJCOPY_split_dwo);
   Config.AddGnuDebugLink = InputArgs.getLastArgValue(OBJCOPY_add_gnu_debuglink);
+  Config.SymbolsPrefix = InputArgs.getLastArgValue(OBJCOPY_prefix_symbols);
 
   for (auto Arg : InputArgs.filtered(OBJCOPY_redefine_symbol)) {
     if (!StringRef(Arg->getValue()).contains('='))
       error("Bad format for --redefine-sym");
     auto Old2New = StringRef(Arg->getValue()).split('=');
     if (!Config.SymbolsToRename.insert(Old2New).second)
       error("Multiple redefinition of symbol " + Old2New.first);
   }