[PATCH] D33964: [LLVM][llvm-objcopy] Added basic plumbing to get things started

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

The implementation looks good from my point of view. I'm wondering about the test coverage. The current test only tests PROGBITS, NULL, and STRTAB sections. However, your original comment comment claims that it will work with NOBITS. I'd like to see that tested, if this is the case. Also test cases with segments in as well, would be good.



================
Comment at: tools/llvm-objcopy/Object.cpp:169
+    // The segment can have a different alignment than the section. We need to
+    // make ensure that both alignments are respected.
+    if (Section->ParentSegment) {
----------------
"make ensure" -> "make sure" or just "ensure"


https://reviews.llvm.org/D33964