- User Since
- Dec 5 2017, 3:14 PM (97 w, 2 d)
Tue, Oct 15
Fri, Sep 27
Mon, Sep 23
Logic looks fine. I have some concerns around alignment terminology that I suggest you address before committing.
Fri, Sep 20
mechanical comment changes
Updating to use path::append to build the bundle contents string.
Thu, Sep 19
Wed, Sep 18
Aug 6 2019
Jul 31 2019
LGTM, visual inspection only.
Jul 30 2019
Jul 2 2019
Jun 24 2019
I wonder why the Mach-O specific commands are unlike that of other utilities ... In any case, these changes look fine.
Jun 21 2019
Jun 20 2019
Jun 19 2019
Jun 18 2019
Would be nice if "llvm-lipo -thin" printed a reasonable error message before dumping usage. Apple lipo will print "missing argument to -thin option"
Jun 15 2019
I'll have a look Monday / early next week. Meanwhile, here are some quick thoughts.
Respectfully, I am not a good reviewer for ELF-specific file format changes. I assume you added me as a proxy for 'enderby', who touched some of the lines of this routine. Looking at the SVN history I believe Kevin Enderby's involvement was limited to promulgating Lang Hames' "Expected<>" as the preferred error handling idiom in libObject and friends. So, again, respectfully, neither Kevin nor I are what I would consider good, authoritative reviewers for ELF.
All this looks reasonable, but my grasp of Mach-O is not as complete as I would like, and I have some questions for you.
Jun 11 2019
Jun 10 2019
approving this revision in light of my notes inline. I still believe this command guide is going to need an editorial pass once all of the options are in. my main concern for the moment is that the information is accurate and complete.
Jun 7 2019
Thanks for making this!
Jun 6 2019
I had trouble getting recommonmark.parser to install on my system, so I hacked at the patch to get the html and groff formatters to run. Should not be relevant to this review.
I am not familiar with lldb or with the DebugView library, so I am pulling in Fred Riss for this review. Thanks!
I did not download or run this code, but I believe it does not correctly divine the Arch flag from a given Mach-O binary. This is straight-forward to fix.
Jun 4 2019
Trim unused space from the macho-dylib.test file.
Jun 3 2019
Given the problems we've seen with windows and netbsd, I think it makes sense for llvm-obdump to have a better, more bespoke error case, one that is stable on all platforms. so let's pull this test.
Jun 1 2019
"It does not seem to serve any real purpose..."
May 30 2019
Remove tabs inserted by emacs (grr)
Increasing error checking to catch symbolic files that are neither
Mach-O nor Universal object files.
May 29 2019
May 23 2019
That's a pretty big hammer. Let's give it a try!
May 22 2019
May 21 2019
While this seems to be a direct way to address this, should the ELF object file instead begin the symbol range at 1 instead of 0? That way all clients of libObject know to skip the undefined range and you don't need to modify every call site.
May 17 2019
May 15 2019
A second thought on the topic of lipo compatibility. libObject is probably reasonable if your primary interest is MH_OBJECT files. There exist a number of final-linked-images that libObject's fat parser will not currently understand; these are formats that cannot be produced by llvm tools today. I wonder if choosing a different name for the tool might give you some flexibility to "do things differently" than lipo. That said, I don't have a better name in mind. :)
May 14 2019
I hate to be "That Guy" but this really needs a docs/CommandGuide file.
Apr 18 2019
Mar 20 2019
Mar 19 2019
Mar 11 2019
Mar 9 2019
Mar 6 2019
Feb 26 2019
I agree, simply updating the enum by itself doesn't accomplish much. Is there something specific driving this change?
Feb 22 2019
Built it locally, stepped through the nm code for Mach-O files, didn't see any unintended problems. Elf changes reviewed visually only, they look mechanical. The name for "getNMSectionTagAndName" seems imperfect to me, as the Mach-O symbol type code isn't strictly about the section (A, I, -); that said, sysv is not a common format for viewing Mach-O, so I just offer it as a minor point to consider.
Feb 11 2019
Crud! The wrong code got into this arc request. I'm going to abandon the revision and try again.
... and fix the lit test when always printing the imp pointer.
Always print the imp pointer.
Jan 25 2019
Jan 24 2019
llvm-nm and BinaryFormat changes look good to me.