[lldb] Fix shared library directory computation on windows

[labath]

Summary

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.

[JDevlieghere]

Seems like a reasonable solution. LGTM.

lldb/include/lldb/Host/HostInfoBase.h

44–46

Why a function pointer and no function_ref?

[labath]

lldb/include/lldb/Host/HostInfoBase.h

44–46

function_ref can only be used for objects which do not escape the current function (just like StringRef), while this value is persisted. It could be a std::function, but that seems a bit overkill.

[JDevlieghere]

lldb/include/lldb/Host/HostInfoBase.h

44–46

Forgive my ignorance, but how does that apply to SharedLibraryDirectoryHelper which is a static method?

[labath]

lldb/include/lldb/Host/HostInfoBase.h

44–46

It doesn't -- persisting a function_ref pointing to a static method would actually work in this case. However, that's the _only_ thing that would work, so there's no benefit to doing it.
And there's the downside that if someone tries to something more fancy through that function_ref, it would blow up. E.g., something like this would be impossible

HostInfo::Initialize([important_variable](FileSpec &this_file) { do_stuff(this_file, important_variable); });

Because by the time that the function_ref gets called, the storage for important_variable (along with the entire lambda object) would be destroyed. It's exactly the same situation as with a StringRef. Imagine code like:

StringRef g_foo;
HostInfo::Initialize(StringRef foo) { g_foo = foo; } 
HostInfo::Stuff() { /* do something with g_foo */ }

This would work just fine if you always pass a string literal for the StringRef, but would blow up as soon as one tries to pass a temporary string. And there are two ways to make it safer:

• use std::string (like std::function)
• use const char * (like a raw function pointer, except function pointers are even safer, as dynamically allocating functions is not common)

[JDevlieghere]

lldb/include/lldb/Host/HostInfoBase.h

44–46

Okay, sounds like we're on the same page, I misunderstood your comment as "this can't work for the current case" which led to the confusion on my part. I agree that making the interface as safe as possible is desirable.

[stella.stamenova]

LGTM.

lldb/include/lldb/Host/HostInfoBase.h

 
 class HostInfoBase {
 private:
   // Static class, unconstructable.
   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.
   ///
   /// \return
   ///     The host target triple.
   static llvm::Triple GetTargetTriple();
 

lldb/include/lldb/Host/linux/HostInfoLinux.h

class HostInfoLinux : public HostInfoPosix {
   friend class HostInfoBase;
 
 private:
   // Static class, unconstructable.
   HostInfoLinux();
   ~HostInfoLinux();
 
 public:
-  static void Initialize();
+  static void Initialize(SharedLibraryDirectoryHelper *helper = nullptr);
 
   static llvm::VersionTuple GetOSVersion();
   static bool GetOSBuildString(std::string &s);
   static bool GetOSKernelDescription(std::string &s);
   static llvm::StringRef GetDistributionId();
   static FileSpec GetProgramFileSpec();
 
 protected:

lldb/include/lldb/Host/windows/HostInfoWindows.h

class HostInfoWindows : public HostInfoBase {
   friend class HostInfoBase;
 
 private:
   // Static class, unconstructable.
   HostInfoWindows();
   ~HostInfoWindows();
 
 public:
-  static void Initialize();
+  static void Initialize(SharedLibraryDirectoryHelper *helper = nullptr);
   static void Terminate();
 
   static size_t GetPageSize();
   static UserIDResolver &GetUserIDResolver();
 
   static llvm::VersionTuple GetOSVersion();
   static bool GetOSBuildString(std::string &s);
   static bool GetOSKernelDescription(std::string &s);

lldb/include/lldb/Initialization/SystemInitializerCommon.h

 //===-- SystemInitializerCommon.h -------------------------------*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_INITIALIZATION_SYSTEMINITIALIZERCOMMON_H
 #define LLDB_INITIALIZATION_SYSTEMINITIALIZERCOMMON_H
 
 #include "SystemInitializer.h"
+#include "lldb/Host/HostInfo.h"
 
 namespace lldb_private {
 /// Initializes common lldb functionality.
 ///
 /// This class is responsible for initializing a subset of lldb
 /// useful to both debug servers and debug clients.  Debug servers
 /// do not use all of LLDB and desire small binary sizes, so this
 /// functionality is separate.  This class is used by constructing
 /// an instance of SystemLifetimeManager with this class passed to
 /// 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
 
 #endif // LLDB_INITIALIZATION_SYSTEMINITIALIZERCOMMON_H

lldb/source/API/SystemInitializerFull.cpp

 #include "llvm/ExecutionEngine/MCJIT.h"
 #pragma clang diagnostic pop
 
 #include <string>
 
 #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() {
   llvm::Error error = SystemInitializerCommon::Initialize();
   if (error) {
     // During active replay, the ::Initialize call is replayed like any other
     // SB API call and the return value is ignored. Since we can't intercept
     // this, we terminate here before the uninitialized debugger inevitably

lldb/source/Host/common/HostInfoBase.cpp

struct HostInfoBaseFields {
   FileSpec m_lldb_system_plugin_dir;
   llvm::once_flag m_lldb_user_plugin_dir_once;
   FileSpec m_lldb_user_plugin_dir;
   llvm::once_flag m_lldb_process_tmp_dir_once;
   FileSpec m_lldb_process_tmp_dir;
   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;
 }
 
 llvm::Triple HostInfoBase::GetTargetTriple() {
   llvm::call_once(g_fields->m_host_triple_once, []() {
     g_fields->m_host_triple =
         HostInfo::GetArchitecture().GetTriple();
bool HostInfoBase::ComputeSharedLibraryDirectory(FileSpec &file_spec) {
   // To get paths related to LLDB we get the path to the executable that
   // contains this function. On MacOSX this will be "LLDB.framework/.../LLDB".
   // On other posix systems, we will get .../lib(64|32)?/liblldb.so.
 
   FileSpec lldb_file_spec(Host::GetModuleFileSpecForHostAddress(
       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();
 
   return (bool)file_spec.GetDirectory();
 }
 
 bool HostInfoBase::ComputeSupportExeDirectory(FileSpec &file_spec) {

lldb/source/Host/linux/HostInfoLinux.cpp

 struct HostInfoLinuxFields {
   std::string m_distribution_id;
   llvm::VersionTuple m_os_version;
 };
 
 HostInfoLinuxFields *g_fields = nullptr;
 }
 
-void HostInfoLinux::Initialize() {
-  HostInfoPosix::Initialize();
+void HostInfoLinux::Initialize(SharedLibraryDirectoryHelper *helper) {
+  HostInfoPosix::Initialize(helper);
 
   g_fields = new HostInfoLinuxFields();
 }
 
 llvm::VersionTuple HostInfoLinux::GetOSVersion() {
   static llvm::once_flag g_once_flag;
   llvm::call_once(g_once_flag, []() {
     struct utsname un;

lldb/source/Host/windows/HostInfoWindows.cpp

protected:
   llvm::Optional<std::string> DoGetGroupName(id_t gid) override {
     return llvm::None;
   }
 };
 } // namespace
 
 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() {
   HostInfoBase::Terminate();
   ::CoUninitialize();
 }
 
 size_t HostInfoWindows::GetPageSize() {

lldb/source/Initialization/SystemInitializerCommon.cpp

 //===-- SystemInitializerCommon.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 "lldb/Initialization/SystemInitializerCommon.h"
 
 #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"
 #include "lldb/Utility/Timer.h"
 #include "lldb/lldb-private.h"
 
 #if defined(__linux__) || defined(__FreeBSD__) || defined(__NetBSD__)
 #include "Plugins/Process/POSIX/ProcessPOSIXLog.h"
 #endif
 
 #if defined(_WIN32)
 #include "Plugins/Process/Windows/Common/ProcessWindowsLog.h"
 #include "lldb/Host/windows/windows.h"
 #include <crtdbg.h>
 #endif
 
 #include "llvm/Support/TargetSelect.h"
 
 #include <string>
 
 using namespace lldb_private;
 using namespace lldb_private::repro;
 
-SystemInitializerCommon::SystemInitializerCommon() {}
+SystemInitializerCommon::SystemInitializerCommon(
+    HostInfo::SharedLibraryDirectoryHelper *helper)
+    : m_shlib_dir_helper(helper) {}
 
 SystemInitializerCommon::~SystemInitializerCommon() {}
 
 /// Initialize the FileSystem based on the current reproducer mode.
 static llvm::Error InitializeFileSystem() {
   auto &r = repro::Reproducer::Instance();
   if (repro::Loader *loader = r.GetLoader()) {
     FileSpec vfs_mapping = loader->GetFile<FileProvider::Info>();
if (!Reproducer::Initialized()) {
     if (auto e = Reproducer::Initialize(ReproducerMode::Off, llvm::None))
       return e;
   }
 
   if (auto e = InitializeFileSystem())
     return e;
 
   Log::Initialize();
-  HostInfo::Initialize();
+  HostInfo::Initialize(m_shlib_dir_helper);
 
   llvm::Error error = Socket::Initialize();
   if (error)
     return error;
 
   LLDB_SCOPED_TIMER();
 
   process_gdb_remote::ProcessGDBRemoteLog::Initialize();

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h

ScriptInterpreterPython(Debugger &debugger)
       : ScriptInterpreter(debugger, lldb::eScriptLanguagePython),
         IOHandlerDelegateMultiline("DONE") {}
 
   static void Initialize();
   static void Terminate();
   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);
   static void ComputePythonDir(llvm::SmallVectorImpl<char> &path);
 };
 } // namespace lldb_private
 
 #endif // LLDB_ENABLE_PYTHON
 #endif // LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTINTERPRETERPYTHON_H

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

#else
     ComputePythonDir(path);
 #endif
     spec.GetDirectory().SetString(path);
     return spec;
   }();
   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;
 }
 
 const char *ScriptInterpreterPython::GetPluginDescriptionStatic() {
   return "Embedded Python interpreter";
 }

lldb/test/API/functionalities/paths/TestPaths.py

 """
 Test some lldb command abbreviations.
 """
 
 
 import lldb
 import os
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
+from lldbsuite.test import lldbplatformutil
 
 
 class TestPaths(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     @no_debug_info_test
     def test_paths(self):
         '''Test to make sure no file names are set in the lldb.SBFileSpec objects returned by lldb.SBHostOS.GetLLDBPath() for paths that are directories'''
         dir_path_types = [lldb.ePathTypeLLDBShlibDir,
                           lldb.ePathTypeSupportExecutableDir,
                           lldb.ePathTypeHeaderDir,
                           lldb.ePathTypePythonDir,
                           lldb.ePathTypeLLDBSystemPlugins,
                           lldb.ePathTypeLLDBUserPlugins,
                           lldb.ePathTypeLLDBTempSystemDir,
                           lldb.ePathTypeClangDir]
 
         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):
         current_directory_spec = lldb.SBFileSpec(os.path.curdir)
         current_directory_string = current_directory_spec.GetDirectory()
         self.assertNotEqual(current_directory_string[-1:], '/')
 
     @skipUnlessPlatform(["windows"])

lldb/tools/lldb-server/SystemInitializerLLGS.h

 //===-- SystemInitializerLLGS.h ---------------------------------*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_TOOLS_LLDB_SERVER_SYSTEMINITIALIZERLLGS_H
 #define LLDB_TOOLS_LLDB_SERVER_SYSTEMINITIALIZERLLGS_H
 
 #include "lldb/Initialization/SystemInitializer.h"
 #include "lldb/Initialization/SystemInitializerCommon.h"
 
 class SystemInitializerLLGS : public lldb_private::SystemInitializerCommon {
 public:
+  SystemInitializerLLGS() : SystemInitializerCommon(nullptr) {}
+
   llvm::Error Initialize() override;
   void Terminate() override;
 };
 
 #endif // LLDB_TOOLS_LLDB_SERVER_SYSTEMINITIALIZERLLGS_H

lldb/tools/lldb-test/SystemInitializerTest.cpp

 
 #include <string>
 
 #define LLDB_PLUGIN(p) LLDB_PLUGIN_DECLARE(p)
 #include "Plugins/Plugins.def"
 
 using namespace lldb_private;
 
-SystemInitializerTest::SystemInitializerTest() = default;
+SystemInitializerTest::SystemInitializerTest()
+    : SystemInitializerCommon(nullptr) {}
 SystemInitializerTest::~SystemInitializerTest() = default;
 
 llvm::Error SystemInitializerTest::Initialize() {
   if (auto e = SystemInitializerCommon::Initialize())
     return e;
 
   // Initialize LLVM and Clang
   llvm::InitializeAllTargets();