[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
----------------
evgeny777 wrote:
> rupprecht wrote:
> > Shouldn't this be an error?
> I don't know. GNU objdump treats this a error. However this line above
> ```
> llvm-objcopy --redefine-sym foo=oof --redefine-sym empty= %t %t2
> ```
> says that empty symbol names are allowed
Hmm.... that's weird. I guess it's consistent.

I'll defer to @alexshap @jakehehrlich @jhenderson if they feel otherwise, but I think it might be fine to deviate from `--redefine-sym` and require two names -- I could imagine someone accidentally filling a text file with "foo=bar" instead of "foo bar". The downside is that if someone wants to set all their symbols to empty names, they'd have to do it painfully (add `--redefine-sym=foo=` for each one), but I think it's worth it for the much more common case of not getting the format right on the first try.

(The flag case is a less error prone, since `foo=` makes it obvious that you are assigning a symbol to something empty)


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

https://reviews.llvm.org/D57738