Skip to content

Commit 46e1e9f

Browse files
labathlabath
authored
Reapply "[lldb/cmake] Plugin layering enforcement mechanism (#144543)" (#145305)
The only difference from the original PR are the added BRIEF and FULL_DOCS arguments to define_property, which are required for cmake<3.23.
1 parent a201f88 commit 46e1e9f

File tree

32 files changed

+221
-0
lines changed

32 files changed

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