Page MenuHomePhabricator

jakehehrlich (Jake Ehrlich)
User

Projects

User does not belong to any projects.

User Details

User Since
May 16 2017, 11:34 AM (108 w, 6 d)

Recent Activity

Fri, Jun 14

jakehehrlich added inline comments to D61672: [llvm-objcopy] Allow strip symtab in executables and DSOs.
Fri, Jun 14, 1:57 PM
jakehehrlich added a comment to D63104: Add GSYM utility files along with unit tests..

A bunch of nits but this looks good to me.

Fri, Jun 14, 12:44 PM · Restricted Project

Thu, Jun 13

jakehehrlich accepted D63238: [llvm-objcopy] Add elf32-sparc and elf32-sparcel target.
Thu, Jun 13, 5:23 PM · Restricted Project

Wed, Jun 12

jakehehrlich added a comment to D63104: Add GSYM utility files along with unit tests..

Sorry I wasn't very quick to get to this. This looks really good to me. I'm going to go over some of the this with a fine tooth comb tomorrow but It basically looks good as at high level glance.

Wed, Jun 12, 5:40 PM · Restricted Project

Mon, Jun 10

jakehehrlich added inline comments to D61767: [llvm-elfabi] Emit ELF header and string table section.
Mon, Jun 10, 1:04 PM · Restricted Project
jakehehrlich updated the diff for D61767: [llvm-elfabi] Emit ELF header and string table section.

Addressed Armando's comments

Mon, Jun 10, 12:59 PM · Restricted Project

Thu, Jun 6

jakehehrlich added a comment to D62970: [clang-doc] De-duplicate comments and locations.

Actually if we can make Description a hash set that would be best. I think using std::set would require an awkward < operator to be defined. There should be standard ways of getting hashes out of all the string and vector types and you can use hash_combine to combine all of these into a single hash.

Thu, Jun 6, 2:48 PM · Restricted Project
jakehehrlich added inline comments to D62970: [clang-doc] De-duplicate comments and locations.
Thu, Jun 6, 2:32 PM · Restricted Project

Wed, Jun 5

jakehehrlich added a comment to D53379: GSYM symbolication format.

This is just where I got tired today but I think I can recommend how to split this up so I could move faster and provide more useful high level review. Prior to splitting I'll keep chugging away for at least a bit each day.

Wed, Jun 5, 5:33 PM
jakehehrlich accepted D62898: [llvm-objcopy] - Emit error and don't crash if program header reaches past end of file..

"goes past the end of the file" is the phrasing that I've seen GNU tools use and gets more to the point so I agree with James. Other than that this looks good to me!

Wed, Jun 5, 3:24 PM · Restricted Project

Tue, Jun 4

jakehehrlich added a reviewer for D61767: [llvm-elfabi] Emit ELF header and string table section: smeenai.
Tue, Jun 4, 7:48 PM · Restricted Project
jakehehrlich added a comment to D62364: llvm-objcopy: Implement --extract-partition and --extract-main-partition..

I'm happy with this as is but I'd like @jhenderson, @rupprecht, and @alexshap to take a look since this represents a fairly major feature addition even if the code is pretty small. I don't believe this imposes any extra assumptions or requirements on the rest of the code however.

Tue, Jun 4, 2:22 PM · Restricted Project
jakehehrlich accepted D62817: [test][llvm-objcopy] Test llvm-objcopy with standard streams.

Neat! I didn't even know this was possible TBH

Tue, Jun 4, 12:30 AM · Restricted Project

Fri, May 31

jakehehrlich accepted D62655: [llvm-readobj/llvm-readelf] - Remove gnu-relocations.test completely..
Fri, May 31, 3:13 PM · Restricted Project
jakehehrlich added a comment to D62711: [MACHO] Replaced calls to getStruct with getStructOrErr in functions returning Error or Expected or similar.

It would be nice to get tests for these cases, although I realize that can be difficult as it requires malformed inputs.

Fri, May 31, 3:13 PM · Restricted Project
jakehehrlich accepted D62711: [MACHO] Replaced calls to getStruct with getStructOrErr in functions returning Error or Expected or similar.

Might label as NFC.

Fri, May 31, 3:07 PM · Restricted Project
jakehehrlich added inline comments to D62718: [llvm-objcopy] Change handling of output file permissions.
Fri, May 31, 3:06 PM · Restricted Project
jakehehrlich accepted D62718: [llvm-objcopy] Change handling of output file permissions.

This looks good to me but please have someone else accept first as well.

Fri, May 31, 3:03 PM · Restricted Project
jakehehrlich added inline comments to D62364: llvm-objcopy: Implement --extract-partition and --extract-main-partition..
Fri, May 31, 2:58 PM · Restricted Project
jakehehrlich added a comment to D62364: llvm-objcopy: Implement --extract-partition and --extract-main-partition..

Ok I spoke offline about some of my questions with Peter. Here are some useful facts:

Fri, May 31, 2:58 PM · Restricted Project

Thu, May 30

jakehehrlich added a comment to D62620: [llvm-objcopy] Fix SHT_GROUP ordering..

Alternatively we could only sort program headers in the layout algorithm and then just layout sections in whatever order we please. It will just mean that our layout won't mirror the input but it will be valid.

Thu, May 30, 11:42 AM · Restricted Project
jakehehrlich added a comment to D62620: [llvm-objcopy] Fix SHT_GROUP ordering..

Alternatively we can sort by Index to allow ourselves to make mistakes in maintaining the order along the way but I think I'd rather not do that since that's just a hack.

Thu, May 30, 11:41 AM · Restricted Project
jakehehrlich added a comment to D62620: [llvm-objcopy] Fix SHT_GROUP ordering..

Can we just sort in the layout code instead of sorting everything? Then we don't have to sort at all. I think we've largely been good about making operations stable so order should be maintained if just don't sort.

Thu, May 30, 11:41 AM · Restricted Project

Wed, May 29

jakehehrlich accepted D62578: [llvm-objcopy][MachO] Print an error message on use of unsupported options.
Wed, May 29, 2:09 PM · Restricted Project
jakehehrlich added a comment to D62364: llvm-objcopy: Implement --extract-partition and --extract-main-partition..

A few questions

Wed, May 29, 12:42 PM · Restricted Project
jakehehrlich added a comment to D62594: [llvm-readobj] - Rewrite reloc-types.test to use YAML. NFCI..

If it isn't too much work I'd prefer these be split up as well. Otherwise this looks like a strict improvement to me!

Wed, May 29, 12:20 PM · Restricted Project

Thu, May 23

jakehehrlich accepted D58426: llvm-objcopy: Change sectionWithinSegment() to use virtual addresses instead of file offsets for SHT_NOBITS sections..

I think this is good. The current behavior treats sectionWithinSegment as not mattering in this case because this predicate is used for getting file offsets right and ignores the SHT_NOBITS case. This is a strict improvement that just sets the predicate in such a way that llvm-objcopy will output the conventional offset now.

Thu, May 23, 5:15 PM · Restricted Project

Wed, May 22

jakehehrlich accepted D62260: [llvm-objdump] - Many minor NFC changes to cleanup/improve the code in ELF/Object.cpp..

Solid improvement. LGTM.

Wed, May 22, 11:29 AM · Restricted Project

Tue, May 21

jakehehrlich accepted D62072: [llvm-objcopy] Tidy up error messages.
Tue, May 21, 8:16 PM · Restricted Project
jakehehrlich added a comment to D62072: [llvm-objcopy] Tidy up error messages.

Thanks for fixing all of my mistakes!

Tue, May 21, 8:15 PM · Restricted Project

Mon, May 20

jakehehrlich accepted D61993: [llvm-objcopy] Add file names to error messages.

Awesome. LGTM. Tell me if you want me or someone else to land it. You can also email Chris Latner to get access yourself.

Mon, May 20, 12:49 PM · Restricted Project

May 16 2019

jakehehrlich added inline comments to D61767: [llvm-elfabi] Emit ELF header and string table section.
May 16 2019, 1:27 PM · Restricted Project
jakehehrlich added a comment to D61672: [llvm-objcopy] Allow strip symtab in executables and DSOs.

I'm still confused. If the behavior intended only has to do with the existence of relocations why are we making this change? A clear difference of behavior needs to be pointed out and that hasn't been made clear.

May 16 2019, 1:27 PM
jakehehrlich added a comment to D61937: [llvm-readelf] - Rework how we parse the .dynamic section..

I have the same question as James with respect to requiring PT_DYNAMIC but otherwise the code looks good to me. I'm still looking at the tests.

May 16 2019, 1:17 PM · Restricted Project
jakehehrlich added a comment to D61969: llvm-objdump:Fix Bugzilla ID 41862 to support checking addresses of disassembled object.

Also what does GNU objdump do in this case?

May 16 2019, 1:12 PM · Restricted Project
jakehehrlich added a comment to D61969: llvm-objdump:Fix Bugzilla ID 41862 to support checking addresses of disassembled object.

When uploading changes make sure to upload the whole context. If using git diff use git diff -U100000 or some other large number to get all of the surrounding context.

May 16 2019, 1:09 PM · Restricted Project
jakehehrlich added reviewers for D61969: llvm-objdump:Fix Bugzilla ID 41862 to support checking addresses of disassembled object: rupprecht, echristo, jhenderson, grimar.
May 16 2019, 1:06 PM · Restricted Project
jakehehrlich added a comment to D61969: llvm-objdump:Fix Bugzilla ID 41862 to support checking addresses of disassembled object.

link to the bug for the lazy: https://bugs.llvm.org/show_bug.cgi?id=41862

May 16 2019, 1:06 PM · Restricted Project

May 13 2019

jakehehrlich added a comment to D61672: [llvm-objcopy] Allow strip symtab in executables and DSOs.

I'm still a bit unclear. Does it strip symbols that are duplicates of symbols in .dynsym? Does it just get rid of the the whole symbol table no matter what?

May 13 2019, 1:47 PM

May 10 2019

jakehehrlich added a comment to D60974: Clang IFSO driver action..

This shouldn't emit the .tbe format at all, the .tbe format is meant to be in near 1-1 correspondence with .so files, not with .o files. It has things like DT_NEEDED, and DT_SONAME related information that don't make sense for .o files for instance.

May 10 2019, 10:25 AM · Restricted Project, Restricted Project

May 9 2019

jakehehrlich added a comment to D55864: [elfabi] Write program headers, .dynamic, .dynstr, and .shstrtab.

The following is the first split. Subsequent splits will be much smaller (adding support for program headers, adding support for soname and needed, adding support for symbols, etc...): https://reviews.llvm.org/D61767#change-LCioCf1j7osU

May 9 2019, 3:55 PM · Restricted Project
jakehehrlich created D61767: [llvm-elfabi] Emit ELF header and string table section.
May 9 2019, 3:55 PM · Restricted Project
jakehehrlich added a comment to D61672: [llvm-objcopy] Allow strip symtab in executables and DSOs.

I'm confused as to what this is trying to accomplish. This really just marks every symbol as unneeded in a DSO and keeps the behavior for other cases. In that case why not strip the symbol table all togethor? This is a very round about way to accomplish that.

May 9 2019, 11:00 AM

May 6 2019

jakehehrlich added a comment to D61510: [SanitizerCoverage] Use different module ctor names for trace-pc-guard and inline-8bit-counters.

Can you explain how this occurred a bit more and how this is solving the issue?

May 6 2019, 11:02 AM · Restricted Project

May 3 2019

jakehehrlich updated the diff for D55864: [elfabi] Write program headers, .dynamic, .dynstr, and .shstrtab.
  1. No more TLS stuff but STT_TLS symbol types are preserved which appears to be sufficient for all known use cases
  2. Fixed all the tests, accidental extra tests, minor issues, and made all tests pass.
May 3 2019, 4:44 PM · Restricted Project
jakehehrlich added a comment to D55864: [elfabi] Write program headers, .dynamic, .dynstr, and .shstrtab.

@MuskRay Have you checked other linkers or tools?

May 3 2019, 3:11 PM · Restricted Project
jakehehrlich updated the diff for D55864: [elfabi] Write program headers, .dynamic, .dynstr, and .shstrtab.

Ok so I went digging on the PT_TLS and tls section issues. Seems like removing both the segment and the section is the right way to go.

May 3 2019, 3:11 PM · Restricted Project
jakehehrlich added a comment to D55864: [elfabi] Write program headers, .dynamic, .dynstr, and .shstrtab.

Is llvm-elfabi used to create interface shared objects? Do you use it to create real(runnable) modules?

If not and if section/segment specification is not required, it can be very simple.
You just need the symbol table and very little other information.
There is one place in lld that checks if the symbol is in a readonly segment (addCopyRelSymbol).
1 PT_LOAD(RWX) + 1 PT_GNU_RELRO works just fine. You don't even need PT_TLS - STT_TLS symbols can be placed in an arbitrary section. Linkers and various binutils don't validate PT_TLS.

May 3 2019, 1:07 PM · Restricted Project
jakehehrlich abandoned D59531: [ELF] Produce multiple PT_NOTE segments as needed.
May 3 2019, 12:43 PM · Restricted Project

May 2 2019

jakehehrlich accepted D61296: [ELF] Place SHT_NOTE sections with the same alignment into one PT_NOTE.

Ok so I'm good with this. We would like to land this ASAP.

May 2 2019, 2:12 PM · Restricted Project
jakehehrlich added a comment to D60974: Clang IFSO driver action..

Jake, I am still not sure what you would prefer for moving forward. The current patch can output the yaml format I originally proposed or the one that looks similar to the one you proposed. What I need to know is:

Do you want the merging if the "ifo" files to happen inside of llvm-elfabi?
Do you care if we upstream the yaml we proposed as a first step (which is really a distilled version of that yaml2obj can consume anyways. this right now functions actually) ???
Or, would you rather the ifo files be merged by a different separate tool (something maybe called llvm-ifsogen)???

May 2 2019, 1:55 PM · Restricted Project, Restricted Project
jakehehrlich added inline comments to D55864: [elfabi] Write program headers, .dynamic, .dynstr, and .shstrtab.
May 2 2019, 1:37 PM · Restricted Project

May 1 2019

jakehehrlich updated the diff for D55864: [elfabi] Write program headers, .dynamic, .dynstr, and .shstrtab.

Ok we're getting closer

May 1 2019, 8:11 PM · Restricted Project
jakehehrlich added a comment to D55864: [elfabi] Write program headers, .dynamic, .dynstr, and .shstrtab.

I can't upload a new patch right now but I plan on making all recommended changes, even the ones I didn't reply to.

May 1 2019, 1:36 PM · Restricted Project
jakehehrlich accepted D61343: [llvm-objcopy] Cache gnu_debuglink's target CRC.

This code looks good to me and the use case seems valid. I'll give my LGTM but please wait for others.

May 1 2019, 1:04 PM · Restricted Project
jakehehrlich accepted D61377: [llvm-strip]Add --no-strip-all to disable --strip-all behaviour (including default stripping).

Good idea! The code looks good as well as testing.

May 1 2019, 12:53 PM · Restricted Project
jakehehrlich added a comment to D60974: Clang IFSO driver action..

@jakehehrlich - unfortunately, removing the attributes on the sections will make the content look different with nm which is something we do need to appear proper for consumers of the interface libraries. Versioning has a number of problems inherent to it (unfortunately, its the closest to multi-level namespaces in ELF). I think that getting something in tree and iterating on it is far better than continuing to argue over this in the dream of something that unifies the TAPI approach and this approach. The section names are relevant (you can add attributes to put symbols into alternate sections and you can have section relative relocations). I think that you are loosing fidelity in the final output which is sufficient for your needs, but I think that there are places where the full fidelity can be needed.

This currently works and allows us to generate the interface library which means that this is actually further than what you are proposing still. Is there something technical that this is doing incorrectly or breaking something? Otherwise, this really does seem like it is devolving into a bike shedding argument that isn't really going anywhere. This is not a large amount of code and there is backing to maintain it, so it is not an issue of "this is adding un-maintained complexity" either.

Just like the LLVM APIs, this can/will evolve. I don't see why this needs to be set in stone from the initial implementation. There are use cases which can come up which require reworking the solution.

May 1 2019, 12:48 PM · Restricted Project, Restricted Project

Apr 30 2019

jakehehrlich added a comment to D55864: [elfabi] Write program headers, .dynamic, .dynstr, and .shstrtab.

Oh and this is the big picture. I'll be splitting this up somehow.

Apr 30 2019, 7:19 PM · Restricted Project
jakehehrlich updated the diff for D55864: [elfabi] Write program headers, .dynamic, .dynstr, and .shstrtab.

Ok so this is the rough idea that I have for doing this. I need to add program headers which turns out to require refactoring this a bit. The general idea takes some explanation. There are multiple ways to think about it however. The goal is for everything to "work itself out" automagically. I'll need to add program headers as many bits of functionality are not easily testable here unless we add them. You'll note that I put the NOBITS sections before the other sections. This doesn't jive well with using a single program header to cover them all and since space is a thing we'd like to constrain here we should support it. Unfortunately I did that as optimization as a means of avoiding the dynamic symbol table needing know the addresses of the NOBITS sections before starting. This is overcomeable using an additional loop however. I think an additional loop would be needed even if you hand rolled however and by adding this loop we can probably avoid maintaining an intermediate vector for the dynamic symbol table. Instead we can compute the overall size first, allocate the FileOutputBuffer, and then write directly into it. I'm not sure the complexity or redundancy is justified by the memory savings and I think the time savings will be pretty minimal.

Apr 30 2019, 7:16 PM · Restricted Project
jakehehrlich added a comment to D60974: Clang IFSO driver action..

Wanted to mention, from my testing with yaml2obj, I needed the section flags (SHF_EXECINSTR, SHF_WRITE, SHF_ALLOC, etc) otherwise nm was spitting the wrong thing. How are you going to handle those?

Apr 30 2019, 6:14 PM · Restricted Project, Restricted Project
jakehehrlich added a comment to D60974: Clang IFSO driver action..

Oh and "ifo" is a good name.

Apr 30 2019, 6:14 PM · Restricted Project, Restricted Project
jakehehrlich added a comment to D60974: Clang IFSO driver action..

The linker doesn't look at section permissions. In fact the only thing it even looks at the section index for is to check for alignment for copy relocations which is something most people don't even use.

Apr 30 2019, 6:09 PM · Restricted Project, Restricted Project
jakehehrlich added a comment to D60974: Clang IFSO driver action..

Well I think it should be a seperate tool but because the code inside llvm-elfabi isn't a library yet (though it is written to be used like one) we can do that by adding a symlink and changing the behavior of the underlying binary to present as a different tool. Many different tools around llvm do this for similar reasons.

Apr 30 2019, 5:27 PM · Restricted Project, Restricted Project
jakehehrlich accepted D60042: [llvm-objcopy] Add --prefix-alloc-sections.

LGTM

Apr 30 2019, 3:56 PM · Restricted Project
jakehehrlich added a comment to D60974: Clang IFSO driver action..

Background:
There are two parts to this. TextAPI is a public interface of textual representations for these sorts of files that already exists in llvm. There exist ELF and MachO textual representations currently. llvm-elfabi is a tool for 1) Creating Stubs 2) Creating .tbe (the TextAPI fully linked ELF format) files 3) Converting between them and 4) Analyzing the details of the formats.

Apr 30 2019, 3:46 PM · Restricted Project, Restricted Project
jakehehrlich added a comment to D61296: [ELF] Place SHT_NOTE sections with the same alignment into one PT_NOTE.

If multiple note sections with unaligned sizes but larger than 4-byte alignment are used this solution doesn't work.

Apr 30 2019, 11:27 AM · Restricted Project
jakehehrlich added a comment to D60974: Clang IFSO driver action..

Does llvm-elfabi consume your proposed Schema format? Has it landed yet?

Apr 30 2019, 11:19 AM · Restricted Project, Restricted Project

Apr 29 2019

jakehehrlich added a comment to D60974: Clang IFSO driver action..

With respect to group handeling. I think we should just check that the details of every symbol across all unmerged files that are being merged are consistent and then ignore duplicates.

Apr 29 2019, 5:49 PM · Restricted Project, Restricted Project
jakehehrlich added a comment to D60974: Clang IFSO driver action..

Me and @compnerd had discussed a more abstracted format like this but decided it was best to just use the same names that are in the ELF already.
Is there any compelling reason not to?
As far as I understand, by having something like "Weak: true" is already harking back to ELF so why not stick to the same names?

I think the !tbd-elf-v1 versioning can help with any changes or alterations we want to make along the way too.
We did discuss the alignment field too.

The format will have to be ELF specific but that doesn't mean we have to use the exact names. The benefit of this format is that you can only do the intended thing with it while anything more. This is also the format that matches most closely with .tbe which is a plus for consistency of this and integration of both tools into the llvm ecosystem. It's obvious how to merge my format into the ELFStub format. Your format has extraneous details that would never matter to the creation of the ELFStub format like the name of the section a symbol was defined in. Also I think much more of the compiler has to be considered to get section names right unless you're just recomputing them and then that's redundant for no gain.

We wanted to use the same names because its just a lot easier understand what is if you've already looked at the ELF header code (ie STT_FUNC vs Function).

Apr 29 2019, 5:44 PM · Restricted Project, Restricted Project
jakehehrlich added a comment to D60974: Clang IFSO driver action..

Also, curious on the lack of denoting the sections and which symbols go in which sections? Do you just assume that "Type: Object" goes into .data?

Apr 29 2019, 5:43 PM · Restricted Project, Restricted Project
jakehehrlich added inline comments to D60974: Clang IFSO driver action..
Apr 29 2019, 4:14 PM · Restricted Project, Restricted Project
jakehehrlich added a comment to D60974: Clang IFSO driver action..

I think the !tbd-elf-v1 versioning can help with any changes or alterations we want to make along the way too.

Apr 29 2019, 4:07 PM · Restricted Project, Restricted Project
jakehehrlich added a comment to D60974: Clang IFSO driver action..

Me and @compnerd had discussed a more abstracted format like this but decided it was best to just use the same names that are in the ELF already.
Is there any compelling reason not to?
As far as I understand, by having something like "Weak: true" is already harking back to ELF so why not stick to the same names?

I think the !tbd-elf-v1 versioning can help with any changes or alterations we want to make along the way too.
We did discuss the alignment field too.

Apr 29 2019, 4:06 PM · Restricted Project, Restricted Project
jakehehrlich added a comment to D60974: Clang IFSO driver action..

To clarify my format is for the unmerged format. The merged format should be the ELFStub internal representation found in llvm's TextAPI. The plan is to have at minimum .tbe and ELF stubs. I'm actively working on getting the ELF stub output which can then be converted to the yaml2obj format since they will just be ELF files.

Apr 29 2019, 3:33 PM · Restricted Project, Restricted Project
jakehehrlich added a comment to D60974: Clang IFSO driver action..

So, @compnerd and I have discussed and we'd like to propose a yaml schema that can express what we need for ifsos for the Sections and the Symbols.

...

Apr 29 2019, 3:33 PM · Restricted Project, Restricted Project
jakehehrlich added a comment to D60974: Clang IFSO driver action..

I think I know the format that I'd like *roughly*. I haven't worked out how to handle COMDAT and section groups yet. Roland seems to think that COMDAT and section groups will be an issue but I'm working out the details with him to make sure I understand it. I'll get back to you soon.

Apr 29 2019, 3:33 PM · Restricted Project, Restricted Project
jakehehrlich updated subscribers of D61272: [llvm-objcopy] Add RISC-V support for -B/-O.

I recall @ruiu telling me that mips had a really odd endianness that wasn't accounted for by the standard ELFT types. Is that still true?

Apr 29 2019, 12:09 PM · Restricted Project

Apr 26 2019

jakehehrlich commandeered D55864: [elfabi] Write program headers, .dynamic, .dynstr, and .shstrtab.

Gonna take this one too.

Apr 26 2019, 5:00 PM · Restricted Project
jakehehrlich updated the diff for D55839: [elfabi] Add support for writing ELF header for binary stubs.
Apr 26 2019, 4:41 PM · Restricted Project
jakehehrlich commandeered D55839: [elfabi] Add support for writing ELF header for binary stubs.

I'm picking this up. Looking for review on this again. I'm going to drive this to completion enough to use on Fuchsia and then maintain it. After it has the features we want for Fuchsia I'll work on adding features that other people want but at a slower pace (like I did after a while with llvm-objcopy) eventually I'll ramp down but honestly, unlike llvm-objcopy, I could see this tool stabilizing.

Apr 26 2019, 4:38 PM · Restricted Project
jakehehrlich added a comment to D59531: [ELF] Produce multiple PT_NOTE segments as needed.

@ruiu and others. I'm still good to land this as is. Are you ok with me landing this?

Apr 26 2019, 2:00 PM · Restricted Project
jakehehrlich added a comment to D59531: [ELF] Produce multiple PT_NOTE segments as needed.

The larger alignment isn't being set by mistake there's just an odd note that's being added to linux binaries right now and its causing this sort of headache.

Apr 26 2019, 1:45 PM · Restricted Project

Apr 25 2019

jakehehrlich added a comment to D60974: Clang IFSO driver action..

Why would you convert the actual format, which is already using YAML, to the yaml2obj format? That seems to have zero gain to me.

Apr 25 2019, 5:44 PM · Restricted Project, Restricted Project
jakehehrlich added a comment to D61092: [llvm-strip] Have --discard-all imply --strip-debug.

This appears to be correct. StripDebug is a preety simple and minimal field so we can go ahead and add this. The keep symbol thing still looks a bit funny.

Apr 25 2019, 2:47 PM · Restricted Project
jakehehrlich added a comment to D60439: [llvm-objcopy] Accept --long-option but not -long-option.

It would be helpful to convert all of the listed tools over as well for consistency

Apr 25 2019, 2:26 PM · Restricted Project
jakehehrlich accepted D60439: [llvm-objcopy] Accept --long-option but not -long-option.

Eh I guess this is fine. I'm kind of 50/50 on this myself. I don't think you'll break any major team or code base I know about but I could be wrong. I don't really like fragmenting the standard used across these tools but grouped options increases compatibility so I think on balance I support this.

Apr 25 2019, 2:26 PM · Restricted Project
jakehehrlich added a comment to D60974: Clang IFSO driver action..

As an example, .tbe and .tbd files use YAML but in a very specific well defined structure. yaml2obj also uses a specific format which is actually well defined, it just doesn't map on very well here.

Apr 25 2019, 2:22 PM · Restricted Project, Restricted Project
jakehehrlich added a comment to D60974: Clang IFSO driver action..

There seems to be some confusion about the term "format" here. There's the YAML format but within that there are specific YAML formats. My proposal will be to have a specific YAML format that just isn't the one used by yaml2obj. YAML isn't the issue, it's the structure that that tool specifically uses.

Apr 25 2019, 2:19 PM · Restricted Project, Restricted Project
jakehehrlich added inline comments to D60042: [llvm-objcopy] Add --prefix-alloc-sections.
Apr 25 2019, 2:17 PM · Restricted Project
jakehehrlich added a comment to D60502: [llvm-nm] Add --special-syms.

Just to see if I understand the comments so far: --special-syms is essentially already the default for llvm-nm (i.e. we print special symbols), but not for GNU nm. And so even though we print special symbols, a configure script that checks if $NM --special-syms is supported will fail when it shouldn't.

If so, then LGTM -- this patch makes things less worse in all regards.

There's a counter-example to this though, which I'm concerned about. If we silently accept --special-syms, with it being the default always, there could be configure scripts that don't specify --special-syms that expect to NOT see those symbols (e.g. they're looking for all "normal" symbols). This in turn could result in a silent breakage.

Apr 25 2019, 2:15 PM · Restricted Project
jakehehrlich added a comment to D60825: [llvm-objcopy] - Check dynamic relocation sections for broken references..

Maybe we should land this and wait for someone to have that issue. Can someone file a bug about that issue now?

Apr 25 2019, 2:03 PM · Restricted Project
jakehehrlich added a comment to D60825: [llvm-objcopy] - Check dynamic relocation sections for broken references..

Hmm...right now we don't parse the dynamic symbol table but parsing it just enough to check if no symbols are needed and then not throwing an error if no symbol table is needed seems like a better option than using that flag. That flag should of course also be respected here however.

Apr 25 2019, 2:03 PM · Restricted Project

Apr 24 2019

jakehehrlich added a comment to D60974: Clang IFSO driver action..

@jakehehrlich - when do you expect to have your idea put up? I don't think that it is fair to have this wait until you have time to put something up that can be discussed. I think that getting this working and then iterating on it and migrating it over to some shared representation is something which we could do - that tends to be a common thing that I have seen happen multiple times with the necessary work never materialising. Re-use of the YAML structure means that we can iterate and identify the pieces that are necessary, though, I expect that largely, what will be needed is the name, the binding, the visibility, possibly the size (for TBEs), the section, and the type, at least for anything which adheres to the GABI. If you have extensions outside of GABI, this will need to be adjusted.

Apr 24 2019, 3:05 PM · Restricted Project, Restricted Project
jakehehrlich added a comment to D60974: Clang IFSO driver action..

To be clear I'm only really giving my thumbs up to a specific high level plan that I believe handles a case that llvm-elfabi wouldn't otherwise handle on its own. I consider the specifics like the output format still in the air right now. I strongly believe that a unique yaml or json format should be used for this. We should discuss the specifics of what should go in that format and have a discussion on that format. The trick is to capture everything in .o files that could wind up mattering to a .tbe file and nothing more. I haven't thought though what that entails completely yet but I'll think about it and throw up my proposal. Once we have at least one proposal we can start iterating on the specifics of it.

Apr 24 2019, 2:16 PM · Restricted Project, Restricted Project

Apr 23 2019

jakehehrlich added a comment to D60974: Clang IFSO driver action..

The strong motivation here is to avoid having the list be maintained outside of the source code. I am not too strongly tied to the YAML approach as long as the binaries that we get at the end are equivalent to the pruned ELF binaries from the YAML2ELF approach. However, the trade off here is valuable - it allows for the source code to be the source of truth for the interface set. The intermediate emission is due to the fact that this requires program level visibility and having the intermediates an extra step to merge is relatively simpler. In fact, you could emit the TBE files from this and check that into the tree if you really wanted (but allowing the source to remain the source of truth I think is a valid and valuable trade off). The reason for the selection of YAML is simply pragmatism - that is already available in the tree. If the TBE linker is available now, I have no issue with that being the intermediate representation (assuming that the representation is easy to parse programatically).

Apr 23 2019, 3:00 PM · Restricted Project, Restricted Project
jakehehrlich added a comment to D60974: Clang IFSO driver action..

I actually don't see why something like this couldn't be used in both scenarios (ie some users generating the files at build time, and others using -emit-ifso to periodically generate and land changes to checked in files).

Apr 23 2019, 1:55 PM · Restricted Project, Restricted Project
jakehehrlich added a comment to D60974: Clang IFSO driver action..

Why is this better than producing output from the output of the static linker at the end? Also how do we plan on integrating this with llvm-elfabi so that we don't have unnecessarily duplicated efforts? Why does that tool not suffice?

Apr 23 2019, 1:12 PM · Restricted Project, Restricted Project

Apr 22 2019

jakehehrlich added a comment to D60974: Clang IFSO driver action..

I'm not misunderstanding, please seem my post on llvm-dev where I layout the different issues involved. You're underestimating the complexities involved here I think. Trivializing what the linker does to "the linker just merges these" is a gross oversimplification that doesn't account for symbol consuming vs exporting, size, alignment, visibility, linkage, symbol type, symbol versioning (which is impossible to handle here without consuming version scripts as was pointed out by someone else), which symbols are untimely exported/consumed or even intermediate operations on .o files via tools like objcopy (which is uncommon and shouldn't be a hard blocker here but something to think about). Also all linkers behave just a bit differently with respect to these issues so 1) the reality is that the thing that merges these will behave just a bit differently than any existing linker but 2) even if it trys to match one linker it will fail to match the other so it will only be compaitable in all cases with atmost one linker (but probably none when edge cases are considered. I also already pointed out the issue with having to search all source files on the tree. I agree that if you were to use the linker to accomplish this that would would have to look at all source files. Further more many flags passed to the linker untimely affect these things making matters more complicated. The "linker" you describe for these cases is far from "cat".

Apr 22 2019, 7:11 PM · Restricted Project, Restricted Project
jakehehrlich added a comment to D60974: Clang IFSO driver action..

There are a few that I have in mind.

  1. -emit-ifso could be added to a build to produce .so files as part of a device SDK (where we don't want to ship the runnable bits in the SDK, we ship those on the device updates).
Apr 22 2019, 3:40 PM · Restricted Project, Restricted Project
jakehehrlich added a comment to D60270: [llvm-objcopy] Add support for Intel HEX output format.

There's a lot of code to review here. I'll keep reviewing it everyday but this is going to take a while to review. Any help on splitting this up and making into smaller chunks would be helpful. Splitting reading and writing up into two separate patches would be helpful and removing features that we can add later would be helpful.

Apr 22 2019, 1:16 PM · Restricted Project