Skip to content

Commit 7439f68

Browse files
labathgithub-actions[bot]
labath
authored and
github-actions[bot]
committed
Automerge: [lldb/cmake] Plugin layering enforcement mechanism (#144543)
Some inter-plugin dependencies are okay, others are not. Yet others not, but we're sort of stuck with them. The idea is to be able to prevent backsliding while making sure that acceptable dependencies are.. accepted. For context, see llvm/llvm-project#139170 and the attached changes to the documentation.
2 parents 7c221e5 + e7c1da7 commit 7439f68

File tree

32 files changed

+213
-0
lines changed

32 files changed

+213
-0
lines changed

‎lldb/CMakeLists.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ endif()
3737

3838
include(LLDBConfig)
3939
include(AddLLDB)
40+
include(LLDBLayeringCheck)
4041

4142
set(LLDB_INCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/include)
4243

@@ -127,6 +128,8 @@ add_subdirectory(source)
127128
add_subdirectory(tools)
128129
add_subdirectory(docs)
129130

131+
check_lldb_plugin_layering()
132+
130133
if (LLDB_ENABLE_PYTHON)
131134
if(LLDB_BUILD_FRAMEWORK)
132135
set(lldb_python_target_dir "${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}/LLDB.framework/Resources/Python/lldb")

‎lldb/cmake/modules/LLDBLayeringCheck.cmake‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
define_property(DIRECTORY PROPERTY LLDB_PLUGIN_KIND)
2+
define_property(TARGET PROPERTY LLDB_PLUGIN_KIND INHERITED)
3+
4+
define_property(DIRECTORY PROPERTY LLDB_ACCEPTABLE_PLUGIN_DEPENDENCIES)
5+
define_property(TARGET PROPERTY LLDB_ACCEPTABLE_PLUGIN_DEPENDENCIES INHERITED)
6+
7+
define_property(DIRECTORY PROPERTY LLDB_TOLERATED_PLUGIN_DEPENDENCIES)
8+
define_property(TARGET PROPERTY LLDB_TOLERATED_PLUGIN_DEPENDENCIES INHERITED)
9+
10+
option(LLDB_GENERATE_PLUGIN_DEP_GRAPH OFF)
11+
12+
function(check_lldb_plugin_layering)
13+
get_property(plugins GLOBAL PROPERTY LLDB_PLUGINS)
14+
foreach(plugin ${plugins})
15+
get_property(plugin_kind TARGET ${plugin} PROPERTY LLDB_PLUGIN_KIND)
16+
get_property(acceptable_deps TARGET ${plugin}
17+
PROPERTY LLDB_ACCEPTABLE_PLUGIN_DEPENDENCIES)
18+
get_property(tolerated_deps TARGET ${plugin}
19+
PROPERTY LLDB_TOLERATED_PLUGIN_DEPENDENCIES)
20+
21+
# A plugin is always permitted to depend on its own kind for the purposes
22+
# subclassing. Ideally the intra-kind dependencies should not form a loop,
23+
# but we're not checking that here.
24+
list(APPEND acceptable_deps ${plugin_kind})
25+
26+
list(APPEND all_plugin_kinds ${plugin_kind})
27+
28+
get_property(link_libs TARGET ${plugin} PROPERTY LINK_LIBRARIES)
29+
foreach(link_lib ${link_libs})
30+
if(link_lib IN_LIST plugins)
31+
get_property(lib_kind TARGET ${link_lib} PROPERTY LLDB_PLUGIN_KIND)
32+
if (lib_kind)
33+
if (lib_kind IN_LIST acceptable_deps)
34+
set(dep_kind green)
35+
elseif (lib_kind IN_LIST tolerated_deps)
36+
set(dep_kind yellow)
37+
else()
38+
set(dep_kind red)
39+
message(SEND_ERROR "Plugin ${plugin} cannot depend on ${lib_kind} "
40+
"plugin ${link_lib}")
41+
endif()
42+
list(APPEND dep_${dep_kind}_${plugin_kind}_${lib_kind} ${plugin})
43+
endif()
44+
endif()
45+
endforeach()
46+
endforeach()
47+
48+
if (LLDB_GENERATE_PLUGIN_DEP_GRAPH)
49+
set(dep_graph "digraph Plugins {\n")
50+
list(REMOVE_DUPLICATES all_plugin_kinds)
51+
foreach (from ${all_plugin_kinds})
52+
foreach (to ${all_plugin_kinds})
53+
foreach (dep_kind green yellow red)
54+
if (dep_${dep_kind}_${from}_${to})
55+
list(REMOVE_DUPLICATES dep_${dep_kind}_${from}_${to})
56+
string(REGEX REPLACE "lldbPlugin|${from}" "" short_deps
57+
"${dep_${dep_kind}_${from}_${to}}")
58+
string(JOIN "\n" plugins ${short_deps})
59+
string(APPEND dep_graph
60+
" ${from}->${to}[color=\"${dep_kind}\" label=\"${plugins}\"];\n")
61+
endif()
62+
endforeach()
63+
endforeach()
64+
endforeach()
65+
string(APPEND dep_graph "}\n")
66+
file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/lldb-plugin-deps.dot" "${dep_graph}")
67+
endif()
68+
endfunction()

‎lldb/docs/resources/contributing.rst‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,56 @@ subset of LLDB tests (the API tests) use a different system. Refer to the
5656
`lldb/test <https://github.com/llvm/llvm-project/tree/main/lldb/test>`_ folder
5757
for examples.
5858

59+
60+
LLDB plugins and their dependencies
61+
-----------------------------------
62+
63+
LLDB has a concept of *plugins*, which are used to provide abstraction
64+
boundaries over functionality that is specific to a certain architecture,
65+
operating system, programming language, etc. A plugin implements an abstract
66+
base class (rarely, a set of related base classes), which is a part of LLDB
67+
core. This setup allows the LLDB core to remain generic while making it possible
68+
to support for new architectures, languages, and so on. For this to work, all
69+
code needs to obey certain rules.
70+
71+
The principal rule is that LLDB core (defined as: everything under lldb/source
72+
*minus* lldb/source/Plugins) must not depend on any specific plugin. The only
73+
way it can interact with them is through the abstract interface. Explicit
74+
dependencies such as casting the base class to the plugin type are not permitted
75+
and neither are more subtle dependencies like checking the name plugin or or
76+
other situations where some code in LLDB core is tightly coupled to the
77+
implementation details of a specific plugin.
78+
79+
The rule for interaction between different plugins is more nuanced. We recognize
80+
that some cross-plugin dependencies are unavoidable or even desirable. For
81+
example, a plugin may want to extend a plugin of the same kind to
82+
add/override/refine some functionality (e.g., Android is a "kind of" Linux, but
83+
it handles some things differently). Alternatively, a plugin of one kind may
84+
want to build on the functionality offered by a specific plugin of another kind
85+
(ELFCore Process plugin uses ELF ObjectFile plugin to create a process out of an
86+
ELF core file).
87+
88+
In cases such as these, direct dependencies are acceptable. However, to keep the
89+
dependency graph manageable, we still have some rules to govern these
90+
relationships:
91+
92+
* All dependencies between plugins of the same kind must flow in the same
93+
direction (if plugin `A1` depends on plugin `B1`, then `B2` must not depend on
94+
`A2`)
95+
* Dependency graph of plugin kinds must not contain loops (dependencies like
96+
`A1->B1`, `B2->C2` and `C3->A3` are forbidden because they induce a cycle in
97+
the plugin kind graph even though the plugins themselves are acyclical)
98+
99+
100+
The first of these rules is checked via CMake scripts (using the
101+
`LLDB_ACCEPTABLE_PLUGIN_DEPENDENCIES` property). Dependencies in this category
102+
are expected and permitted (subject to other constraints such as that dependency
103+
making sense for the particular pair of plugins). Unfortunately, due to historic
104+
reasons, not all plugin dependencies follow this rule, which is why we have
105+
another category called `LLDB_TOLERATED_PLUGIN_DEPENDENCIES`. New dependencies
106+
are forbidden (even though they are accepted by CMake) and existing ones should
107+
be removed whereever possible.
108+
59109
.. _Error handling:
60110

61111
Error handling and use of assertions in LLDB

‎lldb/source/Plugins/ABI/CMakeLists.txt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
set_property(DIRECTORY PROPERTY LLDB_PLUGIN_KIND ABI)
2+
set_property(DIRECTORY PROPERTY LLDB_TOLERATED_PLUGIN_DEPENDENCIES
3+
ProcessUtility
4+
TypeSystem
5+
)
6+
17
foreach(target AArch64 ARM ARC Hexagon LoongArch Mips MSP430 PowerPC RISCV SystemZ X86)
28
if (${target} IN_LIST LLVM_TARGETS_TO_BUILD)
39
add_subdirectory(${target})

‎lldb/source/Plugins/Architecture/CMakeLists.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
set_property(DIRECTORY PROPERTY LLDB_PLUGIN_KIND Architecture)
2+
13
add_subdirectory(Arm)
24
add_subdirectory(Mips)
35
add_subdirectory(PPC64)

‎lldb/source/Plugins/Disassembler/CMakeLists.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,3 @@
1+
set_property(DIRECTORY PROPERTY LLDB_PLUGIN_KIND Disassembler)
2+
13
add_subdirectory(LLVMC)

‎lldb/source/Plugins/DynamicLoader/CMakeLists.txt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
set_property(DIRECTORY PROPERTY LLDB_PLUGIN_KIND DynamicLoader)
2+
set_property(DIRECTORY PROPERTY LLDB_ACCEPTABLE_PLUGIN_DEPENDENCIES ObjectFile)
3+
set_property(DIRECTORY PROPERTY LLDB_TOLERATED_PLUGIN_DEPENDENCIES
4+
Process # part of a loop (Process<->DynamicLoader).
5+
TypeSystem
6+
)
7+
18
add_subdirectory(Darwin-Kernel)
29
add_subdirectory(FreeBSD-Kernel)
310
add_subdirectory(MacOSX-DYLD)

‎lldb/source/Plugins/ExpressionParser/CMakeLists.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,3 @@
1+
set_property(DIRECTORY PROPERTY LLDB_PLUGIN_KIND ExpressionParser)
2+
13
add_subdirectory(Clang)

‎lldb/source/Plugins/Instruction/CMakeLists.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
set_property(DIRECTORY PROPERTY LLDB_PLUGIN_KIND Instruction)
2+
13
add_subdirectory(ARM)
24
add_subdirectory(ARM64)
35
add_subdirectory(LoongArch)

‎lldb/source/Plugins/InstrumentationRuntime/CMakeLists.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
set_property(DIRECTORY PROPERTY LLDB_PLUGIN_KIND InstrumentationRuntime)
2+
13
add_subdirectory(ASan)
24
add_subdirectory(ASanLibsanitizers)
35
add_subdirectory(MainThreadChecker)

0 commit comments

Comments
 (0)