[PATCH] D78799: [llvm-objcopy][MachO] Fix segment's vmsize

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap marked an inline comment as done.
alexshap added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp:161
           Sec->Offset = 0;
-          VMSize += Sec->Size;
         } else {
----------------
alexshap wrote:
> @MaskRay: the issue was here. (VMSize += Sec->Size) - this is not correct (and it results in producing a broken binary),
> also you can  take a look at the newly added test and the comment therein. I've already mentioned in the past reviews that for tests containing sections it's error-prone to modify MachO yaml manually, even removing one seemingly unrelated load command triggers the recalculation of the offsets mentioned inside other load commands + the test is not really large, it's the minimal example which triggers the issue (because of the page alignment). Having a valid binary also enables us to use the existing tools from Xcode to validate / introspect the binary if necessary.
to the best of my knowledge this test is not supposed to be reorganized, and if i'm not mistaken it should not even change (because it copies the binary without modifications). P.S. the test fails (as expected) without the fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78799