Page MenuHomePhabricator

PluginInstructionARM: avoid unnecessary link
Needs ReviewPublic

Authored by compnerd on Apr 30 2019, 7:19 PM.

Details

Summary

lldbPluginInstructionARM does not depend on lldbProcessUtility at link
time. The dependency is for defines and helper functions which are
header only dependencies.

Diff Detail

Repository
rLLDB LLDB

Event Timeline

compnerd created this revision.Apr 30 2019, 7:19 PM
labath added a subscriber: labath.May 2 2019, 12:51 AM

I don't think this is a good way to solve this problem. The llvm's definition of dependencies http://llvm.org/docs/CodingStandards.html#library-layering explicitly that it is not the same as "the thing that's needed to make linkers happy" (emphasis mine):

One library (both its headers and implementation) should only use things from the libraries listed in its dependencies.

***Some*** of this constraint can be enforced by classic Unix linkers...

This doesn’t fully enforce all inter-library dependencies, and importantly doesn’t enforce header file circular dependencies created by inline functions. A good way to answer the “is this layered correctly” would be to consider whether a Unix linker would succeed at linking the program if all inline functions were defined out-of-line...

I am guessing that you the reason for this dependency is the headers defining various register constants. A good way to solve that would be to create a new library, and move all the headers of this kind there...

compnerd added a comment.EditedMay 2 2019, 8:21 AM

@labath - I don't have a problem with the dependency - we can do a add_dependency for the plugin. My issue is that this should not be linked against. That is to say, LINK_LIBS is not the same thing as a dependency. This mess of dependencies makes it difficult to track down the bloat in lldb-server. Currently, lldb-server weighs in at ~9M while gdb-server is ~700K.

compnerd updated this revision to Diff 197793.May 2 2019, 8:24 AM

convert link to a proper dependency

labath added a comment.May 2 2019, 8:25 AM

My point is that we shouldn't need to differentiate between "link" dependencies and "other" dependencies, as it's not a well-defined boundary. It's easy to turn one into the other by moving things in or out of line. Hence my suggestion - move the required things into a different (new?) module.

labath added a comment.May 2 2019, 8:28 AM

Or another thing: if one sees a header from module X being included in file Y, then it's reasonable to assume it's fair game to include another file from the same module X is fair game. But this creates a two classes of header files: files in X, which don't have a link dependency, and those that do...

@labath - but that is the reality of this: there *are* two classes of dependencies. I think that if we try to split up all the modules into linked libraries and header-only libraries, we are going to make an even bigger mess than what we have.