[Lldb-commits] [lldb] e0dbd02 - [lldb/test] Make TestLoadUnload compatible with windows

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Tue Apr 14 02:11:19 PDT 2020


Author: Pavel Labath
Date: 2020-04-14T11:10:59+02:00
New Revision: e0dbd025131c4d77d8a5050a91d391d950529a8c

URL: https://github.com/llvm/llvm-project/commit/e0dbd025131c4d77d8a5050a91d391d950529a8c
DIFF: https://github.com/llvm/llvm-project/commit/e0dbd025131c4d77d8a5050a91d391d950529a8c.diff

LOG: [lldb/test] Make TestLoadUnload compatible with windows

Summary:
This patch introduces a header "dylib.h" which can be used in tests to
handle shared libraries semi-portably. The shared library APIs on
windows and posix systems look very different, but their underlying
functionality is relatively similar, so the mapping is not difficult.

It also introduces two new macros to wrap the functinality necessary to
export/import function across the dll boundary on windows. Previously we
had the LLDB_TEST_API macro for this purpose, which automagically
changed meaning depending on whether we were building the shared library
or the executable. While convenient for simple cases, this approach was
not sufficient for the more complicated setups where one deals with
multiple shared libraries.

Lastly it rewrites TestLoadUnload, to make use of the new APIs. The
trickiest aspect there is the handling of DYLD_LIBRARY_PATH on macos --
previously setting this variable was not needed as the test used
@executable_path-relative dlopens, but the new generic api does not
support that. Other systems do not support such dlopens either so the
test already contained support for setting the appropriate path
variable, and this patch just makes that logic more generic. In doesn't
seem that the purpose of this test was to exercise @executable_path
imports, so this should not be a problem.

These changes are sufficient to make some of the TestLoadUnload tests
pass on windows. Two other tests will start to pass once D77287 lands.

Reviewers: amccarth, jingham, JDevlieghere, compnerd

Subscribers: lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D77662

Added: 
    lldb/packages/Python/lldbsuite/test/make/dylib.h

Modified: 
    lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
    lldb/packages/Python/lldbsuite/test/make/Makefile.rules
    lldb/packages/Python/lldbsuite/test/make/test_common.h
    lldb/test/API/functionalities/load_unload/Makefile
    lldb/test/API/functionalities/load_unload/TestLoadUnload.py
    lldb/test/API/functionalities/load_unload/a.cpp
    lldb/test/API/functionalities/load_unload/b.cpp
    lldb/test/API/functionalities/load_unload/d.cpp
    lldb/test/API/functionalities/load_unload/main.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
index 32c9c20d6eea..44659cac4c77 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
@@ -166,19 +166,20 @@ def findMainThreadCheckerDylib():
 class _PlatformContext(object):
     """Value object class which contains platform-specific options."""
 
-    def __init__(self, shlib_environment_var, shlib_prefix, shlib_extension):
+    def __init__(self, shlib_environment_var, shlib_path_separator, shlib_prefix, shlib_extension):
         self.shlib_environment_var = shlib_environment_var
+        self.shlib_path_separator = shlib_path_separator
         self.shlib_prefix = shlib_prefix
         self.shlib_extension = shlib_extension
 
 
 def createPlatformContext():
     if platformIsDarwin():
-        return _PlatformContext('DYLD_LIBRARY_PATH', 'lib', 'dylib')
+        return _PlatformContext('DYLD_LIBRARY_PATH', ':', 'lib', 'dylib')
     elif getPlatform() in ("freebsd", "linux", "netbsd"):
-        return _PlatformContext('LD_LIBRARY_PATH', 'lib', 'so')
+        return _PlatformContext('LD_LIBRARY_PATH', ':', 'lib', 'so')
     else:
-        return None
+        return _PlatformContext('PATH', ';', '', 'dll')
 
 
 def hasChattyStderr(test_case):

diff  --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
index 4ae54561a28f..ea0fa748bc36 100644
--- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -474,7 +474,7 @@ endif
 # Additional system libraries
 #----------------------------------------------------------------------
 ifeq (1,$(USE_LIBDL))
-	ifneq ($(OS),NetBSD)
+	ifeq (,$(filter $(OS), NetBSD Windows_NT))
 		LDFLAGS += -ldl
 	endif
 endif

diff  --git a/lldb/packages/Python/lldbsuite/test/make/dylib.h b/lldb/packages/Python/lldbsuite/test/make/dylib.h
new file mode 100644
index 000000000000..50abcdbca9a2
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/make/dylib.h
@@ -0,0 +1,55 @@
+#ifndef LLDB_TEST_DYLIB_H
+#define LLDB_TEST_DYLIB_H
+
+#include <stdio.h>
+
+#ifdef _WIN32
+#include <Windows.h>
+
+#define dylib_get_symbol(handle, name) GetProcAddress((HMODULE)handle, name)
+#define dylib_close(handle) (!FreeLibrary((HMODULE)handle))
+#else
+#include <dlfcn.h>
+
+#define dylib_get_symbol(handle, name) dlsym(handle, name)
+#define dylib_close(handle) dlclose(handle)
+#endif
+
+
+inline void *dylib_open(const char *name) {
+  char dylib_prefix[] =
+#ifdef _WIN32
+    "";
+#else
+    "lib";
+#endif
+  char dylib_suffix[] =
+#ifdef _WIN32
+    ".dll";
+#elif defined(__APPLE__)
+    ".dylib";
+#else
+    ".so";
+#endif
+  char fullname[1024];
+  snprintf(fullname, sizeof(fullname), "%s%s%s", dylib_prefix, name, dylib_suffix);
+#ifdef _WIN32
+  return LoadLibraryA(fullname);
+#else
+  return dlopen(fullname, RTLD_NOW);
+#endif
+}
+
+inline const char *dylib_last_error() {
+#ifndef _WIN32
+  return dlerror();
+#else
+  DWORD err = GetLastError();
+  char *msg;
+  FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM,
+      NULL, err, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), (char *)&msg, 0, NULL);
+  return msg;
+#endif
+}
+
+#endif

diff  --git a/lldb/packages/Python/lldbsuite/test/make/test_common.h b/lldb/packages/Python/lldbsuite/test/make/test_common.h
index 529d0952ed3e..aa8960e1c0ae 100644
--- a/lldb/packages/Python/lldbsuite/test/make/test_common.h
+++ b/lldb/packages/Python/lldbsuite/test/make/test_common.h
@@ -1,13 +1,18 @@
 // This header is included in all the test programs (C and C++) and provides a
 // hook for dealing with platform-specifics.
+
 #if defined(_WIN32) || defined(_WIN64)
-#ifdef COMPILING_LLDB_TEST_DLL
-#define LLDB_TEST_API __declspec(dllexport)
+#define LLDB_DYLIB_EXPORT __declspec(dllexport)
+#define LLDB_DYLIB_IMPORT __declspec(dllimport)
 #else
-#define LLDB_TEST_API __declspec(dllimport)
+#define LLDB_DYLIB_EXPORT
+#define LLDB_DYLIB_IMPORT
 #endif
+
+#ifdef COMPILING_LLDB_TEST_DLL
+#define LLDB_TEST_API LLDB_DYLIB_EXPORT
 #else
-#define LLDB_TEST_API
+#define LLDB_TEST_API LLDB_DYLIB_IMPORT
 #endif
 
 #if defined(_WIN32)

diff  --git a/lldb/test/API/functionalities/load_unload/Makefile b/lldb/test/API/functionalities/load_unload/Makefile
index 00054aabd4a1..e73ec7310876 100644
--- a/lldb/test/API/functionalities/load_unload/Makefile
+++ b/lldb/test/API/functionalities/load_unload/Makefile
@@ -26,7 +26,6 @@ ifeq ($(OS),Darwin)
 	install_name_tool -id @executable_path/libloadunload_d.dylib libloadunload_d.dylib
 endif
 
-hidden_lib_d:
-	mkdir -p hidden
+hidden_lib_d: hidden
 	$(MAKE) VPATH=$(SRCDIR)/hidden -C hidden -f $(MAKEFILE_RULES) \
 		DYLIB_ONLY=YES DYLIB_CXX_SOURCES=d.cpp DYLIB_NAME=loadunload_d

diff  --git a/lldb/test/API/functionalities/load_unload/TestLoadUnload.py b/lldb/test/API/functionalities/load_unload/TestLoadUnload.py
index 5c4de256fcb6..7d255a647813 100644
--- a/lldb/test/API/functionalities/load_unload/TestLoadUnload.py
+++ b/lldb/test/API/functionalities/load_unload/TestLoadUnload.py
@@ -13,7 +13,6 @@
 from lldbsuite.test import lldbutil
 
 
- at skipIfWindows  # Windows doesn't have dlopen and friends, dynamic libraries work 
diff erently
 class LoadUnloadTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
@@ -35,25 +34,15 @@ def setUp(self):
 
     def setup_test(self):
         lldbutil.mkdir_p(self.getBuildArtifact("hidden"))
-        if not self.platformIsDarwin():
-            if not lldb.remote_platform and "LD_LIBRARY_PATH" in os.environ:
-                self.runCmd(
-                    "settings set target.env-vars " +
-                    self.dylibPath +
-                    "=" +
-                    os.environ["LD_LIBRARY_PATH"] +
-                    ":" +
-                    self.getBuildDir())
-            else:
-                if lldb.remote_platform:
-                    wd = lldb.remote_platform.GetWorkingDirectory()
-                else:
-                    wd = self.getBuildDir()
-                self.runCmd(
-                    "settings set target.env-vars " +
-                    self.dylibPath +
-                    "=" +
-                    wd)
+        if lldb.remote_platform:
+            path = lldb.remote_platform.GetWorkingDirectory()
+        else:
+            path = self.getBuildDir()
+            if self.dylibPath in os.environ:
+                sep = self.platformContext.shlib_path_separator
+                path = os.environ[self.dylibPath] + sep + path
+        self.runCmd("settings append target.env-vars '{}={}'".format(self.dylibPath, path))
+        self.default_path = path
 
     def copy_shlibs_to_remote(self, hidden_dir=False):
         """ Copies the shared libs required by this test suite to remote.
@@ -142,16 +131,12 @@ def test_modules_search_paths(self):
         # Obliterate traces of libd from the old location.
         os.remove(old_dylib)
         # Inform (DY)LD_LIBRARY_PATH of the new path, too.
-        env_cmd_string = "settings set target.env-vars " + self.dylibPath + "=" + new_dir
+        env_cmd_string = "settings replace target.env-vars " + self.dylibPath + "=" + new_dir
         if self.TraceOn():
             print("Set environment to: ", env_cmd_string)
         self.runCmd(env_cmd_string)
         self.runCmd("settings show target.env-vars")
 
-        remove_dyld_path_cmd = "settings remove target.env-vars " + self.dylibPath
-        self.addTearDownHook(
-            lambda: self.dbg.HandleCommand(remove_dyld_path_cmd))
-
         self.runCmd("run")
 
         self.expect(
@@ -195,17 +180,14 @@ def test_dyld_library_path(self):
         new_dir = os.path.join(wd, special_dir)
         old_dylib = os.path.join(old_dir, dylibName)
 
-        remove_dyld_path_cmd = "settings remove target.env-vars " \
-                               + self.dylibPath
-        self.addTearDownHook(
-            lambda: self.dbg.HandleCommand(remove_dyld_path_cmd))
-
         # For now we don't track (DY)LD_LIBRARY_PATH, so the old
         # library will be in the modules list.
         self.expect("target modules list",
                     substrs=[os.path.basename(old_dylib)],
                     matching=True)
 
+        self.runCmd(env_cmd_string)
+
         lldbutil.run_break_set_by_file_and_line(
             self, "d.cpp", self.line_d_function, num_expected_locations=1)
         # After run, make sure the non-hidden library is picked up.
@@ -214,10 +196,9 @@ def test_dyld_library_path(self):
         self.runCmd("continue")
 
         # Add the hidden directory first in the search path.
-        env_cmd_string = ("settings set target.env-vars %s=%s" %
-                          (self.dylibPath, new_dir))
-        if not self.platformIsDarwin():
-            env_cmd_string += ":" + wd
+        env_cmd_string = ("settings set target.env-vars %s=%s%s%s" %
+                          (self.dylibPath, new_dir,
+                              self.platformContext.shlib_path_separator, self.default_path))
         self.runCmd(env_cmd_string)
 
         # This time, the hidden library should be picked up.
@@ -228,7 +209,7 @@ def test_dyld_library_path(self):
         hostoslist=["windows"],
         triple='.*-android')
     @skipIfFreeBSD  # llvm.org/pr14424 - missing FreeBSD Makefiles/testcase support
-    @skipIfWindows  # Windows doesn't have dlopen and friends, dynamic libraries work 
diff erently
+    @expectedFailureAll(oslist=["windows"]) # process load not implemented
     def test_lldb_process_load_and_unload_commands(self):
         self.setSvr4Support(False)
         self.run_lldb_process_load_and_unload_commands()
@@ -238,7 +219,7 @@ def test_lldb_process_load_and_unload_commands(self):
         hostoslist=["windows"],
         triple='.*-android')
     @skipIfFreeBSD  # llvm.org/pr14424 - missing FreeBSD Makefiles/testcase support
-    @skipIfWindows  # Windows doesn't have dlopen and friends, dynamic libraries work 
diff erently
+    @expectedFailureAll(oslist=["windows"]) # process load not implemented
     def test_lldb_process_load_and_unload_commands_with_svr4(self):
         self.setSvr4Support(True)
         self.run_lldb_process_load_and_unload_commands()
@@ -314,11 +295,13 @@ def run_lldb_process_load_and_unload_commands(self):
         self.runCmd("process continue")
 
     @skipIfFreeBSD  # llvm.org/pr14424 - missing FreeBSD Makefiles/testcase support
+    @expectedFailureAll(oslist=["windows"]) # breakpoint not hit
     def test_load_unload(self):
         self.setSvr4Support(False)
         self.run_load_unload()
 
     @skipIfFreeBSD  # llvm.org/pr14424 - missing FreeBSD Makefiles/testcase support
+    @expectedFailureAll(oslist=["windows"]) # breakpoint not hit
     def test_load_unload_with_svr4(self):
         self.setSvr4Support(True)
         self.run_load_unload()
@@ -362,7 +345,6 @@ def run_load_unload(self):
                     substrs=[' resolved, hit count = 2'])
 
     @skipIfFreeBSD  # llvm.org/pr14424 - missing FreeBSD Makefiles/testcase support
-    @skipIfWindows  # Windows doesn't have dlopen and friends, dynamic libraries work 
diff erently
     @expectedFailureAll(archs="aarch64", oslist="linux",
                         bugnumber="https://bugs.llvm.org/show_bug.cgi?id=27806")
     def test_step_over_load(self):
@@ -370,7 +352,6 @@ def test_step_over_load(self):
         self.run_step_over_load()
 
     @skipIfFreeBSD  # llvm.org/pr14424 - missing FreeBSD Makefiles/testcase support
-    @skipIfWindows  # Windows doesn't have dlopen and friends, dynamic libraries work 
diff erently
     @expectedFailureAll(archs="aarch64", oslist="linux",
                         bugnumber="https://bugs.llvm.org/show_bug.cgi?id=27806")
     def test_step_over_load_with_svr4(self):
@@ -408,7 +389,6 @@ def run_step_over_load(self):
     # executable dependencies are resolved relative to the debuggers PWD. Bug?
     @expectedFailureAll(oslist=["linux"], triple=no_match('aarch64-.*-android'))
     @skipIfFreeBSD  # llvm.org/pr14424 - missing FreeBSD Makefiles/testcase support
-    @skipIfWindows  # Windows doesn't have dlopen and friends, dynamic libraries work 
diff erently
     @expectedFailureNetBSD
     def test_static_init_during_load(self):
         """Test that we can set breakpoints correctly in static initializers"""
@@ -438,7 +418,7 @@ def test_static_init_during_load(self):
                              'stop reason = breakpoint %d' % b_init_bp_num])
         self.expect("thread backtrace",
                     substrs=['b_init',
-                             'dlopen',
+                             'dylib_open',
                              'main'])
 
         self.runCmd("continue")
@@ -448,5 +428,5 @@ def test_static_init_during_load(self):
                              'stop reason = breakpoint %d' % a_init_bp_num])
         self.expect("thread backtrace",
                     substrs=['a_init',
-                             'dlopen',
+                             'dylib_open',
                              'main'])

diff  --git a/lldb/test/API/functionalities/load_unload/a.cpp b/lldb/test/API/functionalities/load_unload/a.cpp
index 497d6070a600..1842506bf7b8 100644
--- a/lldb/test/API/functionalities/load_unload/a.cpp
+++ b/lldb/test/API/functionalities/load_unload/a.cpp
@@ -1,4 +1,4 @@
-extern int b_function ();
+extern LLDB_DYLIB_IMPORT int b_function();
 
 int a_init()
 {

diff  --git a/lldb/test/API/functionalities/load_unload/b.cpp b/lldb/test/API/functionalities/load_unload/b.cpp
index ed89d66e1eb8..506af68528d2 100644
--- a/lldb/test/API/functionalities/load_unload/b.cpp
+++ b/lldb/test/API/functionalities/load_unload/b.cpp
@@ -5,8 +5,4 @@ int b_init()
 
 int b_global = b_init();
 
-int
-b_function ()
-{
-    return 500;
-}
+int LLDB_DYLIB_EXPORT b_function() { return 500; }

diff  --git a/lldb/test/API/functionalities/load_unload/d.cpp b/lldb/test/API/functionalities/load_unload/d.cpp
index 696b224e76ea..9e1408848377 100644
--- a/lldb/test/API/functionalities/load_unload/d.cpp
+++ b/lldb/test/API/functionalities/load_unload/d.cpp
@@ -5,8 +5,6 @@ int d_init()
 
 int d_global = d_init();
 
-int
-d_function ()
-{ // Find this line number within d_dunction().
-    return 700;
+int LLDB_DYLIB_EXPORT d_function() {
+  return 700; // Find this line number within d_dunction().
 }

diff  --git a/lldb/test/API/functionalities/load_unload/main.cpp b/lldb/test/API/functionalities/load_unload/main.cpp
index 4aae9c30db35..46fb5bba89d7 100644
--- a/lldb/test/API/functionalities/load_unload/main.cpp
+++ b/lldb/test/API/functionalities/load_unload/main.cpp
@@ -1,72 +1,57 @@
-#include <stdio.h>
-#include <dlfcn.h>
+#include "dylib.h"
 #include <limits.h>
-#include <string.h>
-#include <unistd.h>
-#include <libgen.h>
+#include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 
-int
-main (int argc, char const *argv[])
-{
-#if defined (__APPLE__)
-    const char *a_name = "@executable_path/libloadunload_a.dylib";
-    const char *c_name = "@executable_path/libloadunload_c.dylib";
-#else
-    const char *a_name = "libloadunload_a.so";
-    const char *c_name = "libloadunload_c.so";
-#endif
-    void *a_dylib_handle = NULL;
-    void *c_dylib_handle = NULL;
-    int (*a_function) (void);
-
-    a_dylib_handle = dlopen (a_name, RTLD_NOW); // Set break point at this line for test_lldb_process_load_and_unload_commands().
-    if (a_dylib_handle == NULL)
-    {
-        fprintf (stderr, "%s\n", dlerror());
-        exit (1);
-    }
-
-    a_function = (int (*) ()) dlsym (a_dylib_handle, "a_function");
-    if (a_function == NULL)
-    {
-        fprintf (stderr, "%s\n", dlerror());
-        exit (2);
-    }
-    printf ("First time around, got: %d\n", a_function ());
-    dlclose (a_dylib_handle);
-
-    c_dylib_handle = dlopen (c_name, RTLD_NOW);
-    if (c_dylib_handle == NULL)
-    {
-        fprintf (stderr, "%s\n", dlerror());
-        exit (3);
-    }
-    a_function = (int (*) ()) dlsym (c_dylib_handle, "c_function");
-    if (a_function == NULL)
-    {
-        fprintf (stderr, "%s\n", dlerror());
-        exit (4);
-    }
-
-    a_dylib_handle = dlopen (a_name, RTLD_NOW);
-    if (a_dylib_handle == NULL)
-    {
-        fprintf (stderr, "%s\n", dlerror());
-        exit (5);
-    }
-
-    a_function = (int (*) ()) dlsym (a_dylib_handle, "a_function");
-    if (a_function == NULL)
-    {
-        fprintf (stderr, "%s\n", dlerror());
-        exit (6);
-    }
-    printf ("Second time around, got: %d\n", a_function ());
-    dlclose (a_dylib_handle);
-
-    int d_function(void);
-    printf ("d_function returns: %d\n", d_function());
-
-    return 0;
+int main(int argc, char const *argv[]) {
+  const char *a_name = "loadunload_a";
+  const char *c_name = "loadunload_c";
+  void *a_dylib_handle = NULL;
+  void *c_dylib_handle = NULL; // Set break point at this line for test_lldb_process_load_and_unload_commands().
+  int (*a_function)(void);
+
+  a_dylib_handle = dylib_open(a_name);
+  if (a_dylib_handle == NULL) {
+    fprintf(stderr, "%s\n", dylib_last_error());
+    exit(1);
+  }
+
+  a_function = (int (*)())dylib_get_symbol(a_dylib_handle, "a_function");
+  if (a_function == NULL) {
+    fprintf(stderr, "%s\n", dylib_last_error());
+    exit(2);
+  }
+  printf("First time around, got: %d\n", a_function());
+  dylib_close(a_dylib_handle);
+
+  c_dylib_handle = dylib_open(c_name);
+  if (c_dylib_handle == NULL) {
+    fprintf(stderr, "%s\n", dylib_last_error());
+    exit(3);
+  }
+  a_function = (int (*)())dylib_get_symbol(c_dylib_handle, "c_function");
+  if (a_function == NULL) {
+    fprintf(stderr, "%s\n", dylib_last_error());
+    exit(4);
+  }
+
+  a_dylib_handle = dylib_open(a_name);
+  if (a_dylib_handle == NULL) {
+    fprintf(stderr, "%s\n", dylib_last_error());
+    exit(5);
+  }
+
+  a_function = (int (*)())dylib_get_symbol(a_dylib_handle, "a_function");
+  if (a_function == NULL) {
+    fprintf(stderr, "%s\n", dylib_last_error());
+    exit(6);
+  }
+  printf("Second time around, got: %d\n", a_function());
+  dylib_close(a_dylib_handle);
+
+  int LLDB_DYLIB_IMPORT d_function(void);
+  printf("d_function returns: %d\n", d_function());
+
+  return 0;
 }


        


More information about the lldb-commits mailing list