[PATCH] D70036: [llvm-objcopy][COFF] Implement --redefine-sym and --redefine-syms

Martin Storsjö via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 12 05:05:28 PST 2019


mstorsjo added a comment.

Looks good in general, with two minor comments



================
Comment at: llvm/docs/CommandGuide/llvm-objcopy.rst:96
+ file, each line represents a single symbol to rename, with the old name and new
+ name separated by an equals sign. Leading and trailing whitespace is ignored,
+ as is anything following a '#'. Can be specified multiple times to read names
----------------
While this just moves the documentation, it seems to me that this documentation is wrong; in the text file, there's no equals sign between old and new names, they're just separated by whitespace.


================
Comment at: llvm/tools/llvm-objcopy/COFF/COFFObjcopy.cpp:138
+      Sym.Name = I->getValue();
+  }
+
----------------
One pretty relevant functional aspect of this, is the fact that a request to rename a symbol that doesn't exist passes through silently (which I presume matches GNU behaviour). It might be good to add a test of that aspect.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70036





More information about the llvm-commits mailing list