[PATCH] D90796: [llvm-objcopy][MachO] Skip sections with zero offset

[Shoaib Meenai via Phabricator via llvm-commits]

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

LGTM



================
Comment at: llvm/test/tools/llvm-objcopy/MachO/zero-offset-section.test:1
+## This test verifies that a regular (non-zero-fill) section with zero offset 
+## is not modifed, this behavior is consistent with cctools' strip.
----------------
alexshap wrote:
> alexshap wrote:
> > smeenai wrote:
> > > Zero offset *and* zero size, right?
> > yeah
> unfortunately I'm not sure I can post the binaries (and not sure how exactly they were created).
> 
> P.S. Initially I was considering skipping sections with zero size, but after some search I've found mentions of binaries with zero-size sections and non-zero offsets (in fact, even in our existing tests we have such examples, but I've noticed them e.g. here: https://gitlab.haskell.org/TDecki/ghc/commit/0d31ccdd5754b10fb27fcdc95ddd6c937ecec1bd). Therefore, the approach in this diff (where we look at the original file offset) seems to be safer (it works fine for both cases).
I meant that the comment should say "with zero offset and zero size" :)

Might be worth adding some test cases for what happens if you have one but not the other.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90796