[Lldb-commits] [PATCH] D62501: Implement GetSharedLibraryInfoAddress

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D62501#1531959 <https://reviews.llvm.org/D62501#1531959>, @aadsm wrote:

> @labath while working on this I had to also move the GetAuxValue function into the NativeProcessELF, which I think it's fine. However, this means this class now depends on the AuxValue class defined in the PluginUtility. Initially I was going to put the NativeProcessELF in the lldbHost but it feels weird that this will now depend on a plugin, even if it's the utility one. Should I create a new ELF plugin just for this?


Yeah, having Host depend on Process/Utility would be suboptimal, as we've just spend a lot of time making sure Host does not depend on anything. I would be fine with creating a new plugin for this. A couple of other possible solutions I see are:

- put this in Plugins/Process/POSIX: These days, the folder is mostly empty, so we could start repurposing it for this. Having an "ELF" process inside "POSIX" folder looks a bit weird, but my reasoning behind that is that we will probably one day need a NativeProcessPOSIX to share the common code between non-elf posix platform (i.e. darwin). Then NativeProcessELF would be a refinement (subclass) of NativeProcessPOSIX
- move all Native***Protocol classes out of Host into some new place. This is pretty specialized code, and I don't think it needs to live in a relatively general place. At that point it becomes less important to have to outgoing dependencies here.
- make the rendezvous structure parser a "utility" too. All this needs to function is the aux vector and something to read the memory with. You could provide these as arguments to its constructor. This way we won't need a NativeProcessELF, side-stepping the issue of where to place it. Additionally, this would open the door for using this code on the client side. Right now, DynamicLoaderPOSIXDYLD plugin has it's own code for parsing this structure, but I don't see a reason why this code couldn't be used there too. It would also solve the testing problems, as you'd just need to mock these two things (auxv, memory), and not the full NativeProcessProtocol API.

When I spell things out this way, I think I'm starting to prefer the last option. Let me know what you think.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62501/new/

https://reviews.llvm.org/D62501