Bug 10782 - Step out and step in steps over outside call
Summary: Step out and step in steps over outside call
Status: RESOLVED FIXED
Alias: None
Product: Runtime
Classification: Mono
Component: Debugger ()
Version: unspecified
Hardware: PC Windows
: High normal
Target Milestone: ---
Assignee: Zoltan Varga
URL:
Depends on:
Blocks:
 
Reported: 2013-02-28 17:23 UTC by Eric Maupin
Modified: 2014-05-11 17:17 UTC (History)
7 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:


Attachments
Debugger Test App (4.30 KB, application/zip)
2013-05-29 12:04 UTC, dean.ellis
Details
Screencast of the issue. (418.69 KB, application/x-shockwave-flash)
2013-09-11 19:25 UTC, Eric Maupin
Details


Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on GitHub or Developer Community with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description Eric Maupin 2013-02-28 17:23:04 UTC
Given:

using Android.App;
using Android.OS;
namespace AndroidApplication1 {
	[Activity(Label = "AndroidApplication1", MainLauncher = true, Icon = "@drawable/icon")]
	public class Activity1 : Activity {
		protected override void OnCreate(Bundle bundle) {
			base.OnCreate (bundle);
			SetContentView (Resource.Layout.Main);

			DoStuff (new Test()); // Set breakpoint here
		}

		private void DoStuff (Test asdf)
		{
			string bar = asdf.ToString();
		}

		class Test {
			public Test() {
				string foo = "bar";
			}
		}
	}
}

Reproduce steps:

1. Step in
2. Step out
3. Step in

Expected: Stepped into DoStuff.
Actual: Stepped over DoStuff.

Tested in VS2012
Comment 1 dean.ellis 2013-05-29 09:52:15 UTC
I've tested this on mono-android-4.7.05205. 

I get exactly the same result in both VS and XS.

The result is as follows

1) Debugger breaks on the DoStuff( new Test()); call
2) Click Step in, debugger steps to the public Test() line
3) Click Step out, debugger does to the } after the DoStuff(new Test()); line.

However after doing something similar with a simple Console App in Visual Studio I get similar results. The only difference is the Step out click results in the DoStuff(new Test()); line being hightlighted again.

This is the C# code for the console app

class Program {
		static void Main(string[] args)
		{
			new TestMain().Init();;
		}
	}

	public class TestMain {

		public void Init()
		{
			DoStuff(new Test());
		}

		private void DoStuff(Test asdf)
		{
			string bar = asdf.ToString();
		}

		class Test {
			public Test()
			{
				string foo = "bar";
			}
		}
	}
Comment 2 Eric Maupin 2013-05-29 11:25:38 UTC
Not sure how you got these same results for a console app, but this is not the .NET/VS behavior. Comparison video: http://screencast.com/t/OYaRF53L
Comment 3 dean.ellis 2013-05-29 12:03:14 UTC
I have managed to replicate this in both VS and XS in Android. Also I have replicated it for a Console App in XS on a Mac (see attachment)
Comment 4 dean.ellis 2013-05-29 12:04:13 UTC
Created attachment 4039 [details]
Debugger Test App
Comment 5 Zoltan Varga 2013-05-30 03:45:30 UTC
mcs generates the following IL code:

    IL_005b:  newobj     instance void test.Test::.ctor()
    IL_0060:  call       instance void test.AppDelegate::DoStuff(class test.Test)

There is no 'nop' instruction between the two calls, and the line number info does not contain an entry
for the inner call either, so the debugger doesn't know that it needs to insert a sequence point between the two calls.
Comment 6 Eric Maupin 2013-06-14 19:03:48 UTC
(In reply to comment #5)
> mcs generates the following IL code:
> 
>     IL_005b:  newobj     instance void test.Test::.ctor()
>     IL_0060:  call       instance void test.AppDelegate::DoStuff(class
> test.Test)
> 
> There is no 'nop' instruction between the two calls, and the line number info
> does not contain an entry
> for the inner call either, so the debugger doesn't know that it needs to insert
> a sequence point between the two calls.

So is this a compiler bug then?
Comment 7 Zoltan Varga 2013-06-14 19:06:03 UTC
I think so.
Comment 8 Eric Maupin 2013-06-14 19:07:24 UTC
(In reply to comment #7)
> I think so.

In the future, if that's the case, can you reassign the bug to the appropriate people so it continues to get followed up on? Thanks.
Comment 9 Rodrigo Kumpera 2013-06-25 14:30:13 UTC
Mareks, any progress on this?
Comment 10 Marek Safar 2013-06-26 11:11:13 UTC
I don't think it's compiler issue (at least not until we decide how to implement stepping into).

Little background first. C# compiler generated sequence points for statements. That means it generated sequence points for stepping over debugging. 

E.g.

var a = Foo () + Bar ();

is 1 sequence point in mdb (same with csc for pdb) so debugger knows what IL to jump when user steps over.

Same sequence points CANNOT be used for stepping into because that would break stepping over. .NET does thing using some heuristics inside JIT which is probably preferable because generating another kind of sequence points for every call expression would inflate mdb quite a lot.
Comment 11 Marek Safar 2013-06-26 11:53:10 UTC
Benefits of implementing this using "another" kind of sequence points inside mdb is that we could do something not possible with VS debugger where you cannot set a breakpoint at specific call argument. We would beed to figure out good UI for it too.

E.g.

Foo (First (), value, Another ());

It's not possible to set a breakpoint at Another call in VS.
Comment 12 Rodrigo Kumpera 2013-06-26 15:29:46 UTC
Mareks,

What places would get those extra sequence points?

We can prototype that into a branch if you think it's a good way to move forward.
Comment 13 Marek Safar 2013-07-05 08:24:47 UTC
Fixed in master
Comment 14 Eric Maupin 2013-09-11 19:24:57 UTC
Tested in both Xamarin.Android 4.8.03061 and an equivalent test on Xamarin.iOS 1.5.61, the given test does not produce the expected result.

Now, instead of returning to the DoStuff() line, the next step is on the bracket below it and stepping in simply continues to run the method.
Comment 15 Eric Maupin 2013-09-11 19:25:38 UTC
Created attachment 4848 [details]
Screencast of the issue.
Comment 16 PJ 2013-09-13 16:09:59 UTC
Ermau has identified this bug as 'Debilitating' and it comes with a repro. => high
Comment 17 Eric Maupin 2013-09-13 16:12:47 UTC
Remember guys, VS does not use our compiler, any solution just for that is not a fix.
Comment 18 Zoltan Varga 2013-09-14 16:56:52 UTC
It looks like VS inserts an implicit sequence point between the two calls, and step over doesn't stop at this sequence point.  Since this happens for csc generated code we have no control over, this means we have to solve this in the JIT. The problem with this is that it is pretty hard to detect nested calls inside the JIT, since we only make a single pass over the IL, and have no access to a cfg like mcs does.
Comment 19 Zoltan Varga 2013-09-14 18:20:13 UTC
It turns out this is easy to implement, MS simply puts a seq point after every call, even non nested calls,
so in this statement:
int i = foo ();
you have to do an extra step in after foo () returned to execute the assignment.
So we basically have to treat seq points at non-empty IL offsets specially during step over.
Comment 20 Zoltan Varga 2013-09-14 19:37:40 UTC
Should be fixed in 488888ce612491aca1c97f79ddbad685302d43cc.

We now generate a seq point after every nonvoid call, and step over skips over these.
Comment 21 Zoltan Varga 2014-05-11 17:17:36 UTC
-> FIXED.