fixing support for static functions.

adding a nice inline test

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.

In D62034#1506879, @mclow.lists wrote:

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

In D62034#1506102, @mstorsjo wrote:

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.

adding all windows.h's in all of monorepo

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

Adding lit test cases.

In D60974#1498604, @compnerd wrote:

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.

In D60974#1498240, @jakehehrlich wrote:

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.

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

In D60974#1498240, @jakehehrlich wrote:

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.

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

renaming things from "ifso" to "interface stubs"

cleaned up VisitNamedDecl

removed all the template functions to handle decls.

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

@jakehehrlich @compnerd ping

In D60974#1490358, @rupprecht wrote:

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.

In D60974#1488563, @jakehehrlich wrote:

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?

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

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

In D60974#1486631, @jakehehrlich wrote:

In D60974#1486384, @compnerd wrote:

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

• change "ifso" for intermediate files to "ifo"
• Updated schemas. I have both our own proposal and the proposal that resembles elfeabi here.

In D60974#1485607, @jakehehrlich wrote:

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.

In D60974#1485610, @jakehehrlich wrote:

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.

In D60974#1485611, @jakehehrlich wrote:

Oh and "ifo" is a good name.

In D60974#1485552, @jakehehrlich wrote:

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.

In D60974#1483479, @jakehehrlich wrote:

In D60974#1483380, @plotfi wrote:

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)

In D60974#1485417, @jakehehrlich wrote:

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.

In D55839#1481114, @jakehehrlich wrote:

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.

In D60974#1484744, @jakehehrlich wrote:

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.

In D60974#1483480, @jakehehrlich wrote:

In D60974#1483399, @plotfi wrote:

In D60974#1483265, @jakehehrlich wrote:

In D60974#1483240, @plotfi wrote:

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.

In D60974#1483265, @jakehehrlich wrote:

In D60974#1483240, @plotfi wrote:

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.

In D60974#1483209, @jakehehrlich wrote:

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

In D60974#1483209, @jakehehrlich wrote:

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

In D60974#1483210, @jakehehrlich wrote:

In D60974#1483164, @plotfi wrote:

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.

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.

In D60974#1479467, @jakehehrlich wrote:

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.

In D60974#1477761, @jakehehrlich wrote:

In D60974#1477690, @compnerd wrote:

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

First stab at trying to properly handle Weak Symbols

In D60974#1477302, @phosek wrote:

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.

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

In D60974#1476084, @jakehehrlich wrote:

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.

In D60974#1474777, @jakehehrlich wrote:

In D60974#1474751, @plotfi wrote:

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?

In D60974#1474994, @compnerd wrote:

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

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.

In D60974#1474591, @jakehehrlich wrote:

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

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

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

In D54674#1322482, @lhames wrote:

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?

In D50725#1204855, @kparzysz wrote:

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

• changed std::function to llvm::function_ref
• added cp %t %t.o to tests

-Cleanup: removing DecompressableSection, reusing CompressedSection.

In D51841#1234544, @jhenderson wrote:

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

In D51841#1234283, @alexshap wrote:

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

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

Added DecompressedSection.

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

In D49678#1226825, @rupprecht wrote:

In D49678#1226534, @plotfi wrote:

In D49678#1222836, @chandlerc wrote:

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.

So with clang-6 I get:

• added zlib::isAvailable() checks to various places before compressing.
• fixed up potential alignment issues with Chdr writes.