noexternal 1/2: refactor testsuite spawnLldbMi args->exe+args

[jankratochvil]

Summary

Currently spawnLldbMi accepts both lldb-mi options and executable to debug as a single parameter. Split them.
As in the next patch we will need to execute one lldb-mi command before loading the exe. Therefore we can no longer use the exe as lldb-mi command-line parameter as then there is no way to execute a command before loading exe specified as lldb-mi command-line parameter.
LocateExecutableSymbolFileDsym should be static, that is also a little refactorization.

[labath]

packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py

53 ↗

(On Diff #178796)

Shouldn't you also expect the ^done response here or something similar?

[jankratochvil]

Thanks for the review!

packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py

53 ↗

(On Diff #178796)

This would break compatibility with previous behavior of this function, see my test_lldbmi_specialchars comment below.

packages/Python/lldbsuite/test/tools/lldb-mi/syntax/TestMiSyntax.py

63 ↗

(On Diff #178796)

Here you can see just by passing complicated_myexe as spawnLldbMi parameter the code still expects to be able to match the -file-exec-and-symbols command.

[labath]

Ok, if that's how things worked previously, then this lgtm.

[jankratochvil]

I have put there a comment:

             self.runCmd("-file-exec-and-symbols \"%s\"" % exe)
+            # Testcases expect to be able to match output of this command,
+            # see test_lldbmi_specialchars.

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py

def tearDown(self):
         if self.TraceOn():
             print("\n\nContents of %s:" % self.mylog)
             try:
                 print(open(self.mylog, "r").read())
             except IOError:
                 pass
         Base.tearDown(self)
 
-    def spawnLldbMi(self, args=None):
+    def spawnLldbMi(self, exe=None, args=None):
         import pexpect
         self.child = pexpect.spawn("%s --interpreter %s" % (
             self.lldbMiExec, args if args else ""), cwd=self.getBuildDir())
         self.child.setecho(True)
         self.mylog = self.getBuildArtifact("child.log")
         self.child.logfile_read = open(self.mylog, "w")
         # wait until lldb-mi has started up and is ready to go
         self.expect(self.child_prompt, exactly=True)
+        if exe:
+            self.runCmd("-file-exec-and-symbols \"%s\"" % exe)
+            # Testcases expect to be able to match output of this command,
+            # see test_lldbmi_specialchars.
 
     def runCmd(self, cmd):
         self.child.sendline(cmd)
 
     def expect(self, pattern, exactly=False, *args, **kwargs):
         if exactly:
             return self.child.expect_exact(pattern, *args, **kwargs)
         return self.child.expect(pattern, *args, **kwargs)

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/startup_options/TestMiStartupOptions.py

class MiStartupOptionsTestCase(lldbmi_testcase.MiTestCaseBase):
 
     @skipIfRemote   # We do not currently support remote debugging via the MI.
     @skipIfWindows  # llvm.org/pr24452: Get lldb-mi tests working on Windows
     @skipIfFreeBSD  # llvm.org/pr22411: Failure presumably due to known thread races
     @skipIfDarwin
     def test_lldbmi_executable_option_file(self):
         """Test that 'lldb-mi --interpreter %s' loads executable file."""
 
-        self.spawnLldbMi(args="%s" % self.myexe)
+        self.spawnLldbMi(exe=self.myexe)
 
         # Test that the executable is loaded when file was specified
         self.expect("-file-exec-and-symbols \"%s\"" % self.myexe)
         self.expect("\^done")
 
         # Test that lldb-mi is ready when executable was loaded
         self.expect(self.child_prompt, exactly=True)
 
class MiStartupOptionsTestCase(lldbmi_testcase.MiTestCaseBase):
     @skipIfFreeBSD  # llvm.org/pr22411: Failure presumably due to known thread races
     @skipIfDarwin
     def test_lldbmi_executable_option_unknown_file(self):
         """Test that 'lldb-mi --interpreter %s' fails on unknown executable file."""
 
         # Prepare path to executable
         path = "unknown_file"
 
-        self.spawnLldbMi(args="%s" % path)
+        self.spawnLldbMi(exe=path)
 
         # Test that the executable isn't loaded when unknown file was specified
         self.expect("-file-exec-and-symbols \"%s\"" % path)
         self.expect(
             "\^error,msg=\"Command 'file-exec-and-symbols'. Target binary '%s' is invalid. error: unable to find executable for '%s'\"" %
             (path, path))
 
         # Test that lldb-mi is ready when executable was loaded
         self.expect(self.child_prompt, exactly=True)
 
     @skipIfRemote   # We do not currently support remote debugging via the MI.
     @skipIfWindows  # llvm.org/pr24452: Get lldb-mi tests working on Windows
     @skipIfFreeBSD  # llvm.org/pr22411: Failure presumably due to known thread races
     @skipIfDarwin
     def test_lldbmi_executable_option_absolute_path(self):
         """Test that 'lldb-mi --interpreter %s' loads executable which is specified via absolute path."""
 
         # Prepare path to executable
-        self.spawnLldbMi(args="%s" % self.myexe)
+        self.spawnLldbMi(exe=self.myexe)
 
         # Test that the executable is loaded when file was specified using
         # absolute path
         self.expect("-file-exec-and-symbols \"%s\"" % self.myexe)
         self.expect("\^done")
 
         # Test that lldb-mi is ready when executable was loaded
         self.expect(self.child_prompt, exactly=True)
 
         # Run
         self.runCmd("-exec-run")
         self.expect("\^running")
         self.expect("\*stopped,reason=\"exited-normally\"")
 
     @skipIfRemote   # We do not currently support remote debugging via the MI.
     @skipIfWindows  # llvm.org/pr24452: Get lldb-mi tests working on Windows
     @skipIfFreeBSD  # llvm.org/pr22411: Failure presumably due to known thread races
     @skipIfDarwin
     def test_lldbmi_executable_option_relative_path(self):
         """Test that 'lldb-mi --interpreter %s' loads executable which is specified via relative path."""
 
         # Prepare path to executable
         path = os.path.relpath(self.myexe, self.getBuildDir())
-        self.spawnLldbMi(args="%s" % path)
+        self.spawnLldbMi(exe=path)
 
         # Test that the executable is loaded when file was specified using
         # relative path
         self.expect("-file-exec-and-symbols \"%s\"" % path)
         self.expect("\^done")
 
         # Test that lldb-mi is ready when executable was loaded
         self.expect(self.child_prompt, exactly=True)
 
         # Run
         self.runCmd("-exec-run")
         self.expect("\^running")
         self.expect("\*stopped,reason=\"exited-normally\"")
 
     @skipIfWindows  # llvm.org/pr24452: Get lldb-mi tests working on Windows
     @skipIfFreeBSD  # llvm.org/pr22411: Failure presumably due to known thread races
     @skipIfDarwin
     def test_lldbmi_executable_option_unknown_path(self):
         """Test that 'lldb-mi --interpreter %s' fails on executable file which is specified via unknown path."""
 
         # Prepare path to executable
         path = "unknown_dir" + self.myexe
 
-        self.spawnLldbMi(args="%s" % path)
+        self.spawnLldbMi(exe=path)
 
         # Test that the executable isn't loaded when file was specified using
         # unknown path
         self.expect("-file-exec-and-symbols \"%s\"" % path)
         self.expect(
             "\^error,msg=\"Command 'file-exec-and-symbols'. Target binary '%s' is invalid. error: unable to find executable for '%s'\"" %
             (path, path))
 
class MiStartupOptionsTestCase(lldbmi_testcase.MiTestCaseBase):
     @skipIfRemote   # We do not currently support remote debugging via the MI.
     @skipIfWindows  # llvm.org/pr24452: Get lldb-mi tests working on Windows
     @skipIfFreeBSD  # llvm.org/pr22411: Failure presumably due to known thread races
     @skipIfDarwin
     def test_lldbmi_log_option(self):
         """Test that 'lldb-mi --log' creates a log file in the current directory."""
 
         logDirectory = self.getBuildDir()
-        self.spawnLldbMi(args="%s --log" % self.myexe)
+        self.spawnLldbMi(exe=self.myexe, args="--log")
 
         # Test that the executable is loaded when file was specified
         self.expect("-file-exec-and-symbols \"%s\"" % self.myexe)
         self.expect("\^done")
 
         # Test that lldb-mi is ready when executable was loaded
         self.expect(self.child_prompt, exactly=True)
 
class MiStartupOptionsTestCase(lldbmi_testcase.MiTestCaseBase):
     @skipIfDarwin
     def test_lldbmi_log_directory_option(self):
         """Test that 'lldb-mi --log --log-dir' creates a log file in the directory specified by --log-dir."""
 
         # Create log in temp directory
         import tempfile
         logDirectory = tempfile.gettempdir()
 
-        self.spawnLldbMi(
-            args="%s --log --log-dir=%s" %
-            (self.myexe, logDirectory))
+        self.spawnLldbMi(exe=self.myexe,
+            args="--log --log-dir=%s" % logDirectory)
 
         # Test that the executable is loaded when file was specified
         self.expect("-file-exec-and-symbols \"%s\"" % self.myexe)
         self.expect("\^done")
 
         # Test that lldb-mi is ready when executable was loaded
         self.expect(self.child_prompt, exactly=True)
 

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/syntax/TestMiSyntax.py

class MiSyntaxTestCase(lldbmi_testcase.MiTestCaseBase):
     def test_lldbmi_specialchars(self):
         """Test that 'lldb-mi --interpreter' handles complicated strings."""
 
         # Create an alias for myexe
         complicated_myexe = self.getBuildArtifact("C--mpl-x file's`s @#$%^&*()_+-={}[]| name")
         os.symlink(self.myexe, complicated_myexe)
         self.addTearDownHook(lambda: os.unlink(complicated_myexe))
 
-        self.spawnLldbMi(args="\"%s\"" % complicated_myexe)
+        self.spawnLldbMi(exe=complicated_myexe)
 
         # Test that the executable was loaded
         self.expect(
             "-file-exec-and-symbols \"%s\"" %
             complicated_myexe, exactly=True)
         self.expect("\^done")
 
         # Check that it was loaded correctly

lldb/trunk/source/Host/common/Symbols.cpp

if (::LookForDsymNextToExecutablePath (module_spec, exec_fspec, dsym_fspec)) {
         parent_dirs.RemoveLastPathComponent();
       }
     }
   }
   dsym_fspec.Clear();
   return false;
 }
 
-FileSpec LocateExecutableSymbolFileDsym(const ModuleSpec &module_spec) {
+static FileSpec LocateExecutableSymbolFileDsym(const ModuleSpec &module_spec) {
   const FileSpec *exec_fspec = module_spec.GetFileSpecPtr();
   const ArchSpec *arch = module_spec.GetArchitecturePtr();
   const UUID *uuid = module_spec.GetUUIDPtr();
 
   static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
   Timer scoped_timer(
       func_cat,
       "LocateExecutableSymbolFileDsym (file = %s, arch = %s, uuid = %p)",