[Lldb-commits] [lldb] 004a264 - [lldb] Fix shared library directory computation on windows
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Thu Feb 18 06:38:03 PST 2021
Author: Pavel Labath
Date: 2021-02-18T15:37:52+01:00
New Revision: 004a264f8c923922ecd34255bcb10f4adaa27ac5
URL: https://github.com/llvm/llvm-project/commit/004a264f8c923922ecd34255bcb10f4adaa27ac5
DIFF: https://github.com/llvm/llvm-project/commit/004a264f8c923922ecd34255bcb10f4adaa27ac5.diff
LOG: [lldb] Fix shared library directory computation on windows
Our code for locating the shared library directory works via dladdr (or
the windows equivalent) to locate the path of an address known to reside
in liblldb. This works great for C++ programs, but there's a catch.
When (lib)lldb is used from python (like in our test suite), this dladdr
call will return a path to the _lldb.so (or such) file in the python
directory. To compensate for this, we have code which attempts to
resolve this symlink, to ensure we get the canonical location. However,
here's the second catch.
On windows, this file is not a symlink (but a copy), so this logic
fails. Since most of our other paths are derived from the liblldb
location, all of these paths will be wrong, when running the test suite.
One effect of this was the failure to find lldb-server in D96202.
To fix this issue, I add some windows-specific code to locate the
liblldb directory. Since it cannot rely on symlinks, it works by
manually walking the directory tree -- essentially doing the opposite of
what we do when computing the python directory.
To avoid python leaking back into the host code, I implement this with
the help of a callback which can be passed to HostInfo::Initialize in
order to assist with the directory location. The callback lives inside
the python plugin.
I also strenghten the existing path test to ensure the returned path is
the right one.
Differential Revision: https://reviews.llvm.org/D96779
Added:
Modified:
lldb/include/lldb/Host/HostInfoBase.h
lldb/include/lldb/Host/linux/HostInfoLinux.h
lldb/include/lldb/Host/windows/HostInfoWindows.h
lldb/include/lldb/Initialization/SystemInitializerCommon.h
lldb/source/API/SystemInitializerFull.cpp
lldb/source/Host/common/HostInfoBase.cpp
lldb/source/Host/linux/HostInfoLinux.cpp
lldb/source/Host/windows/HostInfoWindows.cpp
lldb/source/Initialization/SystemInitializerCommon.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
lldb/test/API/functionalities/paths/TestPaths.py
lldb/tools/lldb-server/SystemInitializerLLGS.h
lldb/tools/lldb-test/SystemInitializerTest.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Host/HostInfoBase.h b/lldb/include/lldb/Host/HostInfoBase.h
index 15bb168aad97..b4fbd88c9304 100644
--- a/lldb/include/lldb/Host/HostInfoBase.h
+++ b/lldb/include/lldb/Host/HostInfoBase.h
@@ -37,7 +37,13 @@ class HostInfoBase {
~HostInfoBase() {}
public:
- static void Initialize();
+ /// A helper function for determining the liblldb location. It receives a
+ /// FileSpec with the location of file containing _this_ code. It can
+ /// (optionally) replace it with a file spec pointing to a more canonical
+ /// copy.
+ using SharedLibraryDirectoryHelper = void(FileSpec &this_file);
+
+ static void Initialize(SharedLibraryDirectoryHelper *helper = nullptr);
static void Terminate();
/// Gets the host target triple.
diff --git a/lldb/include/lldb/Host/linux/HostInfoLinux.h b/lldb/include/lldb/Host/linux/HostInfoLinux.h
index b1c7f9c805bf..e8080033cd8f 100644
--- a/lldb/include/lldb/Host/linux/HostInfoLinux.h
+++ b/lldb/include/lldb/Host/linux/HostInfoLinux.h
@@ -27,7 +27,7 @@ class HostInfoLinux : public HostInfoPosix {
~HostInfoLinux();
public:
- static void Initialize();
+ static void Initialize(SharedLibraryDirectoryHelper *helper = nullptr);
static llvm::VersionTuple GetOSVersion();
static bool GetOSBuildString(std::string &s);
diff --git a/lldb/include/lldb/Host/windows/HostInfoWindows.h b/lldb/include/lldb/Host/windows/HostInfoWindows.h
index 209d9da08214..400a3f4649bd 100644
--- a/lldb/include/lldb/Host/windows/HostInfoWindows.h
+++ b/lldb/include/lldb/Host/windows/HostInfoWindows.h
@@ -25,7 +25,7 @@ class HostInfoWindows : public HostInfoBase {
~HostInfoWindows();
public:
- static void Initialize();
+ static void Initialize(SharedLibraryDirectoryHelper *helper = nullptr);
static void Terminate();
static size_t GetPageSize();
diff --git a/lldb/include/lldb/Initialization/SystemInitializerCommon.h b/lldb/include/lldb/Initialization/SystemInitializerCommon.h
index 3a5081680879..d918b1125a57 100644
--- a/lldb/include/lldb/Initialization/SystemInitializerCommon.h
+++ b/lldb/include/lldb/Initialization/SystemInitializerCommon.h
@@ -10,6 +10,7 @@
#define LLDB_INITIALIZATION_SYSTEMINITIALIZERCOMMON_H
#include "SystemInitializer.h"
+#include "lldb/Host/HostInfo.h"
namespace lldb_private {
/// Initializes common lldb functionality.
@@ -22,11 +23,14 @@ namespace lldb_private {
/// the constructor.
class SystemInitializerCommon : public SystemInitializer {
public:
- SystemInitializerCommon();
+ SystemInitializerCommon(HostInfo::SharedLibraryDirectoryHelper *helper);
~SystemInitializerCommon() override;
llvm::Error Initialize() override;
void Terminate() override;
+
+private:
+ HostInfo::SharedLibraryDirectoryHelper *m_shlib_dir_helper;
};
} // namespace lldb_private
diff --git a/lldb/source/API/SystemInitializerFull.cpp b/lldb/source/API/SystemInitializerFull.cpp
index 0530f94580b3..bac55f647c5d 100644
--- a/lldb/source/API/SystemInitializerFull.cpp
+++ b/lldb/source/API/SystemInitializerFull.cpp
@@ -29,9 +29,22 @@
#define LLDB_PLUGIN(p) LLDB_PLUGIN_DECLARE(p)
#include "Plugins/Plugins.def"
+#if LLDB_ENABLE_PYTHON
+#include "Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h"
+
+constexpr lldb_private::HostInfo::SharedLibraryDirectoryHelper
+ *g_shlib_dir_helper =
+ lldb_private::ScriptInterpreterPython::SharedLibraryDirectoryHelper;
+
+#else
+constexpr lldb_private::HostInfo::SharedLibraryDirectoryHelper
+ *g_shlib_dir_helper = 0;
+#endif
+
using namespace lldb_private;
-SystemInitializerFull::SystemInitializerFull() = default;
+SystemInitializerFull::SystemInitializerFull()
+ : SystemInitializerCommon(g_shlib_dir_helper) {}
SystemInitializerFull::~SystemInitializerFull() = default;
llvm::Error SystemInitializerFull::Initialize() {
diff --git a/lldb/source/Host/common/HostInfoBase.cpp b/lldb/source/Host/common/HostInfoBase.cpp
index 333137a7fd25..d923a40cab3b 100644
--- a/lldb/source/Host/common/HostInfoBase.cpp
+++ b/lldb/source/Host/common/HostInfoBase.cpp
@@ -71,13 +71,18 @@ struct HostInfoBaseFields {
llvm::once_flag m_lldb_global_tmp_dir_once;
FileSpec m_lldb_global_tmp_dir;
};
+} // namespace
-HostInfoBaseFields *g_fields = nullptr;
-}
+static HostInfoBaseFields *g_fields = nullptr;
+static HostInfoBase::SharedLibraryDirectoryHelper *g_shlib_dir_helper = nullptr;
-void HostInfoBase::Initialize() { g_fields = new HostInfoBaseFields(); }
+void HostInfoBase::Initialize(SharedLibraryDirectoryHelper *helper) {
+ g_shlib_dir_helper = helper;
+ g_fields = new HostInfoBaseFields();
+}
void HostInfoBase::Terminate() {
+ g_shlib_dir_helper = nullptr;
delete g_fields;
g_fields = nullptr;
}
@@ -249,9 +254,8 @@ bool HostInfoBase::ComputeSharedLibraryDirectory(FileSpec &file_spec) {
reinterpret_cast<void *>(
HostInfoBase::ComputeSharedLibraryDirectory)));
- // This is necessary because when running the testsuite the shlib might be a
- // symbolic link inside the Python resource dir.
- FileSystem::Instance().ResolveSymbolicLink(lldb_file_spec, lldb_file_spec);
+ if (g_shlib_dir_helper)
+ g_shlib_dir_helper(lldb_file_spec);
// Remove the filename so that this FileSpec only represents the directory.
file_spec.GetDirectory() = lldb_file_spec.GetDirectory();
diff --git a/lldb/source/Host/linux/HostInfoLinux.cpp b/lldb/source/Host/linux/HostInfoLinux.cpp
index a35e5ef9e970..9e8a00f4fac1 100644
--- a/lldb/source/Host/linux/HostInfoLinux.cpp
+++ b/lldb/source/Host/linux/HostInfoLinux.cpp
@@ -33,8 +33,8 @@ struct HostInfoLinuxFields {
HostInfoLinuxFields *g_fields = nullptr;
}
-void HostInfoLinux::Initialize() {
- HostInfoPosix::Initialize();
+void HostInfoLinux::Initialize(SharedLibraryDirectoryHelper *helper) {
+ HostInfoPosix::Initialize(helper);
g_fields = new HostInfoLinuxFields();
}
diff --git a/lldb/source/Host/windows/HostInfoWindows.cpp b/lldb/source/Host/windows/HostInfoWindows.cpp
index ba170b68bbcd..54a07b71b2cd 100644
--- a/lldb/source/Host/windows/HostInfoWindows.cpp
+++ b/lldb/source/Host/windows/HostInfoWindows.cpp
@@ -39,9 +39,9 @@ class WindowsUserIDResolver : public UserIDResolver {
FileSpec HostInfoWindows::m_program_filespec;
-void HostInfoWindows::Initialize() {
+void HostInfoWindows::Initialize(SharedLibraryDirectoryHelper *helper) {
::CoInitializeEx(nullptr, COINIT_MULTITHREADED);
- HostInfoBase::Initialize();
+ HostInfoBase::Initialize(helper);
}
void HostInfoWindows::Terminate() {
diff --git a/lldb/source/Initialization/SystemInitializerCommon.cpp b/lldb/source/Initialization/SystemInitializerCommon.cpp
index d9f69f57703c..004fdb120682 100644
--- a/lldb/source/Initialization/SystemInitializerCommon.cpp
+++ b/lldb/source/Initialization/SystemInitializerCommon.cpp
@@ -11,7 +11,6 @@
#include "Plugins/Process/gdb-remote/ProcessGDBRemoteLog.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Host/Host.h"
-#include "lldb/Host/HostInfo.h"
#include "lldb/Host/Socket.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/ReproducerProvider.h"
@@ -35,7 +34,9 @@
using namespace lldb_private;
using namespace lldb_private::repro;
-SystemInitializerCommon::SystemInitializerCommon() {}
+SystemInitializerCommon::SystemInitializerCommon(
+ HostInfo::SharedLibraryDirectoryHelper *helper)
+ : m_shlib_dir_helper(helper) {}
SystemInitializerCommon::~SystemInitializerCommon() {}
@@ -125,7 +126,7 @@ llvm::Error SystemInitializerCommon::Initialize() {
return e;
Log::Initialize();
- HostInfo::Initialize();
+ HostInfo::Initialize(m_shlib_dir_helper);
llvm::Error error = Socket::Initialize();
if (error)
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
index c2fd1f266f07..68409b724905 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -410,6 +410,31 @@ FileSpec ScriptInterpreterPython::GetPythonDir() {
return g_spec;
}
+void ScriptInterpreterPython::SharedLibraryDirectoryHelper(
+ FileSpec &this_file) {
+ // When we're loaded from python, this_file will point to the file inside the
+ // python package directory. Replace it with the one in the lib directory.
+#ifdef _WIN32
+ // On windows, we need to manually back out of the python tree, and go into
+ // the bin directory. This is pretty much the inverse of what ComputePythonDir
+ // does.
+ if (this_file.GetFileNameExtension() == ConstString(".pyd")) {
+ this_file.RemoveLastPathComponent(); // _lldb.pyd or _lldb_d.pyd
+ this_file.RemoveLastPathComponent(); // lldb
+ for (auto it = llvm::sys::path::begin(LLDB_PYTHON_RELATIVE_LIBDIR),
+ end = llvm::sys::path::end(LLDB_PYTHON_RELATIVE_LIBDIR);
+ it != end; ++it)
+ this_file.RemoveLastPathComponent();
+ this_file.AppendPathComponent("bin");
+ this_file.AppendPathComponent("liblldb.dll");
+ }
+#else
+ // The python file is a symlink, so we can find the real library by resolving
+ // it. We can do this unconditionally.
+ FileSystem::Instance().ResolveSymbolicLink(this_file, this_file);
+#endif
+}
+
lldb_private::ConstString ScriptInterpreterPython::GetPluginNameStatic() {
static ConstString g_name("script-python");
return g_name;
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
index e59fedbd0971..5a75c0a655e5 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
@@ -51,6 +51,7 @@ class ScriptInterpreterPython : public ScriptInterpreter,
static lldb_private::ConstString GetPluginNameStatic();
static const char *GetPluginDescriptionStatic();
static FileSpec GetPythonDir();
+ static void SharedLibraryDirectoryHelper(FileSpec &this_file);
protected:
static void ComputePythonDirForApple(llvm::SmallVectorImpl<char> &path);
diff --git a/lldb/test/API/functionalities/paths/TestPaths.py b/lldb/test/API/functionalities/paths/TestPaths.py
index 5c21622d03d8..8ef20c54974b 100644
--- a/lldb/test/API/functionalities/paths/TestPaths.py
+++ b/lldb/test/API/functionalities/paths/TestPaths.py
@@ -8,6 +8,7 @@
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
from lldbsuite.test import lldbutil
+from lldbsuite.test import lldbplatformutil
class TestPaths(TestBase):
@@ -29,7 +30,18 @@ def test_paths(self):
for path_type in dir_path_types:
f = lldb.SBHostOS.GetLLDBPath(path_type)
# No directory path types should have the filename set
- self.assertTrue(f.GetFilename() is None)
+ self.assertIsNone(f.GetFilename())
+
+ shlib_dir = lldb.SBHostOS.GetLLDBPath(lldb.ePathTypeLLDBShlibDir).GetDirectory()
+ if lldbplatformutil.getHostPlatform() == 'windows':
+ filenames = ['liblldb.dll']
+ elif lldbplatformutil.getHostPlatform() == 'darwin':
+ filenames = ['LLDB', 'liblldb.dylib']
+ else:
+ filenames = ['liblldb.so']
+ self.assertTrue(any([os.path.exists(os.path.join(shlib_dir, f)) for f in
+ filenames]), "shlib_dir = " + shlib_dir)
+
@no_debug_info_test
def test_directory_doesnt_end_with_slash(self):
diff --git a/lldb/tools/lldb-server/SystemInitializerLLGS.h b/lldb/tools/lldb-server/SystemInitializerLLGS.h
index f3d015e94f8c..4469a8ba5f60 100644
--- a/lldb/tools/lldb-server/SystemInitializerLLGS.h
+++ b/lldb/tools/lldb-server/SystemInitializerLLGS.h
@@ -14,6 +14,8 @@
class SystemInitializerLLGS : public lldb_private::SystemInitializerCommon {
public:
+ SystemInitializerLLGS() : SystemInitializerCommon(nullptr) {}
+
llvm::Error Initialize() override;
void Terminate() override;
};
diff --git a/lldb/tools/lldb-test/SystemInitializerTest.cpp b/lldb/tools/lldb-test/SystemInitializerTest.cpp
index 2f6eb8e21a21..2b6e0f26bb49 100644
--- a/lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ b/lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -22,7 +22,8 @@
using namespace lldb_private;
-SystemInitializerTest::SystemInitializerTest() = default;
+SystemInitializerTest::SystemInitializerTest()
+ : SystemInitializerCommon(nullptr) {}
SystemInitializerTest::~SystemInitializerTest() = default;
llvm::Error SystemInitializerTest::Initialize() {
More information about the lldb-commits
mailing list