jakehehrlich (Jake Ehrlich)
User

Projects

User does not belong to any projects.

User Details

User Since
May 16 2017, 11:34 AM (65 w, 5 d)

Recent Activity

Tue, Aug 14

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

Mon, Aug 13

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

Thu, Aug 9

jakehehrlich committed rL339394: Add owner for llvm-objcopy.
Add owner for llvm-objcopy
Thu, Aug 9, 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.

Thu, Aug 9, 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.

Thu, Aug 9, 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.

Thu, Aug 9, 10:53 AM

Wed, Aug 8

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

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

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

LGTM

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

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

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

Mon, Aug 6

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.

Mon, Aug 6, 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.

Mon, Aug 6, 2:33 PM

Fri, Aug 3

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

Thu, Aug 2

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

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

Thu, Aug 2, 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.

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

This LGTM other than one issue

Thu, Aug 2, 11:50 AM

Wed, Aug 1

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.

Wed, Aug 1, 2:48 PM
jakehehrlich added inline comments to D50117: [llvm-objcopy] NFC: Refactor main objcopy method that takes an ELFReader to a generic Reader..
Wed, Aug 1, 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.

Wed, Aug 1, 2:22 PM
jakehehrlich added inline comments to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..
Wed, Aug 1, 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.

Wed, Aug 1, 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.

Wed, Aug 1, 10:44 AM

Tue, Jul 31

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

Tue, Jul 31, 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!

Tue, Jul 31, 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.

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

Can we add test? Otherwise this LGTM.

Tue, Jul 31, 1:22 PM

Mon, Jul 30

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.

Mon, Jul 30, 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.

Mon, Jul 30, 11:36 AM

Fri, Jul 27

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

Forgot to accept

Fri, Jul 27, 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)

Fri, Jul 27, 5:31 PM

Thu, Jul 26

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.

Thu, Jul 26, 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.

Thu, Jul 26, 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.

Thu, Jul 26, 11:22 AM

Wed, Jul 25

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.

Wed, Jul 25, 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.

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

Mon, Jul 23

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

LGTM

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

LGTM

Mon, Jul 23, 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
jakehehrlich committed rL337204: [llvm-objcopy] Add support for large indexes.
[llvm-objcopy] Add support for large indexes
Jul 16 2018, 12:54 PM
jakehehrlich closed D49206: [llvm-objcopy] Add support for large indexes.
Jul 16 2018, 12:54 PM
jakehehrlich added a comment to D49206: [llvm-objcopy] Add support for large indexes.

Nope, I was just about to land it. I wasn't in office Friday and I was conducting an interview this morning. It will be in shortly.

Jul 16 2018, 12:34 PM

Jul 13 2018

jakehehrlich accepted D49331: [CMake] Use libc++ and compiler-rt for sanitizers.
Jul 13 2018, 5:42 PM

Jul 12 2018

jakehehrlich updated the diff for D49206: [llvm-objcopy] Add support for large indexes.

Updated shndx name for this test.

Jul 12 2018, 3:10 PM
jakehehrlich updated the diff for D49206: [llvm-objcopy] Add support for large indexes.

Forgot to update error message for one of James' comments

Jul 12 2018, 3:00 PM
jakehehrlich added inline comments to D49206: [llvm-objcopy] Add support for large indexes.
Jul 12 2018, 3:00 PM
jakehehrlich updated the diff for D49206: [llvm-objcopy] Add support for large indexes.

Now uses python to uncompress.
Responded to comments.

Jul 12 2018, 2:55 PM

Jul 11 2018

jakehehrlich added inline comments to D49206: [llvm-objcopy] Add support for large indexes.
Jul 11 2018, 4:46 PM
jakehehrlich updated the summary of D49206: [llvm-objcopy] Add support for large indexes.
Jul 11 2018, 4:01 PM
jakehehrlich created D49206: [llvm-objcopy] Add support for large indexes.
Jul 11 2018, 4:00 PM

Jun 28 2018

jakehehrlich committed rL335922: [llvm-readobj] Add experimental support for SHT_RELR sections.
[llvm-readobj] Add experimental support for SHT_RELR sections
Jun 28 2018, 2:12 PM
jakehehrlich closed D47919: llvm-readobj: add experimental support for SHT_RELR sections..
Jun 28 2018, 2:12 PM
jakehehrlich added a comment to D47919: llvm-readobj: add experimental support for SHT_RELR sections..

Sure thing.

Jun 28 2018, 1:44 PM
jakehehrlich accepted D48707: [CMake] Disable per-target runtimes for the first stage Fuchsia build.
Jun 28 2018, 1:21 PM
jakehehrlich accepted D47919: llvm-readobj: add experimental support for SHT_RELR sections..

LGTM, sorry for the back and forth!

Jun 28 2018, 1:21 PM

Jun 27 2018

jakehehrlich added inline comments to D47919: llvm-readobj: add experimental support for SHT_RELR sections..
Jun 27 2018, 3:06 PM
jakehehrlich added inline comments to D47919: llvm-readobj: add experimental support for SHT_RELR sections..
Jun 27 2018, 1:51 PM

Jun 26 2018

jakehehrlich added a comment to D47919: llvm-readobj: add experimental support for SHT_RELR sections..

So in considering the case you pointed out about -dyn-relocations I discovered that the current behavior for -relocations is to not print out dynamic relocations at all. This is hidden by the fact that dynamic relocations tend to be duplicated with non-dynamic relocations by default. So we should only print these out on -dyn-relocations unless they're duplicated. As part of my proposal to a solution to this I'm proposing an additional flag so that the raw form that Roland requested can be printed out. This way debugging of the underlying format is possible but for most purposes the decoded form can be used.

Jun 26 2018, 4:13 PM
jakehehrlich added a comment to D47919: llvm-readobj: add experimental support for SHT_RELR sections..

I really feel that non-decoded entries shouldn't have an addend or faked relocation type. It's really confusing to look at those and see relocations. I think non-decoded entries should just print the relr entry.

Jun 26 2018, 12:27 PM

Jun 22 2018

jakehehrlich added a comment to D47919: llvm-readobj: add experimental support for SHT_RELR sections..

I think I'm more or less happy with the llvm-readobj part of this.

Jun 22 2018, 2:54 PM

Jun 11 2018

jakehehrlich added inline comments to D47919: llvm-readobj: add experimental support for SHT_RELR sections..
Jun 11 2018, 7:39 PM
jakehehrlich added a comment to D47919: llvm-readobj: add experimental support for SHT_RELR sections..

I'll have more comments tomorrow. I'm still grokking my way though llvm-readobj as I've never looked at that code before.

Jun 11 2018, 6:41 PM
jakehehrlich added a reviewer for D47919: llvm-readobj: add experimental support for SHT_RELR sections.: jakehehrlich.
Jun 11 2018, 12:41 PM

Jun 7 2018

jakehehrlich added a comment to D47818: [llvm-strip] Expose --strip-unneeded option.

Ah yes. Thanks!

Jun 7 2018, 12:11 AM

Jun 6 2018

jakehehrlich updated subscribers of D47818: [llvm-strip] Expose --strip-unneeded option.

Hey I'm away but I got a bug report requesting -x as an alias in
llvm-strip. I haven't looked at the code for this patch so it might already
be solved here but I'd appreciate it being added. No worries either way.
I'll just add it when aight get back of it isn't there.

Jun 6 2018, 2:16 PM

May 30 2018

jakehehrlich updated subscribers of D47414: [llvm-objcopy] Fix null symbol handling.

Quick drop in. What I was thinking about before was to just never pass the
null symbol to remove or anything else. It can still be there it just
shouldn't be allowed to be updated or removed.

May 30 2018, 10:45 PM

May 25 2018

jakehehrlich accepted D46830: [llvm-objcopy] Add --keep-file-symbols option.
May 25 2018, 12:42 PM
jakehehrlich added a comment to D47229: Make atomic non-member functions as nonnull.

This is causing breaks in fuchsia,

May 25 2018, 9:56 AM

May 24 2018

jakehehrlich accepted D47356: [CMake] Use libc++ and compiler-rt for bootstrap Fuchsia Clang.
May 24 2018, 6:15 PM
jakehehrlich accepted D46896: [llvm-objcopy] Add --strip-unneeded option.

nit: Add comment above loop but otherwise LGTM. Feel free to submit after that.

May 24 2018, 3:14 PM
jakehehrlich added a comment to D46896: [llvm-objcopy] Add --strip-unneeded option.

As always the right solution winds up 10000 times simpler than anything I considered in the interim. This looks good to me after fixing Alex's and my comments.

May 24 2018, 12:34 PM
jakehehrlich closed D47170: [fuchsia] Add line buffering in RawWrite.

Landed in rL333136

May 24 2018, 12:25 PM

May 23 2018

jakehehrlich committed rL333136: [fuchsia] Add line buffering in RawWrite.
[fuchsia] Add line buffering in RawWrite
May 23 2018, 3:31 PM
jakehehrlich committed rCRT333136: [fuchsia] Add line buffering in RawWrite.
[fuchsia] Add line buffering in RawWrite
May 23 2018, 3:31 PM
jakehehrlich updated the diff for D47170: [fuchsia] Add line buffering in RawWrite.

Switched to use index. Fixed to print out full buffer in case no newline exists.

May 23 2018, 3:16 PM

May 22 2018

jakehehrlich accepted D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type.
May 22 2018, 3:59 PM · Restricted Project
jakehehrlich updated the diff for D47170: [fuchsia] Add line buffering in RawWrite.

Modified to group multiple lines into a single flush but still flush at least once if a newline is found.

May 22 2018, 2:50 PM
jakehehrlich accepted D47222: [llvm-strip] Expose --keep-symbol option.

LGTM

May 22 2018, 1:31 PM
jakehehrlich added a comment to D46896: [llvm-objcopy] Add --strip-unneeded option.

Yeah I agree with Alex and James here. I think I'd prefer some sort of interface extension. Something as simple as 'markSymbols' that's done as a loop though the sections which then just sets a Boolean like you had originally. That should a) give the desired time complexity and b) not be super complicated. That loop can also only be triggered if --strip-uneeded is used which is nice.

May 22 2018, 11:43 AM
jakehehrlich updated subscribers of D46896: [llvm-objcopy] Add --strip-unneeded option.

Just a heads up I'm not convinced my shared_ptr idea is ok. I want to
consider what Alex and James are saying. My hope has always been to keep
the symbol table as the owner so if Alex has a proposal I want to read it
first.

May 22 2018, 9:50 AM

May 21 2018

jakehehrlich added a comment to D46896: [llvm-objcopy] Add --strip-unneeded option.

I didn't think about that. I think the key thing here then is to use shared_ptr to store all symbols and then use use_count to keep track of the references.

May 21 2018, 6:30 PM
jakehehrlich accepted D47052: [llvm-objcopy] Fix the behavior of --strip-* and --keep-symbol.

LGTM but please wait on james

May 21 2018, 5:40 PM
jakehehrlich created D47170: [fuchsia] Add line buffering in RawWrite.
May 21 2018, 5:36 PM
jakehehrlich added a comment to D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type.

LGTM with a couple additions.

May 21 2018, 3:36 PM · Restricted Project
jakehehrlich added a comment to D46791: Make -gsplit-dwarf generally available.

Yeah I haven't heard back from Lexan nor have I tried reproducing such a build myself and if we're that close to not relaying on this sort of hack I'd much rather wait on this than risk breaking someone for the sake of a stopgap for such a short period of time. @echristo Can you link the review to that?

May 21 2018, 3:21 PM
jakehehrlich added a comment to D47052: [llvm-objcopy] Fix the behavior of --strip-* and --keep-symbol.

so basically we can implement this option without trying to repeat binutils behavior,
what would you say to this ? if so I can update the patch (and actually the code will be simpler)

I'm of the opinion that if the GNU behaviour doesn't make sense, we shouldn't follow it blindly, so I'd be happy in a divergence here, but I'm not a regular GNU tools user, so it might be wise to get @jakehehrlich's thoughts on this before proceeding.

May 21 2018, 3:17 PM