[PATCH] D53051: [llvm-tapi] initial commit, supports reading ELF

Wed Oct 10 14:52:57 PDT 2018

amontanez added a comment.

In https://reviews.llvm.org/D53051#1259815, @steven_wu wrote:

> Thanks for posting this. I will start with some high level comments.
>
> I see you adopt the idea of a registry from Apple's tapi but your implementation of the different file format is actually really specific. Adding macho support will likely end up being a parallel code path inside the registry which doesn't share any code.
>  Apple's tapi implementation has been changed/improved quite a bit from the version on GitHub and we are working on updating it. There are some fundamental design in Apple's tapi which I really wish we can share but I am not sure it can provide any benefits to ELF if all you care about is stubbing. We have a common intermediate representation that is an abstraction for all the interfaces from a framework (similar to XPISet from the current GitHub version). The intermediate representation can be built from headers, binaries or stubs and it can be serialized, compared or modified. We use this data structure to correlated the interfaces from headers and binaries, and it can be used to do more. Are you interested to build ELF on top of some common abstraction or you just want to stick to the minimal stubbing version?

The choice of using the registry as the only common interface was an effort to quickly produce stubbing functionality since that's the top concern for ELF. Additionally, it helps the ELF component to not interfere with or depend on the Mach-O specifics for the initial commit. With that said, diverging so quickly after the registry was more a choice of quick prototyping to get the discussions started. To answer your question more directly: I believe abstracting more of Apple's TAPI to generically support ELF and Mach-O would be desirable. While the features of Mach-O TAPI don't necessarily match the desired feature path of ELF TAPI, I don't want to shut out the opportunity for feature parity early on.

One option might be giving ELF its own internal representation parallel to XPISet (or the new version), and having both the ELF and Mach-O XPISet implementations meet with a common interface for shared functionality. The data structures in ELFStubFile are rather crude since the goal is to have all that replaced with something that better matches the existing structure of Apple's TAPI. My primary reason for not initially pulling over XPISet and making it ELF-compatible was that I was concerned I might fundamentally break or hinder Mach-O support in some way. I was noticing that Apple's TAPI has Mach-O specifics echoing throughout it, so I wanted to consult someone more familiar with the code before attempting to generalize those. Using Apple TAPI as-is and bolting on ELF would be quite messy and would inevitably cripple ELF support, so I avoided just trying to pull in Apple TAPI as the first commit. I’m not certain how much this picture changes with the updates to Apple TAPI.

Basically, I'd like this patch to include some generalized structure that can be commonly used by ELF and Mach-O, but still provide some sort of basic ELF functionality. To that end, I'm perfectly fine with fundamentally restructuring what I've written. This patch should ideally provide some functionality independently, but also allow the Mach-O implementation to be pulled in immediately in parallel to begin the process of introducing Mach-O and the complete Apple TAPI feature set. I know some features will just be Mach-O or ELF specific due to the nature of different object types, but I'm not sure it will be clear how to best approach those issues until they actually surface. If we wanted to take Apple’s TAPI and provide slightly more generic interfaces that support both ELF and Mach-O, what do you feel would make for the best starting point(s)?

As an aside, would it be a good opportunity to introduce Apple TAPI’s newer changes when Apple TAPI begins migrating into llvm-tapi?

Repository:
  rL LLVM

https://reviews.llvm.org/D53051