[PATCH] D57248: [llvm-objcopy] Support -X|--discard-locals.

Tue Jan 29 07:54:01 PST 2019

rupprecht marked 4 inline comments as done.
rupprecht added a comment.

In D57248#1375099 <https://reviews.llvm.org/D57248#1375099>, @jhenderson wrote:

> Something that's worth noting is that section symbols often don't actually have a name. Some tools use their section name instead for clarity. I'm wondering if this might cause an issue for such a section symbol for a section named ".L..." e.g. ".LLVM.Custom.Section" or whatever? Definitely it deserves including in the testcase.

Added

Of note -- GNU objcopy is indeed enforcing the empty section name by clearing the symbol name when copying the `.L.LocalSection` symbol here. We preserve it, not sure if that matters.
Also two other differences: GNU objcopy is promoting `.L.LocalFile` and `.L.undefined` from locals to globals. Again, I think that doesn't matter too much, those files should probably be global to begin with.

================
Comment at: llvm/test/tools/llvm-objcopy/ELF/discard-locals.test:60-64
+    - Name:     .L.undefined
+    - Name:     .L.abs
+      Index:    SHN_ABS
+    - Name:     .L.common
+      Index:    SHN_COMMON
----------------
jhenderson wrote:
> These aren't local, so won't be discarded anyway (and therefore aren't testing anything). They should probably be in the local list...
I don't think a local SHN_COMMON symbol makes any sense, so I just removed that case, but moved the others to local... although I'm not sure the others make sense to be local either.

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

https://reviews.llvm.org/D57248