[lldb/PlatformPOSIX] Change LoadImage default to RTLD_LAZY

[vsk]

Summary

In general, it seems like the debugger should allow programs to load & run with
libraries as far as possible, instead of defaulting to being super-picky about
unavailable symbols.

This is critical on macOS/Darwin, as libswiftCore.dylib may 1) export a version
symbol using @available markup and then 2) expect that other exported APIs are
only dynamically used once the version symbol is checked. We can't open a
version of the library built with a bleeding-edge SDK on an older OS without
RTLD_LAXY (or pervasive/expensive @available markup added to dyld APIs).

See: https://lists.llvm.org/pipermail/lldb-dev/2021-March/016796.html

[vsk]

This needs a test.

[shafik]

Thank you for adding the commentary on what RTLD_LAZY

[vsk]

The best way I can think of to test this is to:

• Make a library, T1, out of:

% cat t1.c
extern void use();
void f1() {}
void f2() { use(); }

• Make a library, T2, out of:

% cat t2.c
void use() {}

• Link T1.dylib against T2.dylib, so that the reference to use isn't undefined.
• Now, rebuild T2.dylib using an empty source file (delete the definition of use).
• Launch a process, then do "process load T1.dylib"; this will only succeed under RTLD_LAZY.

Any concerns?

[JDevlieghere]

This and the proposed test sound reasonable to me.

lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp

581

I don't think the libswiftCore.dylib example adds much.

[vsk]

• Add a test. It's Darwin-specific as I couldn't work out how to pass the .dylib path to the linker in a platform-agnostic way.
• Trim the comment.

[vsk]

• Make the test generic (not Darwin-specific)

[kastiglione]

lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp

610

Instead of DLOPEN_OPTIONS and string concatenation, what do you think about defining RTLD_LAZY inside this string of code?

static const int RTLD_LAZY = 0x1;

/// code…

result_ptr->image_ptr = dlopen(name, RTLD_LAZY);

The main reason I suggest this is putting the name inline like that makes it more grep friendly.

[vsk]

• Define RTLD_LAZY in the expression as suggested

[vsk]

• Delete an unused '#undef DLOPEN_OPTIONS'

[JDevlieghere]

LGTM

[kastiglione]

🚢

[vsk]

Add asserts checking that the library paths exist, and restrict the test to Darwin. See https://bugs.llvm.org/show_bug.cgi?id=49656

[jankratochvil]

I think you should remove: lldb/test/API/.lit_test_times.txt

[vsk]

Sorry about that. The file was removed in 8248dd91d7f042893d4a605e98d19cb1b89a44d4.

lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp

 std::unique_ptr<UtilityFunction>
 PlatformPOSIX::MakeLoadImageUtilityFunction(ExecutionContext &exe_ctx,
                                             Status &error) {
   // Remember to prepend this with the prefix from
   // GetLibdlFunctionDeclarations. The returned values are all in
   // __lldb_dlopen_result for consistency. The wrapper returns a void * but
   // doesn't use it because UtilityFunctions don't work with void returns at
   // present.
+  //
+  // Use lazy binding so as to not make dlopen()'s success conditional on
+  // forcing every symbol in the library.
+  //
+  // In general, the debugger should allow programs to load & run with
+  // libraries as far as they can, instead of defaulting to being super-picky
+  // about unavailable symbols.
+  //
+  // The value "1" appears to imply lazy binding (RTLD_LAZY) on both Darwin
+  // and other POSIX OSes.
   static const char *dlopen_wrapper_code = R"(
+  const int RTLD_LAZY = 1;
+
   struct __lldb_dlopen_result {
     void *image_ptr;
     const char *error_str;
   };
   
   extern void *memcpy(void *, const void *, size_t size);
   extern size_t strlen(const char *);
   
 
   void * __lldb_dlopen_wrapper (const char *name, 
                                 const char *path_strings,
                                 char *buffer,
                                 __lldb_dlopen_result *result_ptr)
   {
     // This is the case where the name is the full path:
     if (!path_strings) {
-      result_ptr->image_ptr = dlopen(name, 2);
+      result_ptr->image_ptr = dlopen(name, RTLD_LAZY);
       if (result_ptr->image_ptr)
         result_ptr->error_str = nullptr;
       return nullptr;
     }
     
     // This is the case where we have a list of paths:
     size_t name_len = strlen(name);
     while (path_strings && path_strings[0] != '\0') {
       size_t path_len = strlen(path_strings);
       memcpy((void *) buffer, (void *) path_strings, path_len);
       buffer[path_len] = '/';
       char *target_ptr = buffer+path_len+1; 
       memcpy((void *) target_ptr, (void *) name, name_len + 1);
-      result_ptr->image_ptr = dlopen(buffer, 2);
+      result_ptr->image_ptr = dlopen(buffer, RTLD_LAZY);
       if (result_ptr->image_ptr) {
         result_ptr->error_str = nullptr;
         break;
       }
       result_ptr->error_str = dlerror();
       path_strings = path_strings + path_len + 1;
     }
     return nullptr;

lldb/test/API/.lit_test_times.txt

+2.777875e+00 functionalities/load_lazy/TestLoadUsingLazyBind.py

lldb/test/API/functionalities/load_lazy/Makefile

+CXX_SOURCES := main.cpp
+USE_LIBDL := 1
+
+all: t2_0 t2_1 t1 a.out
+
+include Makefile.rules
+
+t1: t2_0
+  $(MAKE) VPATH=$(SRCDIR) -f $(MAKEFILE_RULES) \
+    DYLIB_ONLY=YES DYLIB_C_SOURCES=t1.c DYLIB_NAME=t1 LD_EXTRAS="-L. -lt2_0"
+
+t2_0:
+  $(MAKE) VPATH=$(SRCDIR) -f $(MAKEFILE_RULES) \
+    DYLIB_ONLY=YES DYLIB_C_SOURCES=t2_0.c DYLIB_NAME=t2_0
+
+t2_1:
+  $(MAKE) VPATH=$(SRCDIR) -f $(MAKEFILE_RULES) \
+    DYLIB_ONLY=YES DYLIB_C_SOURCES=t2_1.c DYLIB_NAME=t2_1

lldb/test/API/functionalities/load_lazy/TestLoadUsingLazyBind.py

+"""
+Test that SBProcess.LoadImageUsingPaths uses RTLD_LAZY
+"""
+
+
+
+import os
+import shutil
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class LoadUsingLazyBind(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+    NO_DEBUG_INFO_TESTCASE = True
+
+    @skipIfRemote
+    @skipIfWindows # The Windows platform doesn't implement DoLoadImage.
+    # Failing for unknown reasons on Linux, see
+    # https://bugs.llvm.org/show_bug.cgi?id=49656.
+    @skipUnlessDarwin
+    def test_load_using_lazy_bind(self):
+        """Test that we load using RTLD_LAZY"""
+
+        self.build()
+        wd = os.path.realpath(self.getBuildDir())
+
+        ext = '.so'
+        if self.platformIsDarwin():
+            ext = '.dylib'
+
+        def make_lib_path(name):
+            libpath = os.path.join(wd, name + ext)
+            self.assertTrue(os.path.exists(libpath))
+            return libpath
+
+        libt1 = make_lib_path('libt1')
+        libt2_0 = make_lib_path('libt2_0')
+        libt2_1 = make_lib_path('libt2_1')
+
+        # Overwrite t2_0 with t2_1 to delete the definition of `use`.
+        shutil.copy(libt2_1, libt2_0)
+
+        # Launch a process and break
+        (target, process, thread, _) = lldbutil.run_to_source_breakpoint(self,
+                                                "break here",
+                                                lldb.SBFileSpec("main.cpp"))
+
+        # Load libt1; should fail unless we use RTLD_LAZY
+        error = lldb.SBError()
+        lib_spec = lldb.SBFileSpec('libt1' + ext)
+        paths = lldb.SBStringList()
+        paths.AppendString(wd)
+        out_spec = lldb.SBFileSpec()
+        token = process.LoadImageUsingPaths(lib_spec, paths, out_spec, error)
+        self.assertNotEqual(token, lldb.LLDB_INVALID_IMAGE_TOKEN, "Got a valid token")
+
+        # Calling `f1()` should return 5.
+        frame = thread.GetFrameAtIndex(0)
+        val = frame.EvaluateExpression("f1()")
+        self.assertTrue(val.IsValid())
+        self.assertEquals(val.GetValueAsSigned(-1), 5)

lldb/test/API/functionalities/load_lazy/categories

+basic_process

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

+int main() {
+  return 0; // break here
+}

lldb/test/API/functionalities/load_lazy/t1.c

+extern void use();
+int f1() { return 5; }
+void f2() { use(); }

lldb/test/API/functionalities/load_lazy/t2_0.c

+void use() {}