Test cleanups

[adrian-prantl]

This fixes a race condition (two temp files called "main.o"), works around an issue with hitting a source regex bkpt in a dylib, cleans up a Makefile, and fixes a misunderstanding about how strcat works (ie: not like strdup).

[adrian-prantl]

@swift-ci test

[adrian-prantl]

@jimingham I don't feel good about this change, but it was necessary. Is this supposed to work for a regex breakpoint in a dylib that is going to be dlopened later?

[adrian-prantl]

I wonder if this worked due to a freak accident before: main.swift main.o file replaced the main.m main.o and so the breakpoint was resolved at startup even though the dylib wasn't yet loaded?

[jimingham]

That should work but it looks like there's a bug in the SBTarget bindings. This works:

(lldb) break set -p "Stop here if you can" -f dylib.c -s libdylib.dylib
Breakpoint 1: no locations (pending).
WARNING:  Unable to resolve breakpoint to any actual locations.
(lldb) run
Process 44860 launched: '/tmp/had_dylib/loadit' (x86_64)
1 location added to breakpoint 1
Process 44860 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
    frame #0: 0x000000010015bf64 libdylib.dylib`my_function at dylib.c:5
   2   	
   3   	void
   4   	my_function() {
-> 5   	  printf("Stop here if you can.\n");
    	  ^
   6   	}
Target 0: (loadit) stopped.

But this, which should be equivalent, does not:

(lldb) script
Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D.
>>> files = lldb.SBFileSpecList()
>>> files.Append(lldb.SBFileSpec("dylib.c"))
>>> shlibs = lldb.SBFileSpecList()
>>> shlibs.Append(lldb.SBFileSpec("libdylib.dylib"))
>>> bkpt = lldb.target.BreakpointCreateBySourceRegex("Stop here if you can", shlibs, files)
>>> quit
(lldb) break list
Current breakpoints:
1: source regex = "Stop here if you can", exact_match = 0, module = libdylib.dylib, locations = 0 (pending)

(lldb) run
Process 44892 launched: '/tmp/had_dylib/loadit' (x86_64)
Stop here if you can.
Process 44892 exited with status = 0 (0x00000000) 

[jimingham]

We should also check that run_to_source_breakpoint does the right thing with the exe_module & file arguments.

[jimingham]

I am pretty sure this code is not using run_to_source_breakpoint correctly. The exe_name argument is the name of the module you are going to use to create the target. The bkpt_module argument is for the name of the module you are supposed to set the breakpoint in. If that's not specified, we set it in the main executable module.

[jimingham]

Interesting. The test code you had above used a different version of BreakpointCreateBySourceRegex, not passing lists but a FileSpec for the file and a char * for the module name. That one does seem to work, and also doesn't require the module, only the file name:

(lldb) script
Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D.
>>> bkpt = lldb.target.BreakpointCreateBySourceRegex("Stop here if you can", lldb.SBFileSpec("dylib.c"))
>>> quit
(lldb) run
Process 44942 launched: '/tmp/had_dylib/loadit' (x86_64)
1 location added to breakpoint 1
Process 44942 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
    frame #0: 0x000000010015bf64 libdylib.dylib`my_function at dylib.c:5
   2   	
   3   	void
   4   	my_function() {
-> 5   	  printf("Stop here if you can.\n");
    	  ^
   6   	}
Target 0: (loadit) stopped.

[jimingham]

So IIUC, this should fail at the point where you try to set the breakpoint in run_to_source_breakpoint with the error that the breakpoint has no locations, but if you changed run_to_source_breakpoint to take a parameter to turn off the test, this should work.

[jimingham]

TL;DR

The overload of SBTarget::CreateBreakpointWithSourceRegex that takes a list of files & a list of modules doesn't work, but lldbutil.run_to_source_breakpoint doesn't use that one, it uses the one that takes on file and one module, and that one does work.

lldbutil.run_to_source_breakpoint asserts that the breakpoint have at least one location pre run, which won't be the case for your test. Probably best to fix that by adding a has_locations_before_run parameter to run_to_source_breakpoint for the case where the breakpoint is expected to only take in a dynamically loaded library.

[jimingham]

The reason your fix works is because it DOESN'T check that bpkt has any locations when you set it. It would be better to fix that by fixing run_to_source_breakpoint.

[jimingham]

Like:

diff --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py b/lldb/packages/Python/lldbsuite/test/lldbutil.py
index 20a6d28274b3..2ab372d82b72 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbutil.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py
@@ -953,7 +953,8 @@ def run_to_source_breakpoint(test, bkpt_pattern, source_spec,
                              bkpt_module = None,
                              in_cwd = True,
                              only_one_thread = True,
-                             extra_images = None):
+                             extra_images = None,
+                             has_locations_before_run = True):
     """Start up a target, using exe_name as the executable, and run it to
        a breakpoint set by source regex bkpt_pattern.
 
@@ -964,9 +965,10 @@ def run_to_source_breakpoint(test, bkpt_pattern, source_spec,
     # Set the breakpoints
     breakpoint = target.BreakpointCreateBySourceRegex(
             bkpt_pattern, source_spec, bkpt_module)
-    test.assertTrue(breakpoint.GetNumLocations() > 0,
-        'No locations found for source breakpoint: "%s", file: "%s", dir: "%s"'
-        %(bkpt_pattern, source_spec.GetFilename(), source_spec.GetDirectory()))
+    if has_locations_before_run:
+        test.assertTrue(breakpoint.GetNumLocations() > 0,
+                        'No locations found for source breakpoint: "%s", file: "%s", dir: "%s"'
+                        %(bkpt_pattern, source_spec.GetFilename(), source_spec.GetDirectory()))
     return run_to_breakpoint_do_run(test, target, breakpoint, launch_info,
                                     only_one_thread, extra_images)
 

[adrian-prantl]

@swift-ci test