[lldb] Upstream lldb-rpc-gen and LLDB RPC server-side emitters #136748
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-lldb Author: Chelsea Cassanova (chelcassanova) ChangesThis commit upstreams lldb-rpc-gen, a tool that emits the client and server interfaces used for LLDB RPC. This is the initial commit in the upstreaming process for LLDB RPC. lldb-rpc-gen is a ClangTool that reads the LLDB SB API headers and uses their information to generate the interfaces for RPC. This commit specifically adds the server-side emitters for easier review. The client-side interface will be added in a later commit. RFC: https://discourse.llvm.org/t/rfc-upstreaming-lldb-rpc/85804 Patch is 92.84 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/136748.diff 15 Files Affected:
diff --git a/lldb/scripts/convert-lldb-header-to-rpc-header.py b/lldb/scripts/convert-lldb-header-to-rpc-header.py
new file mode 100755
index 0000000000000..bf1eb526e8b3a
--- /dev/null
+++ b/lldb/scripts/convert-lldb-header-to-rpc-header.py
@@ -0,0 +1,50 @@
+#!/usr/bin/env python3
+# Usage: convert-lldb-header-to-rpc-header.py <path/to/input-header.h> <path/to/output-header.h>
+
+import argparse
+import os
+import re
+import subprocess
+
+def main():
+ parser = argparse.ArgumentParser()
+ parser.add_argument("input")
+ parser.add_argument("output")
+ args = parser.parse_args()
+ input_path = str(args.input)
+ output_path = str(args.output)
+ with open(input_path, "r+") as input_file:
+ lines = input_file.readlines()
+
+ with open(output_path, "w+") as output_file:
+ for line in lines:
+ # NOTE: We do not use lldb-forward or lldb-versioning in RPC.
+ if re.match(r'#include "lldb/lldb-forward|#include "lldb/lldb-versioning', line):
+ continue
+ # For lldb-defines.h
+ elif re.match(r'.+LLDB_LLDB_', line):
+ output_file.write(re.sub(r'LLDB_LLDB_', r'LLDB_RPC_', line))
+ # For SBDefines.h
+ elif re.match(r'.+LLDB_API_', line):
+ output_file.write(re.sub(r'LLDB_API_', r'LLDB_RPC_API_', line))
+ # For lldb-rpc-version.h and lldb-rpc-defines.h
+ elif re.match(r'.+LLDB_VERSION', line):
+ output_file.write(re.sub(r'LLDB_VERSION', r'LLDB_RPC_VERSION', line))
+ elif re.match(r'.+LLDB_REVISION', line):
+ output_file.write(re.sub(r'LLDB_REVISION', r'LLDB_RPC_REVISION', line))
+ elif re.match(r'.+LLDB_VERSION_STRING', line):
+ output_file.write(re.sub(r'LLDB_VERSION_STRING', r'LLDB_RPC_VERSION_STRING', line))
+ # For local #includes
+ elif re.match(r'#include "lldb/lldb-', line):
+ output_file.write(re.sub(r'lldb/lldb-', r'lldb-rpc-', line))
+ # Rename the lldb namespace to lldb-rpc
+ elif re.match(r'namespace lldb', line):
+ output_file.write(re.sub(r'lldb', r'lldb_rpc', line))
+ # Rename namespace references
+ elif re.match(r'.+lldb::', line):
+ output_file.write(re.sub(r'lldb::', r'lldb_rpc::', line))
+ else:
+ # Write any line that doesn't need to be converted
+ output_file.write(line)
+if __name__ == "__main__":
+ main()
diff --git a/lldb/scripts/framework-header-include-fix.py b/lldb/scripts/framework-header-include-fix.py
new file mode 100755
index 0000000000000..fcca7f31b2954
--- /dev/null
+++ b/lldb/scripts/framework-header-include-fix.py
@@ -0,0 +1,30 @@
+#!/usr/bin/env python3
+# Usage: framework-header-include-fix.py <path/to/input-header.h> <path/to/output-header.h>
+
+import argparse
+import os
+import re
+import subprocess
+
+def main():
+ parser = argparse.ArgumentParser()
+ parser.add_argument("input")
+ parser.add_argument("output")
+ args = parser.parse_args()
+ input_path = str(args.input)
+ output_path = str(args.output)
+ with open(input_path, "r+") as input_file:
+ lines = input_file.readlines()
+
+ with open(output_path, "w+") as output_file:
+ for line in lines:
+ if re.match(r'.+<lldb-rpc/common', line):
+ output_file.write(re.sub(r'<lldb-rpc/common', r'<LLDBRPC', line))
+ elif re.match(r'#include "(.*)"', line):
+ include_filename = re.search(r'#include "(.*)"', line).groups()[0]
+ output_file.write(re.sub(r'#include "(.*)"', r'#include <LLDBRPC/' + include_filename + '>', line))
+ else:
+ # Write any line that doesn't need to be converted
+ output_file.write(line)
+if __name__ == "__main__":
+ main()
diff --git a/lldb/scripts/framework-header-version-fix.py b/lldb/scripts/framework-header-version-fix.py
new file mode 100755
index 0000000000000..d2417ea4f9414
--- /dev/null
+++ b/lldb/scripts/framework-header-version-fix.py
@@ -0,0 +1,38 @@
+#!/usr/bin/env python3
+# Usage: framework-header-version-fix.py <path/to/input-header.h> <path/to/output-header.h> MAJOR MINOR PATCH
+
+import argparse
+import os
+import re
+import subprocess
+
+def main():
+ parser = argparse.ArgumentParser()
+ parser.add_argument("input")
+ parser.add_argument("output")
+ parser.add_argument("lldb_version_major")
+ parser.add_argument("lldb_version_minor")
+ parser.add_argument("lldb_version_patch")
+ args = parser.parse_args()
+ input_path = str(args.input)
+ output_path = str(args.output)
+ lldb_version_major = str(args.lldb_version_major)
+ lldb_version_minor = str(args.lldb_version_minor)
+ lldb_version_patch = str(args.lldb_version_patch)
+
+ with open(input_path, "r+") as input_file:
+ lines = input_file.readlines()
+
+ with open(output_path, "w+") as output_file:
+ for line in lines:
+ if re.match(r'//#define LLDB_RPC_VERSION$', line):
+ output_file.write(re.sub(r'//#define LLDB_RPC_VERSION', r'#define LLDB_RPC_VERSION ' + lldb_version_major, line))
+ elif re.match(r'//#define LLDB_RPC_REVISION$', line):
+ output_file.write(re.sub(r'//#define LLDB_RPC_REVISION', r'#define LLDB_RPC_REVISION ' + lldb_version_minor, line))
+ elif re.match(r'//#define LLDB_RPC_VERSION_STRING$', line):
+ output_file.write(re.sub(r'//#define LLDB_RPC_VERSION_STRING', r'#define LLDB_RPC_VERSION_STRING "{0}.{1}.{2}"'.format(lldb_version_major, lldb_version_minor, lldb_version_patch), line))
+ else:
+ # Write any line that doesn't need to be converted
+ output_file.write(line)
+if __name__ == "__main__":
+ main()
diff --git a/lldb/tools/CMakeLists.txt b/lldb/tools/CMakeLists.txt
index 6804dc234555b..6a2859d042148 100644
--- a/lldb/tools/CMakeLists.txt
+++ b/lldb/tools/CMakeLists.txt
@@ -7,6 +7,8 @@ add_subdirectory(intel-features)
# example is `check-lldb`. So, we pass EXCLUDE_FROM_ALL here.
add_subdirectory(lldb-test EXCLUDE_FROM_ALL)
add_subdirectory(lldb-fuzzer EXCLUDE_FROM_ALL)
+add_subdirectory(lldb-rpc EXCLUDE_FROM_ALL)
+
add_lldb_tool_subdirectory(lldb-instr)
add_lldb_tool_subdirectory(lldb-dap)
diff --git a/lldb/tools/lldb-rpc/CMakeLists.txt b/lldb/tools/lldb-rpc/CMakeLists.txt
new file mode 100644
index 0000000000000..ab1e8517b5909
--- /dev/null
+++ b/lldb/tools/lldb-rpc/CMakeLists.txt
@@ -0,0 +1,19 @@
+if(LLDB_CODESIGN_IDENTITY)
+ # Use explicit LLDB identity
+ set(LLVM_CODESIGNING_IDENTITY ${LLDB_CODESIGN_IDENTITY})
+else()
+ # Use explicit LLVM identity or default to ad-hoc signing if empty
+ if(NOT LLVM_CODESIGNING_IDENTITY)
+ set(LLVM_CODESIGNING_IDENTITY -)
+ endif()
+endif()
+
+check_cxx_compiler_flag("-Wno-gnu-zero-variadic-macro-arguments"
+ CXX_SUPPORTS_NO_GNU_ZERO_VARIADIC_MACRO_ARGUMENTS)
+
+# LLDBRPCGeneration.cmake needs the LLDB_RPC_GEN_EXE variable
+# which gets defined in the lldb-rpc-gen folder, so we're adding
+# this folder before we add that file.
+add_lldb_tool_subdirectory(lldb-rpc-gen)
+include(${CMAKE_CURRENT_SOURCE_DIR}/LLDBRPCGeneration.cmake)
+include(${CMAKE_CURRENT_SOURCE_DIR}/LLDBRPCHeaders.cmake)
diff --git a/lldb/tools/lldb-rpc/LLDBRPCGeneration.cmake b/lldb/tools/lldb-rpc/LLDBRPCGeneration.cmake
new file mode 100644
index 0000000000000..c0a041f3e96b7
--- /dev/null
+++ b/lldb/tools/lldb-rpc/LLDBRPCGeneration.cmake
@@ -0,0 +1,95 @@
+if (NOT DEFINED LLDB_RPC_GEN_EXE)
+ message(FATAL_ERROR
+ "Unable to generate lldb-rpc sources because LLDB_RPC_GEN_EXE is not
+ defined. If you are cross-compiling, please build lldb-rpc-gen for your host
+ platform.")
+endif()
+set(lldb_rpc_generated_dir "${CMAKE_CURRENT_BINARY_DIR}/generated")
+set(lldb_rpc_server_generated_source_dir "${lldb_rpc_generated_dir}/server")
+set(lldb_rpc_lib_generated_source_dir "${lldb_rpc_generated_dir}/lib")
+set(lldb_rpc_client_generated_source_dir "${lldb_rpc_generated_dir}/client")
+
+file(GLOB api_headers ${LLDB_SOURCE_DIR}/include/lldb/API/SB*.h)
+# We don't generate SBCommunication
+list(REMOVE_ITEM api_headers ${LLDB_SOURCE_DIR}/include/lldb/API/SBCommunication.h)
+# SBDefines.h is mostly definitions and forward declarations, nothing to
+# generate.
+list(REMOVE_ITEM api_headers ${LLDB_SOURCE_DIR}/include/lldb/API/SBDefines.h)
+
+# Generate the list of byproducts. Note that we cannot just glob the files in
+# the directory with the generated sources because BYPRODUCTS needs to be known
+# at configure time but the files are generated at build time.
+set(lldb_rpc_gen_byproducts
+ ${lldb_rpc_generated_dir}/SBClasses.def
+ ${lldb_rpc_generated_dir}/SBAPI.def
+ ${lldb_rpc_generated_dir}/lldb.py
+ ${lldb_rpc_server_generated_source_dir}/SBAPI.h
+ ${lldb_rpc_lib_generated_source_dir}/LLDBRPC.h
+)
+
+set(lldb_rpc_gen_server_impl_files)
+set(lldb_rpc_gen_lib_header_files)
+set(lldb_rpc_gen_lib_impl_files)
+foreach(path ${api_headers})
+ get_filename_component(filename_no_ext ${path} NAME_WLE)
+
+ set(server_header_file "Server_${filename_no_ext}.h")
+ list(APPEND lldb_rpc_gen_byproducts "${lldb_rpc_server_generated_source_dir}/${server_header_file}")
+
+ set(server_impl_file "Server_${filename_no_ext}.cpp")
+ list(APPEND lldb_rpc_gen_byproducts "${lldb_rpc_server_generated_source_dir}/${server_impl_file}")
+ list(APPEND lldb_rpc_gen_server_impl_files "${lldb_rpc_server_generated_source_dir}/${server_impl_file}")
+
+ set(lib_header_file "${filename_no_ext}.h")
+ list(APPEND lldb_rpc_gen_byproducts "${lldb_rpc_lib_generated_source_dir}/${lib_header_file}")
+ list(APPEND lldb_rpc_gen_lib_header_files "${lldb_rpc_lib_generated_source_dir}/${lib_header_file}")
+
+ set(lib_impl_file "${filename_no_ext}.cpp")
+ list(APPEND lldb_rpc_gen_byproducts "${lldb_rpc_lib_generated_source_dir}/${lib_impl_file}")
+ list(APPEND lldb_rpc_gen_lib_impl_files "${lldb_rpc_lib_generated_source_dir}/${lib_impl_file}")
+endforeach()
+list(APPEND lldb_rpc_gen_lib_impl_files "${CMAKE_CURRENT_BINARY_DIR}/generated/lib/RPCClientSideCallbacks.cpp")
+list(APPEND lldb_rpc_gen_lib_header_files "${lldb_rpc_lib_generated_source_dir}/LLDBRPC.h")
+
+# Make sure that the clang-resource-dir is set correctly or else the tool will
+# fail to run. This is only needed when we do a standalone build.
+set(clang_resource_dir_arg)
+if (LLDB_BUILT_STANDALONE)
+ if (TARGET clang-resource-headers)
+ set(clang_resource_headers_dir
+ $<TARGET_PROPERTY:clang-resource-headers,INTERFACE_INCLUDE_DIRECTORIES>)
+ set(clang_resource_dir_arg --extra-arg="-resource-dir=${clang_resource_headers_dir}/..")
+ else()
+ set(clang_resource_dir_arg --extra-arg="-resource-dir=${LLDB_EXTERNAL_CLANG_RESOURCE_DIR}")
+ endif()
+endif()
+
+add_custom_command(OUTPUT ${lldb_rpc_gen_byproducts}
+ COMMAND ${CMAKE_COMMAND} -E make_directory
+ ${lldb_rpc_generated_dir}
+
+ COMMAND ${CMAKE_COMMAND} -E make_directory
+ ${lldb_rpc_server_generated_source_dir}
+
+ COMMAND ${CMAKE_COMMAND} -E make_directory
+ ${lldb_rpc_lib_generated_source_dir}
+
+ COMMAND ${CMAKE_COMMAND} -E make_directory
+ ${lldb_rpc_client_generated_source_dir}
+
+ COMMAND ${LLDB_RPC_GEN_EXE}
+ -p ${CMAKE_BINARY_DIR}
+ --output-dir=${lldb_rpc_generated_dir}
+ ${clang_resource_dir_arg}
+ --extra-arg="-USWIG"
+ ${api_headers}
+
+ DEPENDS ${LLDB_RPC_GEN_EXE} ${api_headers}
+ COMMENT "Generating sources for lldb-rpc-server..."
+ WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
+)
+
+add_custom_target(lldb-rpc-generate-sources
+ DEPENDS
+ ${lldb_rpc_gen_byproducts}
+ lldb-sbapi-dwarf-enums)
diff --git a/lldb/tools/lldb-rpc/LLDBRPCHeaders.cmake b/lldb/tools/lldb-rpc/LLDBRPCHeaders.cmake
new file mode 100644
index 0000000000000..7a066caceb9cb
--- /dev/null
+++ b/lldb/tools/lldb-rpc/LLDBRPCHeaders.cmake
@@ -0,0 +1,119 @@
+set(derived_headers_location "${CMAKE_CURRENT_BINARY_DIR}/DerivedHeaders")
+set(original_headers_location "${LLDB_SOURCE_DIR}/include/lldb")
+set(headers_to_process
+ API/SBDefines.h
+ lldb-defines.h
+ lldb-enumerations.h
+ lldb-types.h
+)
+
+file(MAKE_DIRECTORY ${derived_headers_location})
+
+set(original_headers)
+set(derived_headers)
+foreach(header ${headers_to_process})
+ set(original_header "${original_headers_location}/${header}")
+
+ get_filename_component(header_filename ${header} NAME)
+ string(REPLACE "lldb-" "lldb-rpc-" rpc_header_filename "${header_filename}")
+ set(derived_header "${derived_headers_location}/${rpc_header_filename}")
+
+ list(APPEND original_headers "${original_header}")
+ list(APPEND derived_headers "${derived_header}")
+ add_custom_command(OUTPUT ${derived_header}
+ COMMAND ${CMAKE_COMMAND} -E copy ${original_header} ${derived_header}
+ COMMAND ${LLDB_SOURCE_DIR}/scripts/convert-lldb-header-to-rpc-header.sh
+ ${derived_header}
+ DEPENDS ${original_header}
+
+ COMMENT "Creating ${derived_header}"
+ )
+endforeach()
+
+set(generated_headers_to_process
+ API/SBLanguages.h
+)
+foreach(header ${generated_headers_to_process})
+ set(original_header "${LLDB_OBJ_DIR}/include/lldb/${header}")
+
+ get_filename_component(header_filename ${header} NAME)
+ string(REPLACE "lldb-" "lldb-rpc-" rpc_header_filename "${header_filename}")
+ set(derived_header "${derived_headers_location}/${rpc_header_filename}")
+
+ list(APPEND original_headers "${original_header}")
+ list(APPEND derived_headers "${derived_header}")
+ add_custom_command(OUTPUT ${derived_header}
+ COMMAND ${CMAKE_COMMAND} -E copy ${original_header} ${derived_header}
+ COMMAND ${LLDB_SOURCE_DIR}/scripts/convert-lldb-header-to-rpc-header.sh
+ ${derived_header}
+ DEPENDS lldb-sbapi-dwarf-enums
+
+ COMMENT "Creating ${derived_header}"
+ )
+endforeach()
+
+
+add_custom_target(copy-aux-rpc-headers DEPENDS ${derived_headers})
+
+set(public_headers ${lldb_rpc_gen_lib_header_files})
+list(APPEND public_headers
+ ${derived_headers_location}/SBDefines.h
+ ${derived_headers_location}/SBLanguages.h
+ ${derived_headers_location}/lldb-rpc-enumerations.h
+ ${derived_headers_location}/lldb-rpc-types.h
+)
+
+# Collect and preprocess headers for the framework bundle
+set(version_header
+ ${derived_headers_location}/lldb-rpc-defines.h
+)
+
+function(FixIncludePaths in subfolder out)
+ get_filename_component(base_name ${in} NAME)
+ set(parked_header ${CMAKE_CURRENT_BINARY_DIR}/ParkedHeaders/${subfolder}/${base_name})
+ set(${out} ${parked_header} PARENT_SCOPE)
+
+ add_custom_command(OUTPUT ${parked_header}
+ COMMAND ${CMAKE_COMMAND} -E copy
+ ${in} ${parked_header}
+ COMMAND ${LLDB_SOURCE_DIR}/scripts/framework-header-include-fix.sh
+ ${parked_header}
+ DEPENDS ${in}
+ COMMENT "Fixing includes in ${in}"
+ )
+endfunction()
+
+function(FixVersions in subfolder out)
+ get_filename_component(base_name ${in} NAME)
+ set(parked_header ${CMAKE_CURRENT_BINARY_DIR}/ParkedHeaders/${subfolder}/${base_name})
+ set(${out} ${parked_header} PARENT_SCOPE)
+
+ add_custom_command(OUTPUT ${parked_header}
+ COMMAND ${CMAKE_COMMAND} -E copy ${in} ${parked_header}
+ COMMAND ${LLDB_SOURCE_DIR}/scripts/framework-header-version-fix.sh
+ ${parked_header} ${LLDB_VERSION_MAJOR} ${LLDB_VERSION_MINOR} ${LLDB_VERSION_PATCH}
+ DEPENDS ${in}
+ COMMENT "Fixing versions in ${liblldbrpc_version_header}"
+ )
+endfunction()
+
+set(preprocessed_headers)
+
+# Apply include-paths fix on all headers and park them.
+foreach(source_header ${public_headers})
+ FixIncludePaths(${source_header} Headers parked_header)
+ list(APPEND preprocessed_headers ${parked_header})
+endforeach()
+
+# Apply include-paths fix and stage in parent directory.
+# Then apply version fix and park together with all the others.
+FixIncludePaths(${version_header} ".." staged_header)
+FixVersions(${staged_header} Headers parked_header)
+list(APPEND preprocessed_headers ${parked_header})
+
+# Wrap header preprocessing in a target, so liblldbrpc can depend on.
+add_custom_target(liblldbrpc-headers DEPENDS ${preprocessed_headers})
+add_dependencies(liblldbrpc-headers copy-aux-rpc-headers)
+set_target_properties(liblldbrpc-headers PROPERTIES
+ LIBRARY_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/ParkedHeaders
+)
diff --git a/lldb/tools/lldb-rpc/lldb-rpc-gen/CMakeLists.txt b/lldb/tools/lldb-rpc/lldb-rpc-gen/CMakeLists.txt
new file mode 100644
index 0000000000000..bc1be9865e159
--- /dev/null
+++ b/lldb/tools/lldb-rpc/lldb-rpc-gen/CMakeLists.txt
@@ -0,0 +1,23 @@
+add_lldb_tool(lldb-rpc-gen
+ RPCCommon.cpp
+ RPCServerHeaderEmitter.cpp
+ RPCServerSourceEmitter.cpp
+ lldb-rpc-gen.cpp
+
+ CLANG_LIBS
+ clangAST
+ clangBasic
+ clangCodeGen
+ clangFrontend
+ clangLex
+ clangRewrite
+ clangSerialization
+ clangTooling
+
+ LINK_COMPONENTS
+ Support
+ )
+
+if (NOT DEFINED LLDB_RPC_GEN_EXE)
+ set(LLDB_RPC_GEN_EXE $<TARGET_FILE:lldb-rpc-gen> CACHE STRING "Executable that generates lldb-rpc-server")
+endif()
diff --git a/lldb/tools/lldb-rpc/lldb-rpc-gen/RPCCommon.cpp b/lldb/tools/lldb-rpc/lldb-rpc-gen/RPCCommon.cpp
new file mode 100644
index 0000000000000..821386644b548
--- /dev/null
+++ b/lldb/tools/lldb-rpc/lldb-rpc-gen/RPCCommon.cpp
@@ -0,0 +1,532 @@
+//===-- RPCCommon.cpp -----------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "RPCCommon.h"
+
+#include "clang/AST/AST.h"
+#include "clang/AST/Mangle.h"
+#include "clang/Lex/Lexer.h"
+
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace clang;
+
+// We intentionally do not generate some classes because they are currently
+// inconvenient, they aren't really used by most consumers, or we're not sure
+// why they exist.
+static constexpr llvm::StringRef DisallowedClasses[] = {
+ "SBCommunication", // What is this used for?
+ "SBInputReader", // What is this used for?
+ "SBCommandPluginInterface", // This is hard to support, we can do it if
+ // really needed though.
+ "SBCommand", // There's nothing too difficult about this one, but many of
+ // its methods take a SBCommandPluginInterface pointer so
+ // there's no reason to support this.
+};
+
+// We intentionally avoid generating certain methods either because they are
+// difficult to support correctly or they aren't really used much from C++.
+// FIXME: We should be able to annotate these methods instead of maintaining a
+// list in the generator itself.
+static constexpr llvm::StringRef DisallowedMethods[] = {
+ // The threading functionality in SBHostOS is deprecated and thus we do not
+ // generate them. It would be ideal to add the annotations to the methods
+ // and then support not generating deprecated methods. However, without
+ // annotations the generator generates most things correctly. This one is
+ // problematic because it returns a pointer to an "opaque" structure
+ // (thread_t) that is not `void *`, so special casing it is more effort than
+ // it's worth.
+ "_ZN4lldb8SBHostOS10ThreadJoinEP17_opaque_pthread_tPPvPNS_7SBErrorE",
+ "_ZN4lldb8SBHostOS12ThreadCancelEP17_opaque_pthread_tPNS_7SBErrorE",
+ "_ZN4lldb8SBHostOS12ThreadCreateEPKcPFPvS3_ES3_PNS_7SBErrorE",
+ "_ZN4lldb8SBHostOS12ThreadDetachEP17_opaque_pthread_tPNS_7SBErrorE",
+ "_ZN4lldb8SBHostOS13ThreadCreatedEPKc",
+};
+
+static constexpr llvm::StringRef ClassesWithoutDefaultCtor[] = {
+ "SBHostOS",
+ "SBReproducer",
+};
+
+static constexpr llvm::StringRef ClassesWithoutCopyOperations[] = {
+ "SBHostOS",
+ "SBReproducer",
+ "SBStream",
+ "SBProgress",
+};
+
+static constexpr llvm::StringRef MethodsWithPointerPlusLen[] = {
+ "_ZN4lldb6SBData11ReadRawDataERNS_7SBErrorEyPvm",
+ "_ZN4lldb6SBData7SetDataERNS_7SBErrorEPKvmNS_9ByteOrderEh",
+ "_ZN4lldb6SBData20SetDataWithOwnershipERNS_7SBErrorEPKvmNS_9ByteOrderEh",
+ "_ZN...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
✅ With the latest revision this PR passed the Python code formatter. |
chelcassanova
force-pushed
the
lldb-rpc/upstream-lldb-rpc-gen
branch
2 times, most recently
from
ded625a to
dd04b9d
Compare
chelcassanova
changed the title
chelcassanova
force-pushed
the
lldb-rpc/upstream-lldb-rpc-gen
branch
from
dd04b9d to
28266f9
Compare
|
A few failures in Linux CI. All are like: |
| @@ -0,0 +1,13 @@ | |||
| // Skipping temporarily due to rdar://149500008 | |||
There was a problem hiding this comment.
Not a great sign that all the client tests are disabled on the system that they're originally intended for.
What exactly is the issue here?
There was a problem hiding this comment.
This is probably just an artifact of the downstream fork, I believe @chelcassanova has a fix for this already
There was a problem hiding this comment.
The issue is that the tests need the clang resource dir in order to run properly (since lldb-rpc-gen is a ClangTool). They were originally skipped so that I could add that dir to LLDB lit here: #136761
There was a problem hiding this comment.
This is a lot to deal with. So I have a bunch of nits on the first few files but my lack of overall understanding of the process meant I can't assess 1000s of lines of C++ implementing it.
I know you explained the mechanics of this in the RFC, but, I think we should have a page on the website about it eventually anyway, and a design doc would really help review this too.
So could you draft something like that? Separate PR from this.
For now I'd just be looking for:
- The use case of LLDB-RPC
- The steps taken to generate the components of it, and what they should produce.
That can remain in review while you upstream the rest of the code, and then we can think of any other sections to add before landing it. E.g. how to maintain LLDB-RPC and so on.
| args = parser.parse_args() | ||
| input_path = str(args.input) | ||
| output_path = str(args.output) | ||
| with open(input_path, "r+") as input_file: |
There was a problem hiding this comment.
Could/should this be opened read only?
There was a problem hiding this comment.
This should be read only, the general pattern that I'm doing with these scripts is to readlines() the input file and make all changes in the output file so I shouldn't need to write to the input file here.
| with open(input_path, "r+") as input_file: | ||
| lines = input_file.readlines() | ||
|
|
||
| with open(output_path, "w+") as output_file: |
There was a problem hiding this comment.
Should this be opened for writing only?
There was a problem hiding this comment.
Yes, I'll change this.
|
|
||
| with open(output_path, "w+") as output_file: | ||
| for line in lines: | ||
| # NOTE: We do not use lldb-forward or lldb-versioning in RPC. |
There was a problem hiding this comment.
These are headers, right? If so:
# NOTE: We do not use lldb-forward.h or lldb-versioning.h in RPC.
For clarity.
| output_path = str(args.output) | ||
| with open(input_path, "r+") as input_file: | ||
| lines = input_file.readlines() | ||
|
|
There was a problem hiding this comment.
Add a comment stating the purpose of this whole loop. It's replacing things, a lot of things, but it's hard to see the overall picture.
There was a problem hiding this comment.
Also a comment at the top of the script stating what it does would be good. The usage is nice, but not enough.
There was a problem hiding this comment.
I'll add this, ditto to adding comments for what each of these scripts do.
| @@ -0,0 +1,39 @@ | |||
| #!/usr/bin/env python3 | |||
| # Usage: framework-header-include-fix.py <path/to/input-header.h> <path/to/output-header.h> | |||
There was a problem hiding this comment.
Add a short description of what this script does. Mainly, what is it fixing specifically?
| args = parser.parse_args() | ||
| input_path = str(args.input) | ||
| output_path = str(args.output) | ||
| with open(input_path, "r+") as input_file: |
There was a problem hiding this comment.
See if you need r+ or would r do. Same for w+ below.
lldb/tools/lldb-rpc/CMakeLists.txt
Outdated
| endif() | ||
| endif() | ||
|
|
||
| check_cxx_compiler_flag("-Wno-gnu-zero-variadic-macro-arguments" |
There was a problem hiding this comment.
What is this used for / supposed to be used for? Because you check the compiler flag works but then don't appear to do anything with it.
Is something later looking for CXX_SUPPORTS_NO_GNU_ZERO_VARIADIC_MACRO_ARGUMENTS?
There was a problem hiding this comment.
The CXX_SUPPORTS_NO_GNU_ZERO_VARIADIC_MACRO_ARGUMENTS variable is used later on in files that haven't been upstreamed yet (If this variable is set then the -Wno-gnu-zero-variadic-macro-arguments gets added as a compile option). For simplicity's sake, this line could be removed until those files are upstreamed.
| // why they exist. | ||
| static constexpr llvm::StringRef DisallowedClasses[] = { | ||
| "SBCommunication", // What is this used for? | ||
| "SBInputReader", // What is this used for? |
There was a problem hiding this comment.
Now this is upstream, can we answer these questions? I have zero idea either :)
| "_ZN4lldb8SBHostOS12ThreadCancelEP17_opaque_pthread_tPNS_7SBErrorE", | ||
| "_ZN4lldb8SBHostOS12ThreadCreateEPKcPFPvS3_ES3_PNS_7SBErrorE", | ||
| "_ZN4lldb8SBHostOS12ThreadDetachEP17_opaque_pthread_tPNS_7SBErrorE", | ||
| "_ZN4lldb8SBHostOS13ThreadCreatedEPKc", |
There was a problem hiding this comment.
All these mangled names, is the mangling stable and how stable across compilers and platforms? Is GCC on Linux going to give different results for example?
There was a problem hiding this comment.
I believe that the mangling should be stable on platforms using Clang. Since this is a ClangTool itself, I'm actually currently unsure as to how this would compile using GCC.
By the way, we can continue this line of discussion on one of the smaller patches I put up to upstream this tool and its shared common code: #138031
| "SBValueList", | ||
| }; | ||
|
|
||
| static llvm::StringMap<llvm::SmallVector<llvm::StringRef>> |
Absolutely! I know this is a lot of code to review, so anything that can be done to break all this down is more than something I'm willing to do. I can write up a design doc in order to better explain exactly what you're looking at here. |
chelcassanova
force-pushed
the
lldb-rpc/upstream-lldb-rpc-gen
branch
from
28266f9 to
ec76377
Compare
This commit upstreams lldb-rpc-gen, a tool that emits the client and server interfaces used for LLDB RPC. This is the initial commit in the upstreaming process for LLDB RPC. lldb-rpc-gen is a ClangTool that reads the LLDB SB API headers and uses their information to generate the interfaces for RPC. This commit specifically adds the server-side emitters for easier review. The client-side interface will be added in a later commit. RFC: https://discourse.llvm.org/t/rfc-upstreaming-lldb-rpc/85804
chelcassanova
force-pushed
the
lldb-rpc/upstream-lldb-rpc-gen
branch
from
ec76377 to
9ab6fdf
Compare
|
Looking at this patch again, I think this can actually be split into 4 patches:
That should make this easier to review. @DavidSpickett I can still draft up a design doc, but for the sake of readability I'm also going to split this patch up as well. |
|
I split this large patch up into 3 smaller ones: This current patch itself is very very large, so I'll close it and let discussion continue on the above 3. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit upstreams lldb-rpc-gen, a tool that emits the client and server interfaces used for LLDB RPC. This is the initial commit in the upstreaming process for LLDB RPC. lldb-rpc-gen is a ClangTool that reads the LLDB SB API headers and uses their information to generate the interfaces for RPC. This commit specifically adds the server-side emitters for easier review. The client-side interface will be added in a later commit.
RFC: https://discourse.llvm.org/t/rfc-upstreaming-lldb-rpc/85804