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

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added a comment.

This LGTM. I'll let @MaskRay sign off too.



================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp:169
       }
+      VMSize = std::max(VMSize, SectOffset + Sec->Size);
     }
----------------
alexshap wrote:
> smeenai wrote:
> > What purpose is the `std::max` serving here, given that `VMSize` is set to 0 before the loop and isn't modified inside the loop, as far as I can see?
> @smeenai  - good question, 
> (if I'm not mistaken this code was influenced by the old LLD for MachO)
> the sections listed in one LC_SEGMENT_64 load command in general appear to be not ordered by their virtual addresses. 
> 
> P.S. I've also done a small experiment - one can see this happens for some object files created by llvm-mc
> 
>    llvm-mc -assemble -triple x86_64-apple-darwin9 -filetype=obj Inputs/various-symbols.s -o real-world-input-copy.test.tmp.various-symbols.o
>    llvm-readobj --sections real-world-input-copy.test.tmp.various-symbols.o ...
> 
Ah, that makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78799