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

[paulsemel]

tools/llvm-objcopy/llvm-objcopy.cpp

215 ↗

(On Diff #159484)

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

22–24 ↗

(On Diff #159484)

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 ↗

(On Diff #159484)

to start -> to the start

tools/llvm-objcopy/llvm-objcopy.cpp

215 ↗

(On Diff #159484)

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.

372 ↗

(On Diff #159484)

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

22–24 ↗

(On Diff #159484)

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 ↗

(On Diff #159484)

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..

372 ↗

(On Diff #159484)

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 ↗

(On Diff #159484)

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.

372 ↗

(On Diff #159484)

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 ↗

(On Diff #159738)

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 ↗

(On Diff #159738)

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();

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

llvm/trunk/tools/llvm-objcopy/ObjcopyOpts.td

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 dump_section : Eq<"dump-section">,
                    MetaVarName<"section=file">,
                    HelpText<"Dump contents of section named <section> into file <file>">;
+defm prefix_symbols : Eq<"prefix-symbols">,
+                       MetaVarName<"prefix">,
+                       HelpText<"Add <prefix> to the start of every symbol name">;

llvm/trunk/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;

llvm/trunk/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> DumpSection;
   std::vector<StringRef> SymbolsToLocalize;
   std::vector<StringRef> SymbolsToGlobalize;
   std::vector<StringRef> SymbolsToWeaken;
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 + Sym.Name).str();
     });
 
     // 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);
   }