[PATCH] D50381: [llvm-objcopy] Add --prefix-symbols option

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: test/tools/llvm-objcopy/prefix-symbols.test:22-24
+    - Name:     foo
+      Type:     STT_SECTION
+      Section:  .text
----------------
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)?


================
Comment at: tools/llvm-objcopy/ObjcopyOpts.td:107
+                       MetaVarName<"prefix">,
+                       HelpText<"Add <prefix> to start of every symbol name">;
----------------
to start -> to the start


================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:215
 
+static std::vector<std::unique_ptr<std::string>> SymbolsPrefixedNames;
+
----------------
paulsemel wrote:
> 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..
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.


================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:372
+        SymbolsPrefixedNames.emplace_back(
+            new std::string(Config.SymbolsPrefix.str() + Sym.Name.str()));
+        Sym.Name = *SymbolsPrefixedNames.back();
----------------
There's not much point in newing a std::string into existence. Just create a vector of strings.


Repository:
  rL LLVM

https://reviews.llvm.org/D50381