[PATCH] D87497: [llvm-objcopy][MachO] Fix --add-section

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added a comment.

The logic LGTM (with my inline questions addressed). I'll let @jhenderson confirm he's happy with the tests now (they make sense to me).



================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp:264
+      for (const std::unique_ptr<Section> &S : LC.Sections)
+        Addr = std::max(Addr, S->Addr + S->Size);
       LC.Sections.push_back(std::make_unique<Section>(Sec));
----------------
Do you need to take section alignment into account here, or does the size already account for that?


================
Comment at: llvm/tools/llvm-objcopy/MachO/Object.cpp:122
+    case MachO::LC_SEGMENT:
+      Addr = std::max(Addr, (uint64_t)MLC.segment_command_data.vmaddr +
+                                MLC.segment_command_data.vmsize);
----------------
Same question about alignment here.

Also nit: I thought `static_cast` was preferred to C-style casts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87497