[PATCH] D57738: [llvm-objcopy] Add --redefine-syms

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht added inline comments.


================
Comment at: test/tools/llvm-objcopy/ELF/redefine-symbol.test:7
+# RUN: echo "  foo   oof #rename foo  " > %t.rename.txt
+# RUN: echo "empty" >> %t.rename.txt
+# RUN: llvm-objcopy --redefine-syms %t.rename.txt %t %t3
----------------
Shouldn't this be an error?


================
Comment at: tools/llvm-objcopy/CopyConfig.cpp:258
+    if (!TrimmedLine.empty()) {
+      auto Pair = TrimmedLine.split(' ');
+      SymbolsToRename.insert({Pair.first, Pair.second.trim()});
----------------
It might be more straightforward (though verbose) to just use the other overload of split that puts thing in a SmallVector, and error if the size != 2.


================
Comment at: tools/llvm-objcopy/CopyConfig.cpp:259
+      auto Pair = TrimmedLine.split(' ');
+      SymbolsToRename.insert({Pair.first, Pair.second.trim()});
+    }
----------------
This needs StringSaver to retain these lines once the memory buffer has gone out of scope.

I'm able to reproduce a crash w/ asan by removing `StringSaver` from `addGlobalSymbolsFromFile` above, and the usage here looks similar.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57738/new/

https://reviews.llvm.org/D57738