Page MenuHomePhabricator

implement MachODumper::printFileHeaders
ClosedPublic

Authored by chilledheart on Nov 6 2014, 9:03 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

chilledheart retitled this revision from to implement MachODumper::printFileHeaders.
chilledheart updated this object.
chilledheart edited the test plan for this revision. (Show Details)
chilledheart added a subscriber: Unknown Object (MLST).

update for code format

friss added a subscriber: friss.Nov 11 2014, 3:55 PM

Thanks for working on this!

This needs some test coverage, you need to update test/tools/llvm-readobj/file-headers.test. Other comments inline.

tools/llvm-readobj/MachODumper.cpp
207–208 ↗(On Diff #15911)

Not sure about the convention of llvm-readobj, but you might want to call that MachHeader instead (The ELF one is called ElfHeader, and moreover this would nicely match name of the structure it is describing)

209–228 ↗(On Diff #15911)

I think this would be nicer with a little template helper to use the same code for dumping the common fields. And then just special case the Reserved field that appears only in the 64bits version (given the name, I'm wondering if it should be dumped at all).

And most of the fields could be pretty-printed, you can take inspiration from what is done in ELFDumper and use the various enum/helpers in llvm/Support/MachO.h.

the name "MachHeader" is better than plain "Header"

use a little template helper

print magic, filetype and cputype more pretty

correct some typos

update

test part is comming soon

update format

add test cases

friss accepted this revision.Nov 12 2014, 11:01 AM
friss added a reviewer: friss.

This LGTM, can you commit it, or do you want me to land it for you?

This revision is now accepted and ready to land.Nov 12 2014, 11:01 AM

Really appreciate it if you land it for me.

nrieck added a subscriber: nrieck.Nov 13 2014, 4:35 AM

Since llvm-readobj is a low-level inspection tool, the Reserved field should also be printed (even more so than unused fields).

chilledheart added a comment.EditedNov 13 2014, 7:24 AM
In D6163#26, @nrieck wrote:

Since llvm-readobj is a low-level inspection tool, the Reserved field should also be printed (even more so than unused fields).

Yeah. As I know, macho-dump does dump the reserved field for 64-bit version of Mach-O format, while otool -h and llvm-objdump don't.

tools/llvm-readobj/MachODumper.cpp
207–208 ↗(On Diff #15911)

Yeah, MachHeader is better.

207–208 ↗(On Diff #15911)

Maybe I am a bit overconcerned about this. Since it (the header) belongs to Mach-O format, is MachOHeader more suitable than MachHeader?

209–228 ↗(On Diff #15911)

Thanks for pointing it out. I will work on it soon.

chilledheart edited edge metadata.

dump reserved field for 64bit mach-o object

Hi Frederic,
If you have some free time this weekend, could you land it for me? Btw I don't have a subversion account yet and am not able to do it myself.

Thanks!

No problem, I'll do it before the end of the weekend. Sorry for the delay, I've been busy with other thinks.

Thanks!
Fred

No worry about that. Take your time.

Cheers,
Chilledheart

friss closed this revision.Nov 16 2014, 5:34 PM
friss updated this revision to Diff 16279.

Closed by commit rL222115 (authored by @friss).