[PATCH] D59763: [llvm-objcopy] - Strip sections before symbols.

Mon Mar 25 08:47:11 PDT 2019

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

In D59763#1441401 <https://reviews.llvm.org/D59763#1441401>, @grimar wrote:

> In D59763#1441316 <https://reviews.llvm.org/D59763#1441316>, @jhenderson wrote:
>
> > I think another interesting test might be `-N <sym>
>
>
> Something is wrong I guess here, as I do not see -N in llvm-objcopy help... I used  `--strip-symbol=` instead.

I'm guessing short aliases aren't in the help text, but --strip-symbol is fine.

LGTM, with a couple of minor comments. Maybe worth leaving it overnight before committing though, in case any of the US contributors have any comments.

================
Comment at: test/tools/llvm-objcopy/ELF/strip-all-relocations.test:1
+# RUN: yaml2obj %s > %t
+
----------------
Okay, since you've added the new test case to this test, this test is badly named again. Sorry! Perhaps "strip-symbol-and-relocation.test"?

================
Comment at: test/tools/llvm-objcopy/ELF/strip-all-relocations.test:29
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ] 
+    Content:         00000000
----------------
Nit: you can delete the Flags, as they aren't relevant to the test.

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

https://reviews.llvm.org/D59763