jakehehrlich (Jake Ehrlich)
User

Projects

User does not belong to any projects.

User Details

User Since
May 16 2017, 11:34 AM (74 w, 4 h)

Recent Activity

Yesterday

jakehehrlich added a comment to D53084: [clang-doc] Add unit tests for YAML.

This just seems like a lit test in unit test form, why does this need to use unit tests and not lit tests?

Mon, Oct 15, 2:26 PM · Restricted Project
jakehehrlich accepted D53083: [clang-doc] Add unit tests for merging.
Mon, Oct 15, 2:22 PM · Restricted Project
jakehehrlich accepted D53081: [clang-doc] Add unit tests for serialization.

LGTM, I'd file a bug about SmallString not taking const char* in its constructor and I'd also put a TODO somewhere to fix that once that's resolved.

Mon, Oct 15, 2:13 PM · Restricted Project

Tue, Oct 9

jakehehrlich added a comment to D51009: [opt] Change the parameter of OptTable::PrintHelp from Name to Usage and don't append "[options] <inputs>".

.

Tue, Oct 9, 5:28 PM
jakehehrlich committed rL344080: [llvm-objcopy] Make -S an alias for --strip-all.
[llvm-objcopy] Make -S an alias for --strip-all
Tue, Oct 9, 2:16 PM
jakehehrlich added a comment to D52182: [llvm-objcopy] Make -S an alias for --strip-all.

Hey, sorry. I've been gone for 2 weeks between ICFP/Strange Loop and being in Seattle. Commit this now.

Tue, Oct 9, 12:19 PM
jakehehrlich accepted D53029: [llvm-objcopy] Add -F|--target compatibility.

I think this is probably fine. I think having the error is better than not having it. I'm just concerned someone somewhere is using GNU objcopy in the way mentioned in the error. I'm fine treating this as an error and only switching behavior if and when someone hits an issue.

Tue, Oct 9, 12:18 PM

Fri, Sep 28

jakehehrlich added a comment to D52660: [CMake][Fuchsia] Use unstable libc++ ABI for Fuchsia toolchain.

We have no such desire. I think we're fine being the test subjects for
ABIv2 however.

Fri, Sep 28, 5:09 PM

Thu, Sep 20

jakehehrlich accepted D52328: [llvm-objcopy/llvm-strip]: handle --version.

So I'm good with this BTW.

Thu, Sep 20, 4:59 PM

Mon, Sep 17

jakehehrlich added a comment to D52180: [llvm-objcopy] Change --only-keep to --only-sections.

Who put me in charge of this? I'm clearly unqualified.

Mon, Sep 17, 12:54 PM
jakehehrlich created D52182: [llvm-objcopy] Make -S an alias for --strip-all.
Mon, Sep 17, 12:05 PM
jakehehrlich created D52180: [llvm-objcopy] Change --only-keep to --only-sections.
Mon, Sep 17, 11:24 AM
jakehehrlich added a comment to D52163: -S as an alias for --strip-all-gnu.

-S should be an alias for --strip-all not --strip-all-gnu. There's seldom if ever going to be someone who rely's on the difference between the two. --strip-all-gnu exists as a way to allow people to hack llvm-objcopy into their system for entirely imagined use cases that we have not thus far actually seen. I'll be updating this patch.

Mon, Sep 17, 11:07 AM

Sep 12 2018

jakehehrlich accepted D51961: [objcopy] make objcopy follow program header standards.

LGTM.

Sep 12 2018, 10:25 AM

Sep 11 2018

jakehehrlich requested changes to D51961: [objcopy] make objcopy follow program header standards.

I'll LGTM after Roland's fixes are done since I agree, that would be nice to also fix.

Sep 11 2018, 7:00 PM
jakehehrlich added inline comments to D51961: [objcopy] make objcopy follow program header standards.
Sep 11 2018, 6:53 PM
jakehehrlich accepted D51961: [objcopy] make objcopy follow program header standards.

Oh also LGTM

Sep 11 2018, 6:10 PM
jakehehrlich added a comment to D51961: [objcopy] make objcopy follow program header standards.

When you write your diff use git diff -U<some big number> origin/master so that the diff is relative to what's upstream not your last change locally. I think git show might be understood by fabricator as well but I'm not 100% sure.

Sep 11 2018, 6:06 PM
jakehehrlich added a comment to D51961: [objcopy] make objcopy follow program header standards.

A couple nits but this LGTM.

Sep 11 2018, 4:56 PM

Sep 10 2018

jakehehrlich added inline comments to D51841: [llvm-objcopy] Dwarf decompression support. .
Sep 10 2018, 2:26 PM
jakehehrlich added a comment to D51841: [llvm-objcopy] Dwarf decompression support. .

So I was thinking that we might try and detect compressed section at section construction time instead. The we can implement decompression in terms of CompressedSection...the only blemish seems to be that CompressedSection always allocates. I'm not super sure what the right solution to that is.

Sep 10 2018, 12:21 PM

Sep 7 2018

jakehehrlich updated subscribers of D51792: [AArch64] Attempt to parse expressions as adr/adrp operands.

Awesome! I'm looking at this now.

Sep 7 2018, 3:30 PM

Sep 4 2018

jakehehrlich accepted D51660: [llvm-strip] Support stripping multiple input files.

Whoops...I meant to say the following already. I was confused why it was missing my LGTM.

Sep 4 2018, 8:11 PM
jakehehrlich accepted D51493: [llvm-strip] Allow copying relocation sections without symbol tables..

Confirmed, using head solves this issue; I was just using a very stale checkout.

Sep 4 2018, 3:19 PM
jakehehrlich added a comment to D51493: [llvm-strip] Allow copying relocation sections without symbol tables..

Hmm...this is odd...maybe I missed a change to yaml2obj. I'm rebuilding now.

Sep 4 2018, 2:12 PM
jakehehrlich added a comment to D51493: [llvm-strip] Allow copying relocation sections without symbol tables..

When I run that yaml2obj test I actually get the following error rather than a silent failure.

Sep 4 2018, 2:09 PM
jakehehrlich added a comment to D51493: [llvm-strip] Allow copying relocation sections without symbol tables..

Did you confirm that yaml2obj respects the explicit Link field?

Sep 4 2018, 1:56 PM
jakehehrlich added a comment to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..

Ah so I think this might be an issue with builders that don't build with zlib or that link zlib in in strange ways. I remember I mentioned (many weeks ago) that we'd need to #ifdef some of this stuff into compartments so that it didn't affect non zlib builds. I didn't remember to check for that before giving the LGTM. That is certainly killing at least some bots. I think the downstream bots that Petr mentioned are however actually linking in zlib. It isn't clear why this is breaking those but I'll try and look into that today.

Sep 4 2018, 10:45 AM
jakehehrlich requested changes to D51493: [llvm-strip] Allow copying relocation sections without symbol tables..

Sorry, I didn't mean to accept this change. I was about to and then thought about the Link issue I mentioned in the test. I don't know how to cancel an approval in phabricator so I'm marking it as "request changes". Nothing is critically wrong here, I just want to make sure the test works as intended.

Sep 4 2018, 10:36 AM
jakehehrlich accepted D51493: [llvm-strip] Allow copying relocation sections without symbol tables..
Sep 4 2018, 10:36 AM
jakehehrlich added inline comments to D51493: [llvm-strip] Allow copying relocation sections without symbol tables..
Sep 4 2018, 10:31 AM

Aug 31 2018

jakehehrlich accepted D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..

I am satisfied that James' two remaining issues (as I understand them, more efficient looping in compressSections and minimal use of MCTargetOptions) have been solved and that mine have been solved.

Aug 31 2018, 1:19 PM

Aug 29 2018

jakehehrlich accepted D51468: [sanitizer] Transition to new _zx_vmar_... calls.
Aug 29 2018, 6:24 PM
jakehehrlich added a comment to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..

This is looking pretty close to good to me. I have a couple more nits about how alignment and RemovePred should work but other than that I think I'm good. If those are fixed and someone else accepts this revision before I get back to this feel free to land it without further input from me.

Aug 29 2018, 3:11 PM

Aug 28 2018

jakehehrlich added inline comments to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..
Aug 28 2018, 2:18 PM
jakehehrlich added inline comments to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..
Aug 28 2018, 1:25 PM

Aug 27 2018

jakehehrlich added inline comments to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..
Aug 27 2018, 3:13 PM
jakehehrlich added inline comments to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..
Aug 27 2018, 1:41 PM

Aug 24 2018

jakehehrlich added inline comments to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..
Aug 24 2018, 5:12 PM
jakehehrlich added inline comments to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..
Aug 24 2018, 3:37 PM

Aug 23 2018

jakehehrlich added inline comments to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..
Aug 23 2018, 6:47 PM
jakehehrlich accepted D51137: [clang-doc] Fix memory leaks.
Aug 23 2018, 10:31 AM

Aug 22 2018

jakehehrlich added inline comments to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..
Aug 22 2018, 1:43 PM
jakehehrlich added inline comments to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..
Aug 22 2018, 1:00 PM
jakehehrlich added a comment to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..

Ok so a few higher level comments

Aug 22 2018, 12:33 PM

Aug 21 2018

jakehehrlich requested changes to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..
Aug 21 2018, 5:22 PM
jakehehrlich added a comment to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..

This is looking better!

Aug 21 2018, 5:20 PM

Aug 14 2018

jakehehrlich added inline comments to D50343: [llvm-objcopy] Add support for -I binary -B <arch>..
Aug 14 2018, 12:10 PM

Aug 13 2018

jakehehrlich accepted D50684: [CMake] Split -gx strip flag into -g -x.
Aug 13 2018, 6:59 PM
jakehehrlich added inline comments to D50343: [llvm-objcopy] Add support for -I binary -B <arch>..
Aug 13 2018, 11:41 AM

Aug 9 2018

jakehehrlich committed rL339394: Add owner for llvm-objcopy.
Add owner for llvm-objcopy
Aug 9 2018, 3:05 PM
jakehehrlich added a comment to D50463: [llvm-objcopy] Add --prefix-sections option.

I don't think I want this option to do anything more than literally add a prefix to all section names. It isn't clear that those precise semantics are being relied on anywhere and the behavior of making relocation names and not renaming symtab etc... is a result of how bfd reconstructs the output binary without consideration for what the user initially put in which is opposite the stated goal of llvm-objcopy.

Aug 9 2018, 1:10 PM
jakehehrlich added a comment to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..

Sorry, I'm not ready to land this yet.

Aug 9 2018, 1:00 PM
jakehehrlich added a comment to D50343: [llvm-objcopy] Add support for -I binary -B <arch>..

Awesome! I'd like to have someone else look over this but this pretty well LGTM.

Aug 9 2018, 10:53 AM

Aug 8 2018

jakehehrlich accepted D50381: [llvm-objcopy] Add --prefix-symbols option.

Nice. We need the std::string change for names for several other changes. LGTM.

Aug 8 2018, 10:49 PM
jakehehrlich accepted D50463: [llvm-objcopy] Add --prefix-sections option.

LGTM

Aug 8 2018, 10:47 PM
jakehehrlich added inline comments to D50343: [llvm-objcopy] Add support for -I binary -B <arch>..
Aug 8 2018, 10:39 PM
jakehehrlich accepted D49979: [llvm-objcopy] Add --dump-section.

oh god I apologize. I was wondering why this wasn't already submitted.

Aug 8 2018, 10:25 PM
jakehehrlich added inline comments to D50343: [llvm-objcopy] Add support for -I binary -B <arch>..
Aug 8 2018, 3:12 PM
jakehehrlich added inline comments to D50343: [llvm-objcopy] Add support for -I binary -B <arch>..
Aug 8 2018, 2:58 PM
jakehehrlich added inline comments to D50343: [llvm-objcopy] Add support for -I binary -B <arch>..
Aug 8 2018, 1:16 PM

Aug 6 2018

jakehehrlich added a comment to D50117: [llvm-objcopy] NFC: Refactor main objcopy method that takes an ELFReader to a generic Reader..

So I think I want to remove -split-dwo as an option all together so that HandleArgs can just not worry about all of this.

Aug 6 2018, 3:08 PM
jakehehrlich added a comment to D50343: [llvm-objcopy] Add support for -I binary -B <arch>..

Ah so I think I'm against BinaryReader being templated against ELFT. That is required to implement getElfType() however. getElfType drives which writer we create. That shouldn't be how things work however. I'll go and add my related comments to this to the previous patch. Sorry if I didn't see what was going on in the other patch and lead you astray. This patch makes things a bit more clear.

Aug 6 2018, 2:33 PM

Aug 3 2018

jakehehrlich accepted D50206: [lit, python] Always add quotes around the python path in lit.
Aug 3 2018, 4:42 PM

Aug 2 2018

jakehehrlich accepted D50208: [clang-doc] Fix unique_ptr error on bots.

Make sure the commit message reflects this change but other than that LGTM

Aug 2 2018, 5:29 PM · Restricted Project
jakehehrlich added a comment to D50117: [llvm-objcopy] NFC: Refactor main objcopy method that takes an ELFReader to a generic Reader..

Speaking of that conflict I mentioned, it won't happen! So we're good to go. This change more or less LGTM. Please resolve Alex's comments however.

Aug 2 2018, 11:59 AM
jakehehrlich added a comment to D49979: [llvm-objcopy] Add --dump-section.

This LGTM other than one issue

Aug 2 2018, 11:50 AM

Aug 1 2018

jakehehrlich added a comment to D49979: [llvm-objcopy] Add --dump-section.

Due to the soon encroaching change where -I binary will be implemented I'm very much in favor of the OriginalData field being added to SectionBase. Really seems to handle everything very cleanly IMO.

Aug 1 2018, 2:48 PM
jakehehrlich added inline comments to D50117: [llvm-objcopy] NFC: Refactor main objcopy method that takes an ELFReader to a generic Reader..
Aug 1 2018, 2:47 PM
jakehehrlich added a comment to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..

I think I'd like to make a recommendation that we implement compression first and then decompression next.

Aug 1 2018, 2:22 PM
jakehehrlich added inline comments to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..
Aug 1 2018, 1:52 PM
jakehehrlich added a comment to D50117: [llvm-objcopy] NFC: Refactor main objcopy method that takes an ELFReader to a generic Reader..

I forsee a slight conflict between this and another change but it isn't clear how that will work out and I'm not sure what the best solution is. This change overall LGTM so maybe we'll just let things shake out and deal with that conflict as we have more concrete plans.

Aug 1 2018, 1:26 PM
jakehehrlich updated subscribers of D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..

Compressed sections are a general kind of section. You can compress
literally any kind of section. All relocations are in terms of the
decompressed section. We can't use ELFSectionWriter on say symbol tables
because the actual data there isn't finalized until finalization. We don't
have to concern ourselves with how compression and modification interact
for debug sections because we don't modify those.

Aug 1 2018, 10:44 AM

Jul 31 2018

jakehehrlich added a comment to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..

Yeah I think (to within the margin of the details we've worked out) we seem to roughly agree @alexshap

Jul 31 2018, 6:23 PM
jakehehrlich added a comment to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..

I'm going to outline a proposal for how I think this stuff should work. It turns out I support creating a new section type after all (needed to support a few things like constructing the section name from a Twine and moving the compressed data into the Section's ownership in a clean way). I think this is a faithful version of what James (James please say so if this is not what you had in mind) and I have in mind for how to handle this (well I had to change my stance slightly). If the name ownership problem is solved and we figure out a nice way to plumb the 32-bit vs 64-bit information though that would be even better!

Jul 31 2018, 6:01 PM
jakehehrlich added a comment to D49979: [llvm-objcopy] Add --dump-section.

So here's an idea. It is ELF specific but it extends the SectionBase interface in a way I've considered for a while and would make both this and another change quite easy. If we add an OriginalData field (ArrayRef<uint8_t>) to SectionBase we can dump that inside of HandleArgs.

Jul 31 2018, 5:05 PM
jakehehrlich accepted D50100: [llvm-objcopy] Make --strip-debug strip .gdb_index.
Jul 31 2018, 2:26 PM
jakehehrlich added inline comments to D50100: [llvm-objcopy] Make --strip-debug strip .gdb_index.
Jul 31 2018, 2:09 PM
jakehehrlich added inline comments to D49979: [llvm-objcopy] Add --dump-section.
Jul 31 2018, 1:24 PM
jakehehrlich added a comment to D50100: [llvm-objcopy] Make --strip-debug strip .gdb_index.

Can we add test? Otherwise this LGTM.

Jul 31 2018, 1:22 PM

Jul 30 2018

jakehehrlich added a comment to D49979: [llvm-objcopy] Add --dump-section.

Can you file a bug against me on the segfault? That sounds like a serious issue that needs to be addressed.

Jul 30 2018, 11:38 AM
jakehehrlich added a comment to D49979: [llvm-objcopy] Add --dump-section.

Thanks! I've been meaning to implement this forever. For instance this is the only way to dump non-allocated sections. I've run into cases debugging things where that was useful and had to use objcopy.

Jul 30 2018, 11:36 AM

Jul 27 2018

jakehehrlich accepted D49870: [llvm-objcopy] Add support for --rename-section flags from gnu objcopy.

Forgot to accept

Jul 27 2018, 5:31 PM
jakehehrlich added a comment to D49870: [llvm-objcopy] Add support for --rename-section flags from gnu objcopy.

LGTM, I had the same confusion James did. I have independently confirmed this is the correct behavior. As much as I dislike this behavior I think we need to match what GNU objcopy does here because this might result in observable differences in behavior (like a segfault if this is run on a ,o file and then linked)

Jul 27 2018, 5:31 PM

Jul 26 2018

jakehehrlich added a comment to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..

Yes, that's exactly why: to make it so that llvm-objcopy could be used as a dropin replacement for objcopy in places where objcopy --compress-debug-sections is being used.

Jul 26 2018, 8:29 PM
jakehehrlich added a comment to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..

Also I think you can build llvm with or without zlib. I think that means you're going to need to #ifdef a lot of this code.

As an additional point, it would be good to minimise the number of distinct areas where we have to add these #ifdefs. I'd recommend trying to hide as much as possible of this behind an interface of some kind, which turns into a no-op or warning/error if --compress-debug-sections is used without zlib support.

Also I wanted to say I'm pro this. This should be achieved regardless of which idea we settle on.

Jul 26 2018, 11:39 AM
jakehehrlich added a comment to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..

Yeah I really really think this should be handled by constructing new OwnedDataSections. The compression code should also be templated over an ELFT as well to enable using ELFT::Chdr.

Jul 26 2018, 11:22 AM

Jul 25 2018

jakehehrlich added a comment to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..

Also also, why do you need this? I ask because from talking with some people about what this is used for there is almost certainly a better way to accomplish any gain you get out of it. The main reason I can still see to do this is to achieve command line compatibility; if that is the goal we can probably noop or just noop with a section rename.

Jul 25 2018, 7:51 PM
jakehehrlich added a comment to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..

Also I think you can build llvm with or without zlib. I think that means you're going to need to #ifdef a lot of this code.

Jul 25 2018, 7:45 PM
jakehehrlich added inline comments to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..
Jul 25 2018, 7:39 PM
jakehehrlich added reviewers for D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections.: jhenderson, jakehehrlich.
Jul 25 2018, 7:08 PM

Jul 23 2018

jakehehrlich accepted D49694: [sanitizer] Transition from _zx_vmar_... to _zx_vmar_..._old calls.

LGTM

Jul 23 2018, 5:18 PM
jakehehrlich accepted D49628: [CMake] Link static libunwind and libc++abi into libc++ in Fuchsia toolchain.

LGTM

Jul 23 2018, 5:14 PM

Jul 20 2018

jakehehrlich accepted D49576: [llvm-objcopy] Add basic support for --rename-section.
Jul 20 2018, 11:25 AM
jakehehrlich requested changes to D49576: [llvm-objcopy] Add basic support for --rename-section.

I want to keep the public interface of Object as small as possible but other than that LGTM

Jul 20 2018, 11:05 AM

Jul 19 2018

jakehehrlich accepted D49515: [llvm-objcopy, tests] Fix several llvm-objcopy tests.

The reason for it was just that this all seemed too complicated to me for "something so simple" but now that I've looked into the problem I think what you've done makes perfect sense.

Jul 19 2018, 10:44 AM

Jul 18 2018

jakehehrlich added a comment to D49515: [llvm-objcopy, tests] Fix several llvm-objcopy tests.

What would the consequence be of just converting the bytes to a string and writing that out? I have neither a windows box nor python 3 installed.

Jul 18 2018, 7:04 PM

Jul 17 2018

jakehehrlich added a comment to D49449: [NFC][llvm-objcopy] Cleanup namespace usage in llvm-objcopy..

Uh...I mean.."unobjectionable"...don't judge me.

Jul 17 2018, 1:30 PM
jakehehrlich accepted D49449: [NFC][llvm-objcopy] Cleanup namespace usage in llvm-objcopy..

Seems pretty non-objectionable to me. LGTM

Jul 17 2018, 1:28 PM

Jul 16 2018

jakehehrlich accepted D49400: [llvm-objcopy] Make helper functions static.

Thanks!

Jul 16 2018, 1:56 PM