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

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

According to the pre-merge bot, there are test failures in the new tests.



================
Comment at: llvm/test/tools/llvm-objcopy/MachO/add-section-64.test:1
+## Show that llvm-objcopy adds a new section into the 64-bit object if
+## --add-section is given.
----------------
alexshap wrote:
> jhenderson wrote:
> > 
> in my defense I can only say that this was copied over from the old version (which was not mine)
Yeah, it's fine. Can't hurt to improve pre-existing comments whilst we're in the area though!


================
Comment at: llvm/test/tools/llvm-objcopy/MachO/add-section-64.test:4
+
+# RUN: yaml2obj %s -o %t
+
----------------
You've lost the `echo` line when removing the error case versions. I think that's causing the test to fail. Same in the other test case.

(You probably still have %t.data in your local test output directory, which is why you won't have seen it locally).


================
Comment at: llvm/tools/llvm-objcopy/MachO/Object.cpp:145-147
+  Seg.maxprot |= (MachO::VM_PROT_READ | MachO::VM_PROT_WRITE | MachO::VM_PROT_EXECUTE);
+  Seg.initprot |= (MachO::VM_PROT_READ | MachO::VM_PROT_WRITE | MachO::VM_PROT_EXECUTE);
+  Seg.vmaddr = SegVMAddr;
----------------
clang-format?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87497