Merge pull request #3040 from xmcclure/debugger-step-recursive

Debugger stepping does not understand recursion (bug 38025)

2 files changed, 361 insertions, 0 deletions

diff --git a/mcs/class/Mono.Debugger.Soft/Test/dtest-app.cs b/mcs/class/Mono.Debugger.Soft/Test/dtest-app.cs
index 150738e6633..4b9d569b1ad 100644
--- a/mcs/class/Mono.Debugger.Soft/Test/dtest-app.cs
+++ b/mcs/class/Mono.Debugger.Soft/Test/dtest-app.cs
@@ -419,6 +419,9 @@ public class Tests : TestsBase, ITest2
 		ss_step_through ();
 		ss_non_user_code ();
 		ss_recursive (1);
+		ss_recursive2 (1);
+		ss_recursive2 (1);
+		ss_recursive_chaotic ();
 		ss_fp_clobber ();
 	}
 
@@ -568,6 +571,92 @@ public class Tests : TestsBase, ITest2
 		ss_recursive (n + 1);
 	}
 
+	// Breakpoint will be placed here
+	[MethodImplAttribute (MethodImplOptions.NoInlining)]
+	public static void ss_recursive2_trap ()
+	{
+	}
+
+	public static void ss_recursive2_at (string s)
+	{
+		// Console.WriteLine (s);
+	}
+
+	// This method is used both for a step over and step out test.
+	[MethodImplAttribute (MethodImplOptions.NoInlining)]
+	public static void ss_recursive2 (int x)
+	{
+		ss_recursive2_at ( "ss_recursive2 in " + x);
+		if (x < 5) {
+			int next = x + 1;
+			ss_recursive2_at ("ss_recursive2 descend " + x);
+			ss_recursive2_trap ();
+			ss_recursive2 (next);
+		}
+		ss_recursive2_at ("ss_recursive2 out " + x);
+	}
+
+	// Breakpoint will be placed here
+	[MethodImplAttribute (MethodImplOptions.NoInlining)]
+	public static void ss_recursive_chaotic_trap ()
+	{
+	}
+
+	[MethodImplAttribute (MethodImplOptions.NoInlining)]
+	public static void ss_recursive_chaotic_at (bool exiting, string at, int n)
+	{
+//		string indent = "";
+//		for (int count = 5 - n; count > 0; count--)
+//			indent += "\t";
+//		Console.WriteLine (indent + (exiting ? "<--" : "-->") + " " + at + " " + n);
+	}
+
+	[MethodImplAttribute (MethodImplOptions.NoInlining)]
+	public static void ss_recursive_chaotic_fizz (int n)
+	{
+		ss_recursive_chaotic_at (false, "fizz", n);
+		if (n > 0) {
+			int next = n - 1;
+			ss_recursive_chaotic_buzz (next);
+			ss_recursive_chaotic_fizzbuzz (next);
+		} else {
+			ss_recursive_chaotic_trap ();
+		}
+		ss_recursive_chaotic_at (true, "fizz", n);
+	}
+
+	[MethodImplAttribute (MethodImplOptions.NoInlining)]
+	public static void ss_recursive_chaotic_buzz (int n)
+	{
+		ss_recursive_chaotic_at (false, "buzz", n);
+		if (n > 0) {
+			int next = n - 1;
+			ss_recursive_chaotic_fizz (next);
+			ss_recursive_chaotic_fizzbuzz (next);
+		}
+		ss_recursive_chaotic_at (true, "buzz", n);
+	}
+
+	[MethodImplAttribute (MethodImplOptions.NoInlining)]
+	public static void ss_recursive_chaotic_fizzbuzz (int n)
+	{
+		ss_recursive_chaotic_at (false, "fizzbuzz", n);
+		if (n > 0) {
+			int next = n - 1;
+			ss_recursive_chaotic_fizz (next);
+			ss_recursive_chaotic_buzz (next);
+			ss_recursive_chaotic_fizzbuzz (next);
+		}
+		ss_recursive_chaotic_at (true, "fizzbuzz", n);
+	}
+
+	// Call a complex tree of recursive calls that has tripped up "step out" in the past.
+	[MethodImplAttribute (MethodImplOptions.NoInlining)]
+	public static void ss_recursive_chaotic ()
+	{
+		ss_recursive_chaotic_fizz (5);
+	}
+
 	[MethodImplAttribute (MethodImplOptions.NoInlining)]
 	public static void ss_fp_clobber () {
 		double v = ss_fp_clobber_1 (5.0);
diff --git a/mcs/class/Mono.Debugger.Soft/Test/dtest.cs b/mcs/class/Mono.Debugger.Soft/Test/dtest.cs
index cd0b6ae51ba..c0af12cb38a 100644
--- a/mcs/class/Mono.Debugger.Soft/Test/dtest.cs
+++ b/mcs/class/Mono.Debugger.Soft/Test/dtest.cs
@@ -42,6 +42,17 @@ public class DebuggerTests
 	public static string runtime = Environment.GetEnvironmentVariable ("DBG_RUNTIME");
 	public static string agent_args = Environment.GetEnvironmentVariable ("DBG_AGENT_ARGS");
 
+	// Not currently used, but can be useful when debugging individual tests.
+	void StackTraceDump (Event e)
+	{
+		int i = 0;
+		foreach (var frame in e.Thread.GetFrames ())
+		{
+			i++;
+			Console.WriteLine ("Frame " + i + ", " + frame.Method.Name);
+		}
+	}
+
 	Event GetNextEvent () {
 		var es = vm.GetNextEventSet ();
 		Assert.AreEqual (1, es.Events.Length);
@@ -124,6 +135,140 @@ public class DebuggerTests
 		return (e as BreakpointEvent);
 	}
 
+	class ReusableBreakpoint {
+		DebuggerTests owner;
+		public string method_name;
+		public BreakpointEventRequest req;
+		public BreakpointEvent lastEvent = null;
+		public ReusableBreakpoint (DebuggerTests owner, string method_name)
+		{
+			this.owner = owner;
+			this.method_name = method_name;
+			MethodMirror m = owner.entry_point.DeclaringType.GetMethod (method_name);
+			Assert.IsNotNull (m);
+			req = owner.vm.SetBreakpoint (m, m.ILOffsets [0]);
+		}
+
+		public void Continue ()
+		{
+			bool survived = false;
+
+			try {
+				Event e = null;
+
+				while (true) {
+					owner.vm.Resume ();
+					e = owner.GetNextEvent ();
+					if (e is BreakpointEvent)
+						break;
+				}
+
+				Assert.IsInstanceOfType (typeof (BreakpointEvent), e);
+				Assert.AreEqual (method_name, (e as BreakpointEvent).Method.Name);
+
+				lastEvent = e as BreakpointEvent;
+
+				survived = true;
+			} finally {
+				if (!survived) { // Ensure cleanup if we triggered an assert
+					Disable ();
+				}
+			}
+		}
+
+		public void Disable ()
+		{
+			req.Disable ();
+		}
+	}
+
+	/* One of the tests executes a complex tree of recursive functions.
+	   The only good way to specify how its behavior should appear from this side
+	   is to just run the function tree once over here and record what it does. */
+	public struct RecursiveChaoticPoint
+	{
+		public bool breakpoint;
+		public string name;
+		public int depth;
+
+		public RecursiveChaoticPoint (bool breakpoint, string name, int depth)
+		{
+			this.breakpoint = breakpoint;
+			this.name = name;
+			this.depth = depth;
+		}
+	}
+
+	// The breakpoint is placed here in dtest-app.cs
+	public static void ss_recursive_chaotic_trap (int n, List<RecursiveChaoticPoint> trace, ref bool didLast, ref bool didAny)
+	{
+		// Depth is calculated as:
+		// Main + single_stepping + ss_recursive_chaotic + (n is 5 at outermost frame and 0 at innermost frame) + ss_recursive_chaotic_trap
+		trace.Add (new RecursiveChaoticPoint (true, "ss_recursive_chaotic_trap", 5 - n + 5));
+		didLast = true;
+	}
+
+	public static void ss_recursive_chaotic_at (string at, int n, List<RecursiveChaoticPoint> trace, ref bool didLast, ref bool didAny)
+	{
+		// This will be called after every return from a function. The other function will return whether "step out" is currently active, and it will be passed in here as didLast.
+		if (didLast) {
+			// Depth is calculated as:
+			// Main + single_stepping + ss_recursive_chaotic + (n is 5 at outermost frame and 0 at innermost frame)
+			trace.Add (new RecursiveChaoticPoint (false, "ss_recursive_chaotic_" + at, 5 - n + 4));
+			didAny = true;
+			didLast = false;
+		}
+	}
+
+	public static bool ss_recursive_chaotic_fizz (int n, List<RecursiveChaoticPoint> trace)
+	{
+		bool didLast = false, didAny = false;
+		if (n > 0) {
+			int next = n - 1;
+			didLast = ss_recursive_chaotic_buzz (next, trace);
+			ss_recursive_chaotic_at ("fizz", n, trace, ref didLast, ref didAny);
+			didLast = ss_recursive_chaotic_fizzbuzz (next, trace);
+			ss_recursive_chaotic_at ("fizz", n, trace, ref didLast, ref didAny);
+		} else {
+			ss_recursive_chaotic_trap (n, trace, ref didLast, ref didAny);
+			ss_recursive_chaotic_at ("fizz", n, trace, ref didLast, ref didAny);
+		}
+		return didAny;
+	}
+
+	public static bool ss_recursive_chaotic_buzz (int n, List<RecursiveChaoticPoint> trace)
+	{
+		bool didLast = false, didAny = false;
+		if (n > 0) {
+			int next = n - 1;
+			didLast = ss_recursive_chaotic_fizz (next, trace);
+			ss_recursive_chaotic_at ("buzz", n, trace, ref didLast, ref didAny);
+			didLast = ss_recursive_chaotic_fizzbuzz (next, trace);
+			ss_recursive_chaotic_at ("buzz", n, trace, ref didLast, ref didAny);
+		}
+		return didAny;
+	}
+
+	public static bool ss_recursive_chaotic_fizzbuzz (int n, List<RecursiveChaoticPoint> trace)
+	{
+		bool didLast = false, didAny = false;
+		if (n > 0) {
+			int next = n - 1;
+			didLast = ss_recursive_chaotic_fizz (next, trace);
+			ss_recursive_chaotic_at ("fizzbuzz", n, trace, ref didLast, ref didAny);
+			didLast = ss_recursive_chaotic_buzz (next, trace);
+			ss_recursive_chaotic_at ("fizzbuzz", n, trace, ref didLast, ref didAny);
+			didLast = ss_recursive_chaotic_fizzbuzz (next, trace);
+			ss_recursive_chaotic_at ("fizzbuzz", n, trace, ref didLast, ref didAny);
+		}
+		return didAny;
+	}
+
+	public static void trace_ss_recursive_chaotic (List<RecursiveChaoticPoint> trace)
+	{
+		ss_recursive_chaotic_fizz (5, trace);
+	}
+
 	Event single_step (ThreadMirror t) {
 		var req = vm.CreateStepRequest (t);
 		req.Enable ();
@@ -372,11 +517,28 @@ public class DebuggerTests
 		Assert.AreEqual (m2.Name, (e as BreakpointEvent).Method.Name);
 	}
 
+	// Assert we have stepped to a location
 	void assert_location (Event e, string method) {
 		Assert.IsTrue (e is StepEvent);
 		Assert.AreEqual (method, (e as StepEvent).Method.Name);
 	}
 
+	// Assert we have breakpointed at a location
+	void assert_location_at_breakpoint (Event e, string method) {
+		Assert.IsTrue (e is BreakpointEvent);
+		Assert.AreEqual (method, (e as BreakpointEvent).Method.Name);
+	}
+
+	// Assert we have stepped to or breakpointed at a location
+	void assert_location_allow_breakpoint (Event e, string method) {
+		if (e is StepEvent)
+			Assert.AreEqual (method, (e as StepEvent).Method.Name);
+		else if (e is BreakpointEvent)
+			Assert.AreEqual (method, (e as BreakpointEvent).Method.Name);
+		else
+			Assert.Fail ("Neither step nor breakpoint event");
+	}
+
 	StepEventRequest create_step (Event e) {
 		var req = vm.CreateStepRequest (e.Thread);
 		step_req = req;
@@ -624,6 +786,95 @@ public class DebuggerTests
 		AssertValue (1, f.GetValue (f.Method.GetLocal ("n")));
 		req.Disable ();
 
+		// Check that step-over stops correctly when inner frames with recursive functions contain breakpoints
+		e = run_until ("ss_recursive2");
+		ReusableBreakpoint breakpoint = new ReusableBreakpoint (this, "ss_recursive2_trap");
+		try {
+			breakpoint.Continue ();
+			e = breakpoint.lastEvent;
+			req = create_step (e);
+			for (int c = 1; c <= 4; c++) {
+				// The first five times we try to step over this function, the breakpoint will stop us
+				assert_location_at_breakpoint (e, "ss_recursive2_trap");
+
+				req.Disable ();
+				req = create_step (e);
+				req.Size = StepSize.Line;
+
+				e = step_out ();
+				assert_location (e, "ss_recursive2");
+
+				// Stack should consist of Main + single_stepping + (1 ss_recursive2 frame per loop iteration)
+				Assert.AreEqual (c+2, e.Thread.GetFrames ().Length);
+				e = step_over_or_breakpoint ();
+			}
+			// At this point we should have escaped the breakpoints and this will be a normal step stop
+			assert_location (e, "ss_recursive2");
+			Assert.AreEqual (6, e.Thread.GetFrames ().Length);
+		} finally {
+			req.Disable ();
+			breakpoint.Disable ();
+		}
+
+		// Check that step-out stops correctly when inner frames with recursive functions contain breakpoints
+		e = run_until ("ss_recursive2");
+		breakpoint = new ReusableBreakpoint (this, "ss_recursive2_trap");
+		try {
+			breakpoint.Continue ();
+			e = breakpoint.lastEvent;
+			req = create_step (e);
+			for (int c = 1; c <= 4; c++) {
+				// The first five times we try to step over this function, the breakpoint will stop us
+				assert_location_at_breakpoint (e, "ss_recursive2_trap");
+
+				req.Disable ();
+				req = create_step (e);
+				req.Size = StepSize.Line;
+
+				e = step_out ();
+				assert_location (e, "ss_recursive2");
+
+				// Stack should consist of Main + single_stepping + (1 ss_recursive2 frame per loop iteration)
+				Assert.AreEqual (c+2, e.Thread.GetFrames ().Length);
+				e = step_out_or_breakpoint ();
+			}
+			for (int c = 3; c >= 1; c--) {
+				assert_location (e, "ss_recursive2");
+				Assert.AreEqual (c + 2, e.Thread.GetFrames ().Length);
+
+				e = step_out ();
+			}
+		} finally {
+			req.Disable ();
+			breakpoint.Disable ();
+		}
+
+		// Test step out with a really complicated call tree
+		List<RecursiveChaoticPoint> trace = new List<RecursiveChaoticPoint>();
+		trace_ss_recursive_chaotic (trace);
+		e = run_until ("ss_recursive_chaotic");
+		try {
+			breakpoint = new ReusableBreakpoint (this, "ss_recursive_chaotic_trap");
+			breakpoint.Continue ();
+			e = breakpoint.lastEvent;
+			foreach (RecursiveChaoticPoint point in trace)
+			{
+				if (point.breakpoint)
+					assert_location_at_breakpoint (e, point.name);
+				else
+					assert_location (e, point.name);
+				Assert.AreEqual (point.depth, e.Thread.GetFrames ().Length);
+
+				req.Disable ();
+				req = create_step (e);
+				req.Size = StepSize.Line;
+				e = step_out_or_breakpoint ();
+			}
+		} finally {
+			req.Disable ();
+			breakpoint.Disable ();
+		}
+
 		// Check that single stepping doesn't clobber fp values
 		e = run_until ("ss_fp_clobber");
 		req = create_step (e);
@@ -1625,6 +1876,27 @@ public class DebuggerTests
 		return step_once ();
 	}
 
+	Event step_once_or_breakpoint () {
+		vm.Resume ();
+		var e = GetNextEvent ();
+		Assert.IsTrue (e is StepEvent || e is BreakpointEvent);
+		return e;
+	}
+
+	Event step_over_or_breakpoint () {
+		step_req.Disable ();
+		step_req.Depth = StepDepth.Over;
+		step_req.Enable ();
+		return step_once_or_breakpoint ();
+	}
+
+	Event step_out_or_breakpoint () {
+		step_req.Disable ();
+		step_req.Depth = StepDepth.Out;
+		step_req.Enable ();
+		return step_once_or_breakpoint ();
+	}
+
 	[Test]
 	public void Locals () {
 		var be = run_until ("locals1");