Page MenuHomePhabricator

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

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

Details

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
69

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
110–111

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