This is an archive of the discontinued LLVM Phabricator instance.

[llvm-objcopy][MachO] Implement --dump-section
ClosedPublic

Authored by seiya on Aug 19 2019, 1:53 AM.

Diff Detail

Event Timeline

seiya created this revision.Aug 19 2019, 1:53 AM
jhenderson added inline comments.Aug 19 2019, 6:31 AM
llvm/test/tools/llvm-objcopy/MachO/dump-section.test
12

Nit: nonexistent -> non-existent

17

It probably makes more sense to put the positive cases before the negative ones.

seiya updated this revision to Diff 215992.Aug 19 2019, 2:46 PM
seiya marked 2 inline comments as done.

Fixed review comments.

This revision is now accepted and ready to land.Aug 19 2019, 3:35 PM
jhenderson accepted this revision.Aug 20 2019, 2:20 AM

LGTM too (though I've not tried to look too hard at the Mach-O code).

MaskRay added inline comments.
llvm/test/tools/llvm-objcopy/MachO/dump-section.test
65

It seems reserved3 is optional but reserved[12] are not:

// lib/ObjectYAML/MachOYAML.cpp#L279
  IO.mapRequired("reserved1", Section.reserved1);
  IO.mapRequired("reserved2", Section.reserved2);
  IO.mapOptional("reserved3", Section.reserved3);

If some field are not significant in the test, they can be deleted.

seiya updated this revision to Diff 216123.Aug 20 2019, 6:08 AM

Update CommandGuide.

jhenderson added inline comments.Aug 20 2019, 6:19 AM
llvm/docs/CommandGuide/llvm-objcopy.rst
70

remove "the" before "<section>".

seiya updated this revision to Diff 216126.Aug 20 2019, 6:19 AM
seiya marked 2 inline comments as done.

Addressed review comments.

llvm/test/tools/llvm-objcopy/MachO/dump-section.test
65

Oh I didn't noticed that. Thanks.

seiya updated this revision to Diff 216131.Aug 20 2019, 6:25 AM

CommandGuide: Removed remove "the" before "<section>".

seiya marked an inline comment as done.Aug 20 2019, 6:25 AM
rupprecht accepted this revision.Aug 20 2019, 12:42 PM
rupprecht added inline comments.
llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp
85–86

llvm::copy(Sec.Content, Buf->getBufferStart())

seiya updated this revision to Diff 230584.Nov 21 2019, 7:50 PM

Rebased. NFCI.

seiya marked an inline comment as done.Nov 21 2019, 7:50 PM
seiya added a comment.Nov 22 2019, 8:19 PM

I'll commit this patch on next Monday.

MaskRay added inline comments.Nov 22 2019, 9:10 PM
llvm/test/tools/llvm-objcopy/MachO/dump-section.test
5

-o (it seems -o is more common. I prefer it because it avoids a shell redirection)

seiya updated this revision to Diff 230829.Nov 24 2019, 7:21 PM
seiya marked an inline comment as done.

Addressed a comment.

This revision was automatically updated to reflect the committed changes.