[PATCH] D50381: [llvm-objcopy] Add --prefix-symbols option
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 7 08:46:15 PDT 2018
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
More information about the llvm-commits
mailing list