[lldb] Only set the executable module for a target once

[teemperor]

Summary

When we try to find the executable module for our target we don't check
if we already have an executable module set. This causes that when debugging
a program that dlopens another executable, LLDB will take that other executable
as the new executable of the target (which causes that future launches of the
target will launch the dlopen'd executable instead of the original executable).

This just adds a check that we only set the executable when we haven't already
found one.

Fixes rdar://63443099

[jasonmolenda]

This looks good to me, thanks Raphael.

[jingham]

I don't think that test is right. There's no guarantee that your GetExecutable().GetFilename() runs before or after the dlopen in the process has run. If it runs before you aren't testing anything.

Can you set a second breakpoint at the return i and do lldbutil.continue_to_breakpoint instead of just Continue?

[teemperor]

@jingham Yeah that is a good point. Updated the test. Thanks!

[jingham]

lldb/test/API/functionalities/dlopen_other_executable/TestDlopenOtherExecutable.py

23

One little nit, breakpoint.IsValid doesn't actually check very much. You would have to miss-call a breakpoint API for it to return an invalid breakpoint. After all, breakpoints are search kernels, and IsValid just tells you whether you made a valid search kernel. What you really want to check here is whether your breakpoint got any locations. I think this mistake is made a bunch of times in the testsuite...

It's not terribly vital because if you didn't "continue_to_breakpoint" is going to return no stopped threads.

[teemperor]

• Fix breakpoint validation (thanks Jim!)

[teemperor]

• Disable the test on Linux as glibc's dlopen doesn't support opening executables.

[teemperor]

@jingham I assume this just slipped out of your review queue because Jason already accepted it, so I'll go ahead and land this. I can fix any further issues post-commit (users are pinging me to land this).

lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp

if (image_infos[i].header.filetype == llvm::MachO::MH_DYLINKER) {
         // dyld
         dyld_idx = i;
       }
     } else if (image_infos[i].header.filetype == llvm::MachO::MH_EXECUTE) {
       exe_idx = i;
     }
   }
 
-  if (exe_idx != UINT32_MAX) {
+  // Set the target executable if we haven't found one so far.
+  if (exe_idx != UINT32_MAX && !target.GetExecutableModule()) {
     const bool can_create = true;
     ModuleSP exe_module_sp(FindTargetModuleForImageInfo(image_infos[exe_idx],
                                                         can_create, nullptr));
     if (exe_module_sp) {
       LLDB_LOGF(log, "Found executable module: %s",
                 exe_module_sp->GetFileSpec().GetPath().c_str());
       target.GetImages().AppendIfNeeded(exe_module_sp);
       UpdateImageLoadAddress(exe_module_sp.get(), image_infos[exe_idx]);

lldb/test/API/functionalities/dlopen_other_executable/Makefile

+C_SOURCES := main.c
+USE_LIBDL := 1
+
+other:
+  $(MAKE) -f $(MAKEFILE_RULES) C_SOURCES=other.c EXE=other
+all: other
+
+include Makefile.rules

lldb/test/API/functionalities/dlopen_other_executable/TestDlopenOtherExecutable.py

+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    @skipIfRemote
+    @skipIfWindows
+    # glibc's dlopen doesn't support opening executables.
+    # https://sourceware.org/bugzilla/show_bug.cgi?id=11754
+    @skipIfLinux
+    @no_debug_info_test
+    def test(self):
+        self.build()
+        # Launch and stop before the dlopen call.
+        lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.c"))
+
+        # Delete the breakpoint we no longer need.
+        self.target().DeleteAllBreakpoints()
+
+        # Check that the executable is the test binary.
+        self.assertEqual(self.target().GetExecutable().GetFilename(), "a.out")
+
+        # Continue so that dlopen is called.
+        breakpoint = self.target().BreakpointCreateBySourceRegex(
+            "// break after dlopen", lldb.SBFileSpec("main.c"))
+        self.assertNotEqual(breakpoint.GetNumResolvedLocations(), 0)
+        stopped_threads = lldbutil.continue_to_breakpoint(self.process(), breakpoint)
+        self.assertEqual(len(stopped_threads), 1)
+
+        # Check that the executable is still the test binary and not "other".
+        self.assertEqual(self.target().GetExecutable().GetFilename(), "a.out")
+
+        # Kill the process and run the program again.
+        err = self.process().Kill()
+        self.assertTrue(err.Success(), str(err))
+
+        # Test that we hit the breakpoint after dlopen.
+        lldbutil.run_to_breakpoint_do_run(self, self.target(), breakpoint)

lldb/test/API/functionalities/dlopen_other_executable/main.c

+#include <dlfcn.h>
+#include <assert.h>
+
+int main() {
+  int i = 0; // break here
+  // dlopen the 'other' test executable.
+  int h = dlopen("other", RTLD_LAZY);
+  assert(h && "dlopen failed?");
+  return i; // break after dlopen
+}

lldb/test/API/functionalities/dlopen_other_executable/other.c

+int main() {}