Page MenuHomePhabricator

plotfi (Puyan Lotfi)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 30 2017, 10:52 AM (76 w, 5 d)

Recent Activity

Yesterday

plotfi updated the diff for D60974: Clang IFSO driver action..

fixing support for static functions.

Tue, May 21, 3:53 PM · Restricted Project, Restricted Project

Mon, May 20

plotfi updated the diff for D60974: Clang IFSO driver action..

adding a nice inline test

Mon, May 20, 8:33 PM · Restricted Project, Restricted Project

Fri, May 17

plotfi added a comment to D62032: build: use clang-cl for runtimes when targeting Windows.

Note: I think we should probably do something about the linker here too. so use lld-link instead of ld.lld in the Windows case.

Fri, May 17, 12:36 PM · Restricted Project
plotfi added a comment to D62034: compiler-rt/lib/builtins: s/#include <windows.h>/#include <Windows.h>/g to match proper case..

I have no objection to this; but I'm not really confident that this won't break more than it solves.

Fri, May 17, 11:34 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project
plotfi added a comment to D62034: compiler-rt/lib/builtins: s/#include <windows.h>/#include <Windows.h>/g to match proper case..

Yes, MinGW headers are all lowercase.

And the Windows SDK, where the file is spelled with a capital W, isn't self-consistent (some headers include others with a different capitalization than the files actually are stored with), so to use the official Windows SDK on a case sensitive file system, you need some sort of case fixup anyway. (Or did that change very recently?) And in that case, making it all lowercase is the simplest, which also happens to match the MinGW headers.

Fri, May 17, 11:17 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project
plotfi updated the diff for D62034: compiler-rt/lib/builtins: s/#include <windows.h>/#include <Windows.h>/g to match proper case..

adding all windows.h's in all of monorepo

Fri, May 17, 11:12 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project

Thu, May 16

plotfi created D62034: compiler-rt/lib/builtins: s/#include <windows.h>/#include <Windows.h>/g to match proper case..
Thu, May 16, 2:37 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project

Mon, May 13

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

@compnerd Any other test cases you'd like before the first drop? @jakehehrlich @rupprecht Do things here look good to you guys? Both formats are marked experimental, I plan to work on a merge action next.

Mon, May 13, 9:36 AM · Restricted Project, Restricted Project

Sat, May 11

plotfi committed rGa10f016006ca: [NFC] yaml2obj/yam2elf.cpp whitespace changes: dos2unix removed CRs. (authored by plotfi).
[NFC] yaml2obj/yam2elf.cpp whitespace changes: dos2unix removed CRs.
Sat, May 11, 10:02 AM

Fri, May 10

plotfi updated the diff for D60974: Clang IFSO driver action..

Adding lit test cases.

Fri, May 10, 5:22 PM · Restricted Project, Restricted Project
plotfi added a comment to D60974: Clang IFSO driver action..

This really needs more tests. Please add a positive/negative test for the driver argument. Please try to organise the tests a bit to show what it is that they are testing, emission of public functions, not of protected functions or hidden functions. Behaviour with C++ classes. Behaviour with templates. I don't see any test for the vtable for classes that are public/private.

Fri, May 10, 2:09 PM · Restricted Project, Restricted Project
plotfi 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.

If we put things under an --experimental flags, minimize what we add in the first patch, and try and make each additional field added need based, I'm ok with whatever being added to Clang assuming it doesn't interfere with other parts of the compiler.

Fri, May 10, 2:01 PM · Restricted Project, Restricted Project
plotfi updated the diff for D60974: Clang IFSO driver action..

Changing !tapi-tbe to !experimental-tapi-elf-v1, removing Endianness field from tapi-tbe.

Fri, May 10, 12:44 PM · Restricted Project, Restricted Project
plotfi 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.

If we put things under an --experimental flags, minimize what we add in the first patch, and try and make each additional field added need based, I'm ok with whatever being added to Clang assuming it doesn't interfere with other parts of the compiler.

Fri, May 10, 12:11 PM · Restricted Project, Restricted Project
plotfi added inline comments to D60974: Clang IFSO driver action..
Fri, May 10, 11:58 AM · Restricted Project, Restricted Project
plotfi added a comment to D60974: Clang IFSO driver action..

@jakehehrlich What else are you looking for here? Can the patch for merging tbe files come later? @compnerd @rupprecht thoughts? I think this is a reasonable compromise: a minimal subset of what the yaml2obj and llvm-elfabi (with the addition of the Endian field) tools can already consume.

Fri, May 10, 10:18 AM · Restricted Project, Restricted Project

Thu, May 9

plotfi added inline comments to D60974: Clang IFSO driver action..
Thu, May 9, 9:38 PM · Restricted Project, Restricted Project
plotfi updated the diff for D60974: Clang IFSO driver action..

renaming things from "ifso" to "interface stubs"

Thu, May 9, 8:18 PM · Restricted Project, Restricted Project
plotfi updated the diff for D60974: Clang IFSO driver action..

cleaned up VisitNamedDecl

Thu, May 9, 6:21 PM · Restricted Project, Restricted Project
plotfi updated the diff for D60974: Clang IFSO driver action..

removed all the template functions to handle decls.

Thu, May 9, 5:49 PM · Restricted Project, Restricted Project
plotfi updated the diff for D60974: Clang IFSO driver action..

addressing a lot of @compnerd 's feedback. Still lots of templating.

Thu, May 9, 2:05 PM · Restricted Project, Restricted Project
plotfi added inline comments to D60974: Clang IFSO driver action..
Thu, May 9, 1:44 PM · Restricted Project, Restricted Project
plotfi added inline comments to D60974: Clang IFSO driver action..
Thu, May 9, 8:24 AM · Restricted Project, Restricted Project

Tue, May 7

plotfi added inline comments to D60974: Clang IFSO driver action..
Tue, May 7, 9:06 AM · Restricted Project, Restricted Project
plotfi added a comment to D60974: Clang IFSO driver action..

@jakehehrlich @compnerd ping

Tue, May 7, 7:44 AM · Restricted Project, Restricted Project

Mon, May 6

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

I didn't follow the technical details, but I don't see anything wrong with moving forward on this patch. I think this seems like an interesting idea worth experimenting with.

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)???

This is my proposal:

  1. We should plan on adding the code ofr merging these files inside of llvm/tools/llvm-elfabi but will it be a separate tool from llvm-elfabi. You can create "separate" tools in the same directory using the symlink trick. See llvm-ar and friends or llvm-objcopy and llvm-strip. The tool name can be discussed in review.

The symlink trick is usually used when the new frontend tool is just a thin layer over the underlying tool. Is that going to be the case here?

For example,

  • llvm-ranlib is equivalent to llvm-ar s
  • llvm-readelf is equivalent to llvm-readobj --elf-output-style=GNU
  • llvm-addr2line is equivalent to llvm-symbolizer --output-style=GNU
  • llvm-strip is equivalent to llvm-objcopy --strip-all (maybe not exactly)

    In other words, is llvm-mergeifo (placeholder name) going to be roughly equivalent to llvm-elfabi --merge-ifo (again, placeholder flag name)? And if so, it doesn't seem like a symlink is strictly necessary -- users can just call llvm-elfabi --merge-ifo. Am I missing something?
Mon, May 6, 10:04 AM · Restricted Project, Restricted Project

Sat, May 4

plotfi updated the diff for D60974: Clang IFSO driver action..

Changing experimental flag to -interface-stubs-version=experimental-ifo-elf-v1

Sat, May 4, 9:03 PM · Restricted Project, Restricted Project
plotfi updated the diff for D60974: Clang IFSO driver action..

Adding support for multiple formats using -interface-stubs-version=.

Sat, May 4, 6:19 PM · Restricted Project, Restricted Project

Thu, May 2

plotfi 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.

  1. The output of nm when you give it what exactly is different? What output are you expecting? You aren't making that specific. Passing the unmerged output though nm doesn't make any sense. If you have a precise use case for section names you should mention it. We can always add symbol information back as needed if we first start with something more minimal. What output of nm are you expecting a
  2. We're basically in agreement on what should happen here. I don't think it's a dream of unifying these things. It looks like a very close reality to me.
  3. You mention sections and relocation as being relevant information. Can you give a specific use case? Section names in general are not meaningful information. They're just there for humans to debug things. Putting symbols in different sections lets the linker treat them specially but in the end binary sections are not relevant to much of anything.
Thu, May 2, 10:24 AM · Restricted Project, Restricted Project

Wed, May 1

plotfi updated the diff for D60974: Clang IFSO driver action..
  • change "ifso" for intermediate files to "ifo"
  • Updated schemas. I have both our own proposal and the proposal that resembles elfeabi here.
Wed, May 1, 2:49 PM · Restricted Project, Restricted Project

Tue, Apr 30

plotfi 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.

Tue, Apr 30, 9:29 PM · Restricted Project, Restricted Project
plotfi 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?

I'm not sure what you mean. yaml2obj is a different tool that we shouldn't be using IMO. yaml2obj would require the sections to denote that a symbol was defined. When producing an elf file from the .tbe format you have to synthesize sections for this same reason.

Tue, Apr 30, 9:24 PM · Restricted Project, Restricted Project
plotfi added a comment to D60974: Clang IFSO driver action..

Oh and "ifo" is a good name.

Tue, Apr 30, 6:33 PM · Restricted Project, Restricted Project
plotfi 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.

Tue, Apr 30, 6:02 PM · Restricted Project, Restricted Project
plotfi 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?

You don't need that information and the linker doesn't use it. It only uses that information to get alignment. In the code that I'm writing right now it happens to put everything in a single rodata section (except for tls which it puts in a TLS rodata section, which isn't a thing that even exists in the wild)

Tue, Apr 30, 6:01 PM · Restricted Project, Restricted Project
plotfi 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.

You asked about the format I proposed here, not the already accepted .tbe format. The format proposed here could be added to TextAPI where it could subsequently be used in both Clang and a tool that would use the llvm-elfabi code to create stubs. I imagine the flags needed for this new tool would be very differnet from what llvm-elfabi uses or would use in the future so we'd probably want a second symlink the behaved very differently.

It can currently read ELF and .tbe files and can currently write .tbe files. I'll have the basics of ELF writing up for review by the end of the day and that will, for the next month or two be my primary work which should likely see it close to stabilized. I'd expect landing the basics of ELF writing to take another week or two since its somewhat involved and I'll likely need to split the patches up from what I have. Given that no such writer currently exists this should put a version of your tool well ahead of schedule. Such are the wonderful benefits of open source.

Tue, Apr 30, 5:20 PM · Restricted Project, Restricted Project
plotfi added a comment to 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.

Tue, Apr 30, 3:37 PM · Restricted Project
plotfi added a comment to D60974: Clang IFSO driver action..

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

No, I just proposed it and explained my reasoning. If you wanted to add this format to TextAPI that would be acceptable.

Tue, Apr 30, 3:09 PM · Restricted Project, Restricted Project

Mon, Apr 29

plotfi 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).

This is a reasonable opinion and was my opinion as well. But that isn't the way review went for .tbe and so now we have a responsibility to be consistent. This is bike shed level stuff. I could care less either way except for consistency.

Mon, Apr 29, 9:18 PM · Restricted Project, Restricted Project
plotfi 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.

Mon, Apr 29, 5:17 PM · Restricted Project, Restricted Project
plotfi 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.
Version: 1.0
Arch: AArch64
Symbols:
  - Name: foo
    Type: Function
    Weak: true
  - Name: bar
    Type: Function
    Defined: false
  - Name: baz
    Type: Object
    Size: 256
    Alignment: 32
Mon, Apr 29, 5:10 PM · Restricted Project, Restricted Project
plotfi 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.
Version: 1.0
Arch: AArch64
Symbols:
  - Name: foo
    Type: Function
    Weak: true
  - Name: bar
    Type: Function
    Defined: false
  - Name: baz
    Type: Object
    Size: 256
    Alignment: 32
Mon, Apr 29, 3:42 PM · Restricted Project, Restricted Project
plotfi 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.

...

Can you clarify is this is for the fully merged format or for the unmerged format? I *really* strongly believe that the fully merged format should go though llvm-elfabi.

Mon, Apr 29, 3:34 PM · Restricted Project, Restricted Project
plotfi 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.

Mon, Apr 29, 3:08 PM · Restricted Project, Restricted Project
plotfi added a comment to D60974: Clang IFSO driver action..
Mon, Apr 29, 1:47 PM · Restricted Project, Restricted Project

Thu, Apr 25

plotfi 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.

Thu, Apr 25, 5:40 PM · Restricted Project, Restricted Project

Wed, Apr 24

plotfi 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.

I don't know when but in the next 2 days likely. Regardless of my timeline I don't think I'm blocking anyone. I'm just saying that I will put up a proposal. You should also put up a proposal as well. The yaml2obj format is just not well designed for any purpose but is far from designed for this purpose. yaml2obj works ok-ish for testing. I'd rather start with a minimal format and then add things to it rather than starting with a bloated and ill designed format and then create a new format from that experience. Creating a minimal format shouldn't be hard and I suspect our proposal will look extremely similar; I'd wager if you put up a proposal I'd probably just review your proposal rather than bother writing out my own.

As for what I think it would entail. I think name, weather or not the symbol is defined or undefined, visibility, size, alignment (this is a feature of the section and the symbol offset) and type will all mater but not all combinations make sense. Sections don't matter as it turns out but alignment does for copy relocations. When we started llvm-elfabi I thought at least the section permissions mattered but they don't really. While .tbe has things like soname and dt_needed that isn't needed here. The top of the file should probably contain the architecture. Other details in the ELF header shouldn't be needed.

Wed, Apr 24, 11:35 PM · Restricted Project, Restricted Project
plotfi updated the diff for D60974: Clang IFSO driver action..

First stab at trying to properly handle Weak Symbols

Wed, Apr 24, 3:27 PM · Restricted Project, Restricted Project
plotfi added a comment to D60974: Clang IFSO driver action..

Aside from comments that @jakehehrlich already made which I agree with, I'm also concerned about the use of yaml2obj. AFAIK that tool has always been intended as a testing, not a production tool, but with this change this would no longer be the case. Specifically, it formalizes the yaml2obj input format by making it one of the output formats supported by Clang. I believe that this is such a fundamental change that it should be discussed separately before moving on with this implementation.

Wed, Apr 24, 2:06 PM · Restricted Project, Restricted Project
plotfi updated the diff for D60974: Clang IFSO driver action..

Updated to support properly setting OBJECT/FUNC types as well as section flags.

Wed, Apr 24, 1:57 PM · Restricted Project, Restricted Project

Tue, Apr 23

plotfi 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?

I'm well versed in the complexities of a linker - I've worked extensively on the GNU linkers as well as with lld. The visibility of the symbols is actually computed by the compiler - the STV_* flags associated with the symtab entry give you that information which is actually tracked through the frontend to the backend. Yes, each linker behaves differently - including lld which completely changes the semantics of Unix/ELF linking guarantees. In fact every single attribute that you mentioned is something which is emitted from the compiler - which we have access to here given that we are in clang itself. I think that this can be made to work properly, though will probably require some iteration to get all the corner cases, and that you may be slightly dismissive of this approach.

Ah, okay, I didn't pick up that you were using module as the actual module from the context since normally the module doesn't control what it exposes itself.

Note that I am not advocating that this change go in as is - I think that this is far from something which would be useful, and until it can produce something meaningful, this shouldn't really be merged (i.e. generate something which obj2yaml or some other tool can consume to generate the barest ELF stub).

It's not a matter of where they're produced, its how you treat them and ensure that they're correctly handled so that the final produced stub has the correct output. I don't think this is entirely trivial when you consider all of those different aspects that I listed.

Tue, Apr 23, 1:45 PM · Restricted Project, Restricted Project
plotfi 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).

This would require creating a linker for these files. Is this what you intend to do?

Tue, Apr 23, 11:49 AM · Restricted Project, Restricted Project
plotfi added a comment to D60974: Clang IFSO driver action..

Note that I am not advocating that this change go in as is - I think that this is far from something which would be useful, and until it can produce something meaningful, this shouldn't really be merged (i.e. generate something which obj2yaml or some other tool can consume to generate the barest ELF stub).

Tue, Apr 23, 11:37 AM · Restricted Project, Restricted Project
plotfi updated the diff for D60974: Clang IFSO driver action..

I've added an output format. The YAML coming out of -emit-ifso can now run through yaml2obj to produce a striped .o file. Those .o files can be handled by an existing linker.

Tue, Apr 23, 11:35 AM · Restricted Project, Restricted Project

Mon, Apr 22

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

Can you elaborate on the use case for this? Like can you explain end to end how this would be used?

Mon, Apr 22, 3:13 PM · Restricted Project, Restricted Project
plotfi added inline comments to D60974: Clang IFSO driver action..
Mon, Apr 22, 11:43 AM · Restricted Project, Restricted Project
plotfi added a reviewer for D60974: Clang IFSO driver action.: jakehehrlich.
Mon, Apr 22, 11:37 AM · Restricted Project, Restricted Project
plotfi added reviewers for D60974: Clang IFSO driver action.: compnerd, alexshap, phosek.
Mon, Apr 22, 11:26 AM · Restricted Project, Restricted Project
plotfi created D60974: Clang IFSO driver action..
Mon, Apr 22, 11:26 AM · Restricted Project, Restricted Project

Mar 28 2019

plotfi committed rG6c8269575328: [yaml2obj] Fixing opening empty yaml files. (authored by plotfi).
[yaml2obj] Fixing opening empty yaml files.
Mar 28 2019, 3:54 PM
plotfi updated the diff for D59964: [yaml2obj] Fixing opening empty yaml files..
Mar 28 2019, 3:48 PM · Restricted Project
plotfi added reviewers for D59964: [yaml2obj] Fixing opening empty yaml files.: beanz, compnerd.
Mar 28 2019, 3:00 PM · Restricted Project
plotfi created D59964: [yaml2obj] Fixing opening empty yaml files..
Mar 28 2019, 2:59 PM · Restricted Project

Mar 24 2019

plotfi abandoned D59745: [NFC] Move writeFuncOrVarName out of class CodegenNameGenerator so that it can be reused more easily. .

Found a better way to do what I needed without this unnecessary change.

Mar 24 2019, 5:33 PM · Restricted Project

Mar 23 2019

plotfi created D59745: [NFC] Move writeFuncOrVarName out of class CodegenNameGenerator so that it can be reused more easily. .
Mar 23 2019, 11:05 PM · Restricted Project

Feb 21 2019

plotfi closed D58462: Fixing NDEBUG typo in include/llvm/Support/raw_ostream.h.

Landed in e7e16a7 (github) / r354495 (svn).

Feb 21 2019, 9:09 AM · Restricted Project

Feb 20 2019

plotfi committed rGe7e16a72a642: Fixing NDEBUG typo in include/llvm/Support/raw_ostream.h (authored by plotfi).
Fixing NDEBUG typo in include/llvm/Support/raw_ostream.h
Feb 20 2019, 10:31 AM
plotfi created D58462: Fixing NDEBUG typo in include/llvm/Support/raw_ostream.h.
Feb 20 2019, 10:13 AM · Restricted Project

Dec 9 2018

plotfi added a comment to D54674: [llvm-objcopy] First bits for MachO .

Apart from these specific comments, my general thought is that this is fine as far as it goes, but as it grows more fully featured it seems likely to start overlapping more libObject and MC functionality.

Have you considered ways to refactor libObject/MC to allow objcopy to be built on top of them?

Dec 9 2018, 11:04 PM · Restricted Project

Nov 3 2018

plotfi added a comment to D50725: [SystemZ] Replace subreg_r with subreg_h.

Yes, I do have one. I'll post it on Monday.

Nov 3 2018, 10:25 PM

Sep 30 2018

plotfi updated the diff for D51841: [llvm-objcopy] Dwarf decompression support. .
Sep 30 2018, 6:05 PM
plotfi added inline comments to D51841: [llvm-objcopy] Dwarf decompression support. .
Sep 30 2018, 5:47 PM
plotfi updated the diff for D51841: [llvm-objcopy] Dwarf decompression support. .
  • changed std::function to llvm::function_ref
  • added cp %t %t.o to tests
Sep 30 2018, 5:45 PM

Sep 24 2018

plotfi added inline comments to D51841: [llvm-objcopy] Dwarf decompression support. .
Sep 24 2018, 9:10 AM
plotfi updated the diff for D51841: [llvm-objcopy] Dwarf decompression support. .
Sep 24 2018, 9:10 AM

Sep 21 2018

plotfi added inline comments to D51841: [llvm-objcopy] Dwarf decompression support. .
Sep 21 2018, 10:59 AM
plotfi updated the diff for D51841: [llvm-objcopy] Dwarf decompression support. .
Sep 21 2018, 10:59 AM
plotfi updated the diff for D51841: [llvm-objcopy] Dwarf decompression support. .
Sep 21 2018, 10:53 AM

Sep 17 2018

plotfi updated the diff for D51841: [llvm-objcopy] Dwarf decompression support. .
Sep 17 2018, 10:24 AM

Sep 15 2018

plotfi updated the diff for D51841: [llvm-objcopy] Dwarf decompression support. .
Sep 15 2018, 5:31 PM

Sep 14 2018

plotfi updated the diff for D51841: [llvm-objcopy] Dwarf decompression support. .
Sep 14 2018, 4:17 PM
plotfi updated the diff for D51841: [llvm-objcopy] Dwarf decompression support. .
Sep 14 2018, 2:50 PM
plotfi updated the diff for D51841: [llvm-objcopy] Dwarf decompression support. .

-Cleanup: removing DecompressableSection, reusing CompressedSection.

Sep 14 2018, 1:31 PM
plotfi added inline comments to D51841: [llvm-objcopy] Dwarf decompression support. .
Sep 14 2018, 11:28 AM
plotfi updated the diff for D51841: [llvm-objcopy] Dwarf decompression support. .
Sep 14 2018, 11:28 AM
plotfi added inline comments to D51841: [llvm-objcopy] Dwarf decompression support. .
Sep 14 2018, 10:56 AM
plotfi updated the diff for D51841: [llvm-objcopy] Dwarf decompression support. .
Sep 14 2018, 10:50 AM
plotfi added inline comments to D51841: [llvm-objcopy] Dwarf decompression support. .
Sep 14 2018, 9:03 AM
plotfi added a comment to D51841: [llvm-objcopy] Dwarf decompression support. .

I may have missed this somewhere, but what happens if you specify --decompress/--compress-debug-sections on a system without zlib?

Sep 14 2018, 8:21 AM

Sep 13 2018

plotfi added a comment to D51841: [llvm-objcopy] Dwarf decompression support. .

although I'd probably try to make the implementation more symmetric (i mean "compression" and "decompression"). Would that be possible ?

Sep 13 2018, 7:07 PM
plotfi added inline comments to D51841: [llvm-objcopy] Dwarf decompression support. .
Sep 13 2018, 10:49 AM
plotfi updated the diff for D51841: [llvm-objcopy] Dwarf decompression support. .
  • using two new classes: DecompressableSection and DecompressedSection.
  • When a compressed debug section is encountered in makeSection we create a DecompressableSection instead of a Section so that we can keep track of the DecompressedSize (which we need ELFT to read).
  • in handleArgs if the Decompression command line arg is set then we replace all of the DecompressableSections with DecompressedSections. The difference being that the decompressed sections unset the SHF_COMPRESSED flag, undo the .zdebug* naming, and set the Section size (and alignment to DecompressedSize and DecompressedAlign. DecompressedSections Writer function also does the decompressing whereas DecompressableSection just writes the original uncompressed data.
Sep 13 2018, 10:49 AM
plotfi added inline comments to D51841: [llvm-objcopy] Dwarf decompression support. .
Sep 13 2018, 9:25 AM

Sep 12 2018

plotfi updated the diff for D51841: [llvm-objcopy] Dwarf decompression support. .

Added DecompressedSection.

Sep 12 2018, 3:22 PM
plotfi added inline comments to D51841: [llvm-objcopy] Dwarf decompression support. .
Sep 12 2018, 8:42 AM

Sep 9 2018

plotfi created D51841: [llvm-objcopy] Dwarf decompression support. .
Sep 9 2018, 1:41 PM
plotfi closed D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..

Closing. Everything is in. (I think I messed up the case for "Differential Revision" in my commit)

Sep 9 2018, 1:41 PM

Sep 7 2018

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

Reverted temporarily in r341360 to unbreak my build and the aarch64 build bot. Let me know if I can help with the investigation in any way.

@jakehehrlich @chandlerc I need help reproing this. I finally got an aarch64 setup up and running and the tests pass. I don't understand why this would fail.

Looks like I'm able to repro this on my machine. (I hit it while I was running my own objcopy tests and happened to be synced to when this was recommitted). I can work with you on this.

I'm wrapping up for the day so I don't have time to go much further, but one thing I did notice that was really funny is there are multiple sections with the same name, e.g. for compress-debug-sections-zlib.test.tmp-compressed.o:

Section {
  Index: 1
  Name: .debug_foo (20)
  Type: SHT_PROGBITS (0x1)
  Flags [ (0x0)
  ]
  Address: 0x0
  Offset: 0x40
  Size: 8
  Link: 0
  Info: 0
  AddressAlignment: 0
  EntrySize: 0
}
Section {
  Index: 2
  Name: .debug_foo (20)
  Type: SHT_PROGBITS (0x1)
  Flags [ (0x800)
    SHF_COMPRESSED (0x800)
  ]
  Address: 0x0
  Offset: 0x48
  Size: 35
  Link: 0
  Info: 0
  AddressAlignment: 8
  EntrySize: 0
}

In other words, the FileCheck test that .debug_foo has the SHF_COMPRESSED flag is matching the wrong one. (Of course, it's not FileCheck that's wrong, it's the fact that there are somehow two sections with the same name).

I don't think there's anything unique about my machine setup... it's just a regular x86-64 linux machine, cmake is configured to use a near-head clang to build things... I hope to have time to take a closer look at this tomorrow.

Sep 7 2018, 1:15 AM

Sep 6 2018

plotfi updated subscribers of D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..

So with clang-6 I get:

Sep 6 2018, 8:20 PM
plotfi updated the diff for D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..
  • added zlib::isAvailable() checks to various places before compressing.
  • fixed up potential alignment issues with Chdr writes.
Sep 6 2018, 3:53 PM