[PATCH] D78474: [llvm-objcopy][MachO] Make --remove-section clean up dead symbols

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-objcopy/MachO/remove-section-dead-symbols.test:4
+
+# RUN: llvm-readobj --sections --symbols %t.copy | FileCheck %s
+
----------------
alexshap wrote:
> jhenderson wrote:
> > What's the reason for dumping the sections here?
> we have removed one section ("C") - thus we'd like to verify that the list of sections is correct 
> ["__text", "__data"]
I guess my question is more along the lines of "we test the section is removed elsewhere, so do we need to test here?" Happy either way, if you think there's additional value.


================
Comment at: llvm/test/tools/llvm-objcopy/MachO/remove-section-error.test:98
+        reserved3:       0x00000000
+        content:         1400000000000000017A520001781001100C070890010000240000001C000000B0FFFFFFFFFFFFFF0C0000000000000000410E108602430D0600000000000000
+  - cmd:             LC_VERSION_MIN_MACOSX
----------------
alexshap wrote:
> alexshap wrote:
> > alexshap wrote:
> > > jhenderson wrote:
> > > > I know that mach-o yaml2obj requires very noisy output, but I'm guessing you don't need eh_frame data at least? Similarly, is there anything else you can remove?
> > > it's kinda hard to remove things from here because it's tedious and error-prone to manually recalculate (e.g. harder than for ELF) the offsets specified in various places (e.g. in the load commands) and still get a valid binary (so that it can be parsed by llvm-readobj/llvm-objdump/otool).
> > > 
> > > this is particular test is based on a real world binary:
> > >    a.c: 
> > >        __attribute__((section("__DATA,C"))) int a = 2;
> > >        int f() { return a; }
> > >    clang -c a.c -o a.o
> > typo: this particular test is ...
> clang -fno-exceptions -fno-unwind-tables -c a.o seems to work (to get rid of eh_frame, __compact_unwind), I'll trim down the test tomorrow.
Okay, got it. It might be worth including the input and compilation command somewhere in a comment, so that the output can be recreated if needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78474