[lldb][CMake] Enforce not linking against plugin libs in core libs

[bulbazord]

Summary

Non-plugin lldb libraries should generally not be linking against lldb
plugin libraries. Enforce this in CMake.

[JDevlieghere]

LGTM

[DavidSpickett]

Is this policy documented anywhere? Perhaps you can update one of the design pages if it isn't already there.

Because I am a bit confused about it. Some parts of lldb use plugins and those parts are called by commands through a few other layers. However commands can't link to the plugins directly, but some of them do show information that further down came from a plugin. I'm wondering what the best practices are (and some examples would be great!).

[bulbazord]

In D146553#4212417, @DavidSpickett wrote:

Is this policy documented anywhere? Perhaps you can update one of the design pages if it isn't already there.

Because I am a bit confused about it. Some parts of lldb use plugins and those parts are called by commands through a few other layers. However commands can't link to the plugins directly, but some of them do show information that further down came from a plugin. I'm wondering what the best practices are (and some examples would be great!).

This policy is not really documented anywhere. I've been working towards making the non-plugins not depend on any plugins for a few years now (along with some other folks, some of whom are no longer working on LLDB). As for updating one of the design pages, I think I'll update this one: https://lldb.llvm.org/design/overview.html
Currently, we are down to maybe 3-4 places where we're using plugins in non-plugin contexts and I have a few plans to remove those dependencies. They're a bit challenging to remove so I'll be moving a bit more carefully.

I can understand the confusion. Navigating LLDB's design is quite challenging because a lot of it is kind of ad-hoc and grew organically while some transitions from one thing to another weren't always completed 100%. I personally think that the name "plugin" is a bit misleading because they're not really things you can add/remove at runtime. You can't really even add/remove them at CMake configure time (though there was some interest in doing this in the past). I think a name better than "plugin" would be "implementation" because that's what the plugins are: specific implementations of more general debugger concepts (e.g. ABI support, Platforms, Languages and LanguageRuntimes, ObjectFiles, etc).

As for best practices, I suppose that depends on exactly which plugin you're using. If the goal is just to get some information from a relevant plugin, you can look at the way the Language plugins are designed. The Language class in lldb/source/Target/ has several functions like "FindPlugin" and "ForEach" which let you find the correct plugin for a language and operate over all known language plugins respectively. A slightly different model might be how the TypeSystems work. To get a scratch TypeSystem for example, we usually go through the Target that we have with Target::GetScratchTypeSystemForLanguage or something to this effect. I suppose it just depends on what you're trying to do, which I realize may not be the most helpful advice but I hope that with enough examples it becomes at least a little more clear how things in lldb are currently done...

One thing you may notice as you use and extend these plugins is that the base class for the plugin you're working with may not be able to support your use case. In this case, it may make sense to extend the interface to support your use case. The existing interfaces (e.g. TypeSystem) only support what has needed to be done up until now, so there may be a use case that makes sense that nobody bothered to add before. In other cases, it may make sense to rely on specific implementation details in said plugins. Ideally we would not be doing that in non-plugins as those are supposed to be more general though and maybe we can figure out a way to do it in a plugin instead.

This is all very off the cuff. I hope it makes sense and helps. I will be updating the design document (and I'll add you as a reviewer).

[jingham]

David,

I think you were also thinking about things like the language cplusplus commands or the settings set plugin.*** settings. That isn't a case of generic code depending on specific plugin implementations. Rather that's a general feature of the plugin loader, it queries the plugin to see if it has any commands or settings to add to the command interpreter when loaded. But the command interpreter doesn't "depend" on these plugin commands, it just vends them.

Jim

[jingham]

That general facility is why we think of plugins as something more than just implementations. Along with the "on load" actions, they also have the job of detecting "Am I the right implementation for this binary, this language, this OS version, etc". That also informs the proper way to use them. You start with something of a particular plugin type you want to work with - a path to a binary or a the language type you got from a StackFrame - which is managed by a particular plugin class, and you first ask the manager for that plugin type "find me the plugin that handles my entity", then call the generic API's on that plugin. You shouldn't dial up particular plugins or call anything but the plugin interface methods on them. If you have to do something that's specific to a particular plugin implementation, that needs to be done inside the plugin, and vended in some neutral way.

Jim

[DavidSpickett]

But the command interpreter doesn't "depend" on these plugin commands, it
just vends them.

Right, I'm conflating depending on as in linking to or specifically asking
for a certain plugin, vs. making use of the information that category of
plugins could provide if they are present. Commands don't just crash if
there's no plugin for a certain thing, they just say ok we don't know
anything at this time.

You start with something of a particular plugin type you want to work
with - a path to a binary or a the language type you got from a StackFrame

• which is managed by a particular plugin class, and you first ask the

manager for that plugin type "find me the plugin that handles my entity",
then call the generic API's on that plugin. You shouldn't dial up
particular plugins or call anything but the plugin interface methods on
them. If you have to do something that's specific to a particular plugin
implementation, that needs to be done inside the plugin, and vended in some
neutral way.

Understood. I should take what I want to do and make an API for that task.
Then the "how" goes in a plugin.

[DavidSpickett]

I suppose it just depends on what you're trying to do, which I realize may not be the most helpful advice but I hope that with enough examples it becomes at least a little more clear how things in lldb are currently done...

No it makes sense, I think I have an idea how to do it. Certainly the philosophy behind keeping them separated like this is clear to me.

lldb/cmake/modules/AddLLDB.cmake

 function(add_lldb_library name)
   include_directories(BEFORE
     ${CMAKE_CURRENT_BINARY_DIR}
 )
 
   # only supported parameters to this macro are the optional
   # MODULE;SHARED;STATIC library type and source files
   cmake_parse_arguments(PARAM
-    "MODULE;SHARED;STATIC;OBJECT;PLUGIN;FRAMEWORK;NO_INTERNAL_DEPENDENCIES"
+    "MODULE;SHARED;STATIC;OBJECT;PLUGIN;FRAMEWORK;NO_INTERNAL_DEPENDENCIES;NO_PLUGIN_DEPENDENCIES"
     "INSTALL_PREFIX;ENTITLEMENTS"
     "EXTRA_CXXFLAGS;DEPENDS;LINK_LIBS;LINK_COMPONENTS;CLANG_LIBS"
     ${ARGN})
   llvm_process_sources(srcs ${PARAM_UNPARSED_ARGUMENTS})
   list(APPEND LLVM_LINK_COMPONENTS ${PARAM_LINK_COMPONENTS})
 
   if(PARAM_NO_INTERNAL_DEPENDENCIES)
     foreach(link_lib ${PARAM_LINK_LIBS})
       if (link_lib MATCHES "^lldb")
         message(FATAL_ERROR
           "Library ${name} cannot depend on any other lldb libs "
           "(Found ${link_lib} in LINK_LIBS)")
       endif()
     endforeach()
   endif()
 
+  if(PARAM_NO_PLUGIN_DEPENDENCIES)
+    foreach(link_lib ${PARAM_LINK_LIBS})
+      if (link_lib MATCHES "^lldbPlugin")
+        message(FATAL_ERROR
+          "Library ${name} cannot depend on a plugin (Found ${link_lib} in "
+          "LINK_LIBS)")
+      endif()
+    endforeach()
+  endif()
+
   if(PARAM_PLUGIN)
     set_property(GLOBAL APPEND PROPERTY LLDB_PLUGINS ${name})
   endif()
 
   if (MSVC_IDE OR XCODE)
     string(REGEX MATCHALL "/[^/]+" split_path ${CMAKE_CURRENT_SOURCE_DIR})
     list(GET split_path -1 dir)
     file(GLOB_RECURSE headers

lldb/source/Breakpoint/CMakeLists.txt

-add_lldb_library(lldbBreakpoint
+add_lldb_library(lldbBreakpoint NO_PLUGIN_DEPENDENCIES
   Breakpoint.cpp
   BreakpointID.cpp
   BreakpointIDList.cpp
   BreakpointList.cpp
   BreakpointLocation.cpp
   BreakpointLocationCollection.cpp
   BreakpointLocationList.cpp
   BreakpointName.cpp

lldb/source/Commands/CMakeLists.txt

 lldb_tablegen(CommandOptions.inc -gen-lldb-option-defs
   SOURCE Options.td
   TARGET LLDBOptionsGen)
 
-add_lldb_library(lldbCommands
+add_lldb_library(lldbCommands NO_PLUGIN_DEPENDENCIES
   CommandCompletions.cpp
   CommandObjectApropos.cpp
   CommandObjectBreakpoint.cpp
   CommandObjectBreakpointCommand.cpp
   CommandObjectCommands.cpp
   CommandObjectDiagnostics.cpp
   CommandObjectDisassemble.cpp
   CommandObjectDWIMPrint.cpp

lldb/source/Core/CMakeLists.txt

if (LLDB_ENABLE_CURSES)
   if(LLVM_ENABLE_TERMINFO)
     list(APPEND LLDB_CURSES_LIBS ${TERMINFO_LIB})
   endif()
   if (LLVM_BUILD_STATIC)
     list(APPEND LLDB_CURSES_LIBS gpm)
   endif()
 endif()
 
+# TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbCore
 add_lldb_library(lldbCore
   Address.cpp
   AddressRange.cpp
   AddressResolver.cpp
   AddressResolverFileLine.cpp
   Communication.cpp
   DataFileCache.cpp
   Debugger.cpp

lldb/source/DataFormatters/CMakeLists.txt

-add_lldb_library(lldbDataFormatters
+add_lldb_library(lldbDataFormatters NO_PLUGIN_DEPENDENCIES
   CXXFunctionPointer.cpp
   DataVisualization.cpp
   DumpValueObjectOptions.cpp
   FormatCache.cpp
   FormatClasses.cpp
   FormatManager.cpp
   FormattersHelpers.cpp
   LanguageCategory.cpp

lldb/source/Expression/CMakeLists.txt

+# TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbExpression
 add_lldb_library(lldbExpression
   DiagnosticManager.cpp
   DWARFExpression.cpp
   DWARFExpressionList.cpp
   Expression.cpp
   ExpressionVariable.cpp
   FunctionCaller.cpp
   IRExecutionUnit.cpp

lldb/source/Host/CMakeLists.txt

 
 if (LLDB_ENABLE_LIBEDIT)
   list(APPEND LLDB_LIBEDIT_LIBS LibEdit::LibEdit)
   if (LLVM_BUILD_STATIC)
     list(APPEND LLDB_SYSTEM_LIBS gpm)
   endif()
 endif()
 
-add_lldb_library(lldbHost
+add_lldb_library(lldbHost NO_PLUGIN_DEPENDENCIES
   ${HOST_SOURCES}
 
   LINK_LIBS
     lldbUtility
     ${EXTRA_LIBS}
     ${LLDBObjCLibs}
     ${LLDB_LIBEDIT_LIBS}
 
   LINK_COMPONENTS
     Object
     Support
   )
 

lldb/source/Host/macosx/objcxx/CMakeLists.txt

 
 remove_module_flags()
 include_directories(..)
 
-add_lldb_library(lldbHostMacOSXObjCXX
+add_lldb_library(lldbHostMacOSXObjCXX NO_PLUGIN_DEPENDENCIES
   Host.mm
   HostInfoMacOSX.mm
   HostThreadMacOSX.mm
 
   LINK_LIBS
     lldbUtility
     ${EXTRA_LIBS}
 
   LINK_COMPONENTS
     Support
     TargetParser
   )
 
 target_compile_options(lldbHostMacOSXObjCXX PRIVATE -fno-objc-exceptions)

lldb/source/Interpreter/CMakeLists.txt

 lldb_tablegen(InterpreterProperties.inc -gen-lldb-property-defs
   SOURCE InterpreterProperties.td
   TARGET LLDBInterpreterPropertiesGen)
 
 lldb_tablegen(InterpreterPropertiesEnum.inc -gen-lldb-property-enum-defs
   SOURCE InterpreterProperties.td
   TARGET LLDBInterpreterPropertiesEnumGen)
 
-add_lldb_library(lldbInterpreter
+add_lldb_library(lldbInterpreter NO_PLUGIN_DEPENDENCIES
   CommandAlias.cpp
   CommandHistory.cpp
   CommandInterpreter.cpp
   CommandObject.cpp
   CommandOptionValidators.cpp
   CommandReturnObject.cpp
   OptionArgParser.cpp
   OptionGroupArchitecture.cpp

lldb/source/Symbol/CMakeLists.txt

 set(LLVM_OPTIONAL_SOURCES LocateSymbolFileMacOSX.cpp)
 
 if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
   set(PLATFORM_SOURCES
     LocateSymbolFileMacOSX.cpp
     )
 endif()
 
-add_lldb_library(lldbSymbol
+add_lldb_library(lldbSymbol NO_PLUGIN_DEPENDENCIES
   ArmUnwindInfo.cpp
   Block.cpp
   CompactUnwindInfo.cpp
   CompileUnit.cpp
   CompilerDecl.cpp
   CompilerDeclContext.cpp
   CompilerType.cpp
   DWARFCallFrameInfo.cpp

lldb/source/Target/CMakeLists.txt

 lldb_tablegen(TargetProperties.inc -gen-lldb-property-defs
   SOURCE TargetProperties.td
   TARGET LLDBTargetPropertiesGen)
 
 lldb_tablegen(TargetPropertiesEnum.inc -gen-lldb-property-enum-defs
   SOURCE TargetProperties.td
   TARGET LLDBTargetPropertiesEnumGen)
 
-add_lldb_library(lldbTarget
+add_lldb_library(lldbTarget NO_PLUGIN_DEPENDENCIES
   ABI.cpp
   AssertFrameRecognizer.cpp
   DynamicRegisterInfo.cpp
   ExecutionContext.cpp
   InstrumentationRuntime.cpp
   InstrumentationRuntimeStopInfo.cpp
   JITLoader.cpp
   JITLoaderList.cpp

lldb/source/Version/CMakeLists.txt

configure_file(
   ${version_inc})
 
 set_source_files_properties("${version_inc}"
   PROPERTIES GENERATED TRUE
              HEADER_FILE_ONLY TRUE)
 
 include_directories(${CMAKE_CURRENT_BINARY_DIR})
 
-add_lldb_library(lldbVersion
+add_lldb_library(lldbVersion NO_PLUGIN_DEPENDENCIES
   Version.cpp
   ${vcs_version_inc}
   ${version_inc})