[lldb/test] Make TestLoadUnload compatible with windows

[labath]

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.

[labath]

include some changes I forgot (they're the ones that need cleaning up the most)

[jingham]

lldb/test/API/functionalities/load_unload/TestLoadUnload.py

414

You could also do this by making an SBLaunchInfo, adding the environment variable to it, and then you could use lldbutil.run_to_name_breakpoint. That would remove some of the boilerplate below.

[amccarth]

I like where this is going.

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

51

This leaks the buffer allocated for the message. I guess we don't expect this to happen often, so maybe that's not a big deal, and it's a terrible API to have to deal with.

lldb/test/API/functionalities/load_unload/TestLoadUnload.py

209

The env_cmd_string is going to eliminate ALL of the environment variables for the target except the one(s) that it explicitly sets. Is that what you intended?

The lldb help says:

Warning: The 'set' command re-sets the entire array or dictionary. If you just want to add, remove or update individual values (or add something to the end), use one of the other settings sub-commands: append, replace, insert-before or insert-after.

[labath]

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

51

yeah, that's just the test code and it's only purpose is to give some indication for when things go wrong. I've contemplated returning std::string here, but then I figured we may want to use this from some C code too and it may be better to just stick to C.

lldb/test/API/functionalities/load_unload/TestLoadUnload.py

209

That's not as bad as it sounds because target.env-vars (as of last week, anyway) will only contain the variables set by the user, and the host environment will be applied separately.

Nonetheless, using append does seem like a better choice here.

414

run_to_name_breakpoint does not seem like a completely good fit here as the test has assertions about numbers of breakpoint locations being found before running the process.

However, I think I've found an even better solution -- the test actually already contains logic for setting these paths, but it was hidden behind an if(!darwin) as it was not needed due to the use of @executable_path in dlopen calls. Now I've just removed the if and made that code slightly more generic.

[labath]

Cleaning up the patch. It should be ready now, I'm just going to give it one more spin on windows.

[labath]

Remove bogus runCmd. Everything should check out now.

[amccarth]

lldb/packages/Python/lldbsuite/test/make/Makefile.rules

477

I'm no expert in makefile syntax (especially not for Unix-y versions), but this looks weird, with the leading comma and the matching parenthesis for $(filter sitting near the end of the expression. Is this right?

[labath]

lldb/packages/Python/lldbsuite/test/make/Makefile.rules

477

Yes, it is.

The "leading" comma introduces a empty string as the first argument to ifeq. Then the $filter thingy will turn $OS into an empty string iff it matches one of the strings it got as an argument (technically it will just remove any subwords which match those strings, but since $OS is just a single word, the effect is the same).
So this is a very convoluted way of writing $(OS) in [NetBSD, Windows_NT] but:
a) it is consistent with other usages in these makefiles
b) I don't actually know of a better way to write that. If you do, let me know.

[amccarth]

Well, it looks to go _me_.

[compnerd]

@labath - can this get merged so that I can rebase and get D77287 merged?

lldb/packages/Python/lldbsuite/test/lldbplatformutil.py

 
 
 def createPlatformContext():
     if platformIsDarwin():
         return _PlatformContext('DYLD_LIBRARY_PATH', 'lib', 'dylib')
     elif getPlatform() in ("freebsd", "linux", "netbsd"):
         return _PlatformContext('LD_LIBRARY_PATH', 'lib', 'so')
     else:
-        return None
+        return _PlatformContext('PATH', '', 'dll')
 
 
 def hasChattyStderr(test_case):
     """Some targets produce garbage on the standard error output. This utility function
     determines whether the tests can be strict about the expected stderr contents."""
     if match_android_device(test_case.getArchitecture(), ['aarch64'], range(22, 25+1)):
         return True  # The dynamic linker on the device will complain about unknown DT entries
     return False

lldb/packages/Python/lldbsuite/test/make/Makefile.rules

else
     LDFLAGS += -stdlib=libc++
   endif
 endif
 
 #----------------------------------------------------------------------
 # Additional system libraries
 #----------------------------------------------------------------------
 ifeq (1,$(USE_LIBDL))
-  ifneq ($(OS),NetBSD)
+  ifeq (,$(filter $(OS), NetBSD Windows_NT))
     LDFLAGS += -ldl
   endif
 endif
 
 #----------------------------------------------------------------------
 # dylib settings
 #----------------------------------------------------------------------
 

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

+#ifndef LLDB_TEST_DYLIB_H
+#define LLDB_TEST_DYLIB_H
+
+#include <string.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];
+  strcat(strcat(strcpy(fullname, 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

lldb/packages/Python/lldbsuite/test/make/test_common.h

 // 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)
 #define LLVM_PRETTY_FUNCTION __FUNCSIG__
 #else
 #define LLVM_PRETTY_FUNCTION LLVM_PRETTY_FUNCTION
 #endif
 

lldb/test/API/functionalities/load_unload/Makefile

 
 lib_d:
   $(MAKE) -f $(MAKEFILE_RULES) \
     DYLIB_ONLY=YES DYLIB_CXX_SOURCES=d.cpp DYLIB_NAME=loadunload_d
 ifeq ($(OS),Darwin)
   install_name_tool -id @executable_path/libloadunload_d.dylib libloadunload_d.dylib
 endif
 
-hidden_lib_d:
-  mkdir -p hidden
+hidden:
+  mkdir 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

lldb/test/API/functionalities/load_unload/TestLoadUnload.py

 """
 Test that breakpoint by symbol name works correctly with dynamic libs.
 """
 
 from __future__ import print_function
 
 
 import os
 import re
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 
-@skipIfWindows  # Windows doesn't have dlopen and friends, dynamic libraries work differently
 class LoadUnloadTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     NO_DEBUG_INFO_TESTCASE = True
 
     def setUp(self):
         # Call super's setUp().
def test_dyld_library_path(self):
 
         # 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)
 
         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.
         self.expect("run", substrs=["return", "700"])
 
         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
         self.runCmd(env_cmd_string)
 
         # This time, the hidden library should be picked up.
         self.expect("run", substrs=["return", "12345"])
 
     @expectedFailureAll(
         bugnumber="llvm.org/pr25805",
         hostoslist=["windows"],
         triple='.*-android')
     @skipIfFreeBSD  # llvm.org/pr14424 - missing FreeBSD Makefiles/testcase support
-    @skipIfWindows  # Windows doesn't have dlopen and friends, dynamic libraries work differently
+    @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()
 
     @expectedFailureAll(
         bugnumber="llvm.org/pr25805",
         hostoslist=["windows"],
         triple='.*-android')
     @skipIfFreeBSD  # llvm.org/pr14424 - missing FreeBSD Makefiles/testcase support
-    @skipIfWindows  # Windows doesn't have dlopen and friends, dynamic libraries work differently
+    @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()
 
     def run_lldb_process_load_and_unload_commands(self):
         """Test that lldb process load/unload command work correctly."""
         self.copy_shlibs_to_remote()
 
def run_lldb_process_load_and_unload_commands(self):
             dylibName,
             patterns=[
                 "Unloading .* with index %s.*ok" %
                 index])
 
         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()
 
     def run_load_unload(self):
         """Test breakpoint by name works correctly with dlopen'ing."""
         self.copy_shlibs_to_remote()
 
def run_load_unload(self):
                              'a_function',
                              'stop reason = breakpoint'])
 
         # The breakpoint should have a hit count of 2.
         self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
                     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 differently
     @expectedFailureAll(archs="aarch64", oslist="linux",
                         bugnumber="https://bugs.llvm.org/show_bug.cgi?id=27806")
     def test_step_over_load(self):
         self.setSvr4Support(False)
         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 differently
     @expectedFailureAll(archs="aarch64", oslist="linux",
                         bugnumber="https://bugs.llvm.org/show_bug.cgi?id=27806")
     def test_step_over_load_with_svr4(self):
         self.setSvr4Support(True)
         self.run_step_over_load()
 
     def run_step_over_load(self):
         """Test stepping over code that loads a shared library works correctly."""
def run_step_over_load(self):
         self.expect("thread list", "step over succeeded.",
                     substrs=['stopped',
                              'stop reason = step over'])
 
     # We can't find a breakpoint location for d_init before launching because
     # 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 differently
     @expectedFailureNetBSD
     def test_static_init_during_load(self):
         """Test that we can set breakpoints correctly in static initializers"""
         self.copy_shlibs_to_remote()
 
         exe = self.getBuildArtifact("a.out")
         self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
 
         a_init_bp_num = lldbutil.run_break_set_by_symbol(
             self, "a_init", num_expected_locations=0)
         b_init_bp_num = lldbutil.run_break_set_by_symbol(
             self, "b_init", num_expected_locations=0)
         d_init_bp_num = lldbutil.run_break_set_by_symbol(
             self, "d_init", num_expected_locations=1)
 
         self.runCmd("run", RUN_SUCCEEDED)
 
         self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
                     substrs=['stopped',
                              'd_init',
                              'stop reason = breakpoint %d' % d_init_bp_num])
 
         self.runCmd("continue")
         self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
                     substrs=['stopped',
                              'b_init',
                              'stop reason = breakpoint %d' % b_init_bp_num])
         self.expect("thread backtrace",
                     substrs=['b_init',
-                             'dlopen',
+                             'dylib_open',
                              'main'])
 
         self.runCmd("continue")
         self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
                     substrs=['stopped',
                              'a_init',
                              'stop reason = breakpoint %d' % a_init_bp_num])
         self.expect("thread backtrace",
                     substrs=['a_init',
-                             'dlopen',
+                             'dylib_open',
                              'main'])

lldb/test/API/functionalities/load_unload/a.cpp

-extern int b_function ();
+extern LLDB_DYLIB_IMPORT int b_function();
 
 int a_init()
 {
     return 234;
 }
 
 int a_global = a_init();
 
 extern "C" int
 a_function ()
 {
     return b_function ();
 }

lldb/test/API/functionalities/load_unload/b.cpp

 int b_init()
 {
     return 345;
 }
 
 int b_global = b_init();
 
-int
-b_function ()
-{
-    return 500;
-}
+int LLDB_DYLIB_EXPORT b_function() { return 500; }

lldb/test/API/functionalities/load_unload/d.cpp

 int d_init()
 {
     return 123;
 }
 
 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().
 }

lldb/test/API/functionalities/load_unload/main.cpp

-#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);
+  const char *a_name = "loadunload_a";
+  const char *c_name = "loadunload_c";
+  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 = 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_dylib_handle = dylib_open(a_name);
+  if (a_dylib_handle == NULL) {
+    fprintf(stderr, "%s\n", dylib_last_error());
+    exit(1);
 
-    a_function = (int (*) ()) dlsym (a_dylib_handle, "a_function");
-    if (a_function == NULL)
-    {
-        fprintf (stderr, "%s\n", dlerror());
-        exit (2);
-    }
+  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 ());
-    dlclose (a_dylib_handle);
+    dylib_close(a_dylib_handle);
 
-    c_dylib_handle = dlopen (c_name, RTLD_NOW);
+    c_dylib_handle = dylib_open(c_name);
     if (c_dylib_handle == NULL)
     {
-        fprintf (stderr, "%s\n", dlerror());
-        exit (3);
+      fprintf(stderr, "%s\n", dylib_last_error());
+      exit(3);
     }
-    a_function = (int (*) ()) dlsym (c_dylib_handle, "c_function");
+    a_function = (int (*)())dylib_get_symbol(c_dylib_handle, "c_function");
     if (a_function == NULL)
     {
-        fprintf (stderr, "%s\n", dlerror());
-        exit (4);
+      fprintf(stderr, "%s\n", dylib_last_error());
+      exit(4);
     }
 
-    a_dylib_handle = dlopen (a_name, RTLD_NOW);
+    a_dylib_handle = dylib_open(a_name);
     if (a_dylib_handle == NULL)
     {
-        fprintf (stderr, "%s\n", dlerror());
-        exit (5);
+      fprintf(stderr, "%s\n", dylib_last_error());
+      exit(5);
     }
 
-    a_function = (int (*) ()) dlsym (a_dylib_handle, "a_function");
+    a_function = (int (*)())dylib_get_symbol(a_dylib_handle, "a_function");
     if (a_function == NULL)
     {
-        fprintf (stderr, "%s\n", dlerror());
-        exit (6);
+      fprintf(stderr, "%s\n", dylib_last_error());
+      exit(6);
     }
     printf ("Second time around, got: %d\n", a_function ());
-    dlclose (a_dylib_handle);
+    dylib_close(a_dylib_handle);
 
-    int d_function(void);
+    int LLDB_DYLIB_IMPORT d_function(void);
     printf ("d_function returns: %d\n", d_function());
 
     return 0;
 }