[Lldb-commits] [lldb] [lldb] Fix Windows build with dynamic Python plugin (PR #200843)

[via lldb-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Nerixyz (Nerixyz)

<details>
<summary>Changes</summary>

When building with `LLDB_ENABLE_DYNAMIC_SCRIPTINTERPRETERS` on Windows, there were a few issues:

1. Building `lldb` didn't build the shared library for the script interpreter. This was fixed by adding a dependency on `lldbPluginScriptInterpreterPython`. I'm not sure how this works on other platforms.
2. The library failed to link, because symbols for LLVMSupport were missing. I added `Support` as a dependency. Are the symbols resolved through liblldb on other platforms?
3. After adding Support, the library failed to find `lldb_private::File::kInvalidDescriptor`. The fix is rather hacky, because it declares the static as `constexpr static`. Again, my guess is that this resolves through liblldb on other platforms where symbols are always exported.

With this, you can build lldb, but it still fails to load the plugin. I'll fix that in a followup.

---
Full diff: https://github.com/llvm/llvm-project/pull/200843.diff


4 Files Affected:

- (modified) lldb/bindings/python/CMakeLists.txt (+3) 
- (modified) lldb/include/lldb/Host/FileBase.h (+1-1) 
- (modified) lldb/source/Host/common/File.cpp (-1) 
- (modified) lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt (+10) 


``````````diff
diff --git a/lldb/bindings/python/CMakeLists.txt b/lldb/bindings/python/CMakeLists.txt
index 2916d004a389e..d76b4d5f4951e 100644
--- a/lldb/bindings/python/CMakeLists.txt
+++ b/lldb/bindings/python/CMakeLists.txt
@@ -157,6 +157,9 @@ function(finish_swig_python swig_target lldb_python_bindings_dir lldb_python_tar
   create_relative_symlink(${swig_target} ${LIBLLDB_SYMLINK_DEST}
                           ${lldb_python_target_dir}/native/ ${LIBLLDB_SYMLINK_OUTPUT_FILE})
 
+  if (LLDB_ENABLE_DYNAMIC_SCRIPTINTERPRETERS)
+    add_dependencies(${swig_target} lldbPluginScriptInterpreterPython)
+  endif()
 
   if (NOT WIN32)
   add_dependencies(${swig_target} lldb-python-wrapper)
diff --git a/lldb/include/lldb/Host/FileBase.h b/lldb/include/lldb/Host/FileBase.h
index 54ca89f3201c6..bc0b2914ded8c 100644
--- a/lldb/include/lldb/Host/FileBase.h
+++ b/lldb/include/lldb/Host/FileBase.h
@@ -33,7 +33,7 @@ LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
 /// or stream functionality but are not backed by any host OS file.
 class File : public IOObject {
 public:
-  static int kInvalidDescriptor;
+  static constexpr int kInvalidDescriptor = -1;
   static FILE *kInvalidStream;
 
   // NB this enum is used in the lldb platform gdb-remote packet
diff --git a/lldb/source/Host/common/File.cpp b/lldb/source/Host/common/File.cpp
index 499df4e542ab2..a863e323c65ff 100644
--- a/lldb/source/Host/common/File.cpp
+++ b/lldb/source/Host/common/File.cpp
@@ -93,7 +93,6 @@ Expected<File::OpenOptions> File::GetOptionsFromMode(llvm::StringRef mode) {
       "invalid mode, cannot convert to File::OpenOptions");
 }
 
-int File::kInvalidDescriptor = -1;
 FILE *File::kInvalidStream = nullptr;
 
 Status File::Read(void *buf, size_t &num_bytes) {
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt b/lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
index 574dcdf874ada..55d8b6060486c 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
@@ -48,6 +48,8 @@ if (LLDB_ENABLE_DYNAMIC_SCRIPTINTERPRETERS)
   # resolved via liblldb's re-exports, so lldb_private libs aren't linked here.
   add_lldb_library(lldbPluginScriptInterpreterPython SHARED
     ${python_plugin_sources}
+    LINK_COMPONENTS
+      Support
     LINK_LIBS
       liblldb
       ${Python3_LIBRARIES}
@@ -78,6 +80,14 @@ if (LLDB_ENABLE_DYNAMIC_SCRIPTINTERPRETERS)
 
   target_link_directories(lldbPluginScriptInterpreterPython PRIVATE ${PYTHON_SABI_LIBRARY_DIRS})
   target_link_directories(lldbStaticScriptInterpreterPython PUBLIC ${PYTHON_SABI_LIBRARY_DIRS})
+  if(WIN32 AND NOT MINGW)
+    # Windows doesn't have "lib" as CMAKE_SHARED_LIBRARY_PREFIX. To be
+    # consistent with Unix, the output is still prefixed with "lib".
+    set_target_properties(lldbPluginScriptInterpreterPython
+      PROPERTIES
+      OUTPUT_NAME liblldbPluginScriptInterpreterPython
+    )
+  endif()
 else()
   add_lldb_library(lldbPluginScriptInterpreterPython PLUGIN
     ${python_plugin_sources}

``````````

</details>


https://github.com/llvm/llvm-project/pull/200843