[Lldb-commits] [PATCH] D26093: Limit LLDB_EXPORT_ALL_SYMBOLS to additionally export only the lldb_private namespace symbols

[Todd Fiala via lldb-commits]

tfiala added a comment.

In https://reviews.llvm.org/D26093#582659, @beanz wrote:

> In https://reviews.llvm.org/D26093#582618, @tfiala wrote:
>
> > The current usage of CommandLine via global static constructors in Debug.cpp seems like a poor choice, as it forces all users of llvm with NDEBUG not defined to get some command line behavior that they may never use.  Maybe some aspect of that can be made lazy?
>
>
> It can't be made lazy without completely re-designing how cl::opt works. Which is something I've had lengthy discussions with Chandler about. Currently the biggest issue with fixing it relates to how the new pass manager works. Specifically in order to provide meaningful -help output we need a way to register command line options in passes, which we have today, but won't with the new pass manager.
>
> It is an open problem that we're trying to come up with a solution to.


Cool - glad to see some thought is going into it.

On other projects where there are firewalls between linkage domains, I've had one subsystem on one side to be initialized with a pointer to the storage location(s) on the other, by way of an init() routine that is called by a third party passing it over the barrier.

Something like this, in terms of our problem here (pseudo-code and maybe wrong naming convention!):

// in lldb-mi::main(), main binary
int main() {

  // Ask lldb-mi's local llvm's CommandLine to return a handle to its internal data structures,
  // and pass that to the agent that controls the llvm embedded in liblldb.
  liblldbInitCommandLine(cl::CommandLine::GetHandle());
  ...

}

// in liblldb so/dylib/dll, exported method:

void liblldbInitCommandLine(void *commandLineHandle) {

  // Pass the given opaque handle to the LLVM CommandLine embedded in liblldb.
  cl::CommandLine::SetHandle(commandLineHandle);

}

// in CommandLine

void *CommandLine::GetHandle() {

  return static_cast<void*>(my_internal_pimpl);

}

void CommandLine::SetHandle(void *external_pimpl_to_use_here) {

  // Make concurrency safe if desired...
  // Merge existing results with those present in the passed in version, if desired (see below)
  delete my_internal_pimpl;
  my_internal_pimpl = (some_type*)(external_pimpl_to_use_here);

}

The trick to this working, though, is to not have uncontrolled, global static initializers permuting these on both sides until after the dance of synchronizing their storage explicitly.  And that isn't the case here.  We can probably work around it with 'debug' since we know both sides will do it, and so there is no race to avoid.  But if other clients on either side do the same thing, we can lose them.  Unless, we have some kind of merge semantics on the SetHandle(void*) call, which I suppose could be done to collate any items that were initialized on either side.

Anyways, food for thought...  I'm not thrilled with the manual aspect of it, but I have used it in production.

>> Unless there is strong opposition, I'd like to put this in.
> 
> No objection from me.

Okay - Pavel seemed alright enough with it for the time being as well, so I'll put this in, with the one wording correction I called out above.


https://reviews.llvm.org/D26093