[runtime] Remove eager exception stack construction when it contains dynamic method. Fixes #33064

We used to construct exception stack traces as string at throw site (for dynamic methods) but
that does not work correctly because exception stack trace can be decomposed via
new StackTrace (exception) or merged via ExceptionDispatchInfo both of them need the exception
stack in raw form and not as a string.

Further the format used by local stack builder was not same as format used by
System.Diagnostics.StackTrace.

The orignal reason for the specialization was that dynamic method can be collected
before stack is rendered which leads to stack frame to be reported as

at <unknown method> instead of
at (wrapper dynamic-method)

That should be addressed by gc-link between dynamic method and exception or
stack-frame.

external/referencesource

@@ -1 +1 @@
-Subproject commit 3976a6088118b35ed318f8e78767066cb78ffdf9
+Subproject commit a09accbcb9771db77a3b0370c954f643d59f4ce7

mcs/class/corlib/System.Diagnostics/StackTrace.cs

@@ -120,11 +120,6 @@ namespace System.Diagnostics {
 		}
 
 		public StackTrace (Exception e, int skipFrames, bool fNeedFileInfo)
-			: this (e, skipFrames, fNeedFileInfo, false)
-		{
-		}
-
-		internal StackTrace (Exception e, int skipFrames, bool fNeedFileInfo, bool returnNativeFrames)
 		{
 			if (e == null)
 				throw new ArgumentNullException ("e");
@@ -133,23 +128,6 @@ namespace System.Diagnostics {
 
 			frames = get_trace (e, skipFrames, fNeedFileInfo);
 
-			if (!returnNativeFrames) {
-				bool resize = false;
-				for (int i = 0; i < frames.Length; ++i)
-					if (frames [i].GetMethod () == null)
-						resize = true;
-
-				if (resize) {
-					var l = new List<StackFrame> ();
-
-					for (int i = 0; i < frames.Length; ++i)
-						if (frames [i].GetMethod () != null)
-							l.Add (frames [i]);
-
-					frames = l.ToArray ();
-				}
-			}
-
 			captured_traces = e.captured_traces;
 		}
 

mcs/class/corlib/System/Exception.cs

@@ -199,7 +199,7 @@ namespace System
 					/* Not thrown yet */
 					return null;
 
-				StackTrace st = new StackTrace (this, 0, true, true);
+				StackTrace st = new StackTrace (this, 0, true);
 				return stack_trace = st.ToString ();
 			}
 		}
@@ -298,6 +298,7 @@ namespace System
 		{
 			captured_traces = (StackTrace[]) exceptionDispatchInfo.BinaryStackTraceArray;
 			trace_ips = null;
+			stack_trace = null;
 		}
 
 		//

mcs/class/corlib/Test/System.Reflection.Emit/DynamicMethodTest.cs

@@ -12,6 +12,8 @@ using System.Reflection;
 using System.Reflection.Emit;
 using System.Runtime.InteropServices;
 using System.Text;
+using System.Diagnostics;
+using System.Runtime.ExceptionServices;
 
 using NUnit.Framework;
 
@@ -477,6 +479,104 @@ namespace MonoTests.System.Reflection.Emit
 			public string Name;
 		}
 
+		class ExceptionHandling_Test_Support
+		{
+			public static Exception Caught;
+			public static string CaughtStackTrace;
+
+			public static void ThrowMe ()
+			{
+				Caught = null;
+				CaughtStackTrace = null;
+				throw new Exception("test");
+			}
+
+			public static void Handler (Exception e)
+			{
+				Caught = e;
+				CaughtStackTrace = e.StackTrace.ToString ();
+			}
+		}
+
+		[Test]
+		public void ExceptionHandling ()
+		{
+			var method = new DynamicMethod ("", typeof(void), new[] { typeof(int) }, typeof (DynamicMethodTest));
+			var ig = method.GetILGenerator ();
+
+			ig.BeginExceptionBlock();
+			ig.Emit(OpCodes.Call, typeof(ExceptionHandling_Test_Support).GetMethod("ThrowMe"));
+
+			ig.BeginCatchBlock(typeof(Exception));
+			ig.Emit(OpCodes.Call, typeof(ExceptionHandling_Test_Support).GetMethod("Handler"));
+			ig.EndExceptionBlock();
+
+			ig.Emit(OpCodes.Ret);
+
+			var invoke = (Action<int>) method.CreateDelegate (typeof(Action<int>));
+			invoke (456324);
+
+			Assert.IsNotNull (ExceptionHandling_Test_Support.Caught, "#1");
+			Assert.AreEqual (2, ExceptionHandling_Test_Support.CaughtStackTrace.Split (new[] { Environment.NewLine }, StringSplitOptions.None).Length, "#2");
+
+			var st = new StackTrace (ExceptionHandling_Test_Support.Caught, 0, true);
+
+			// Caught stack trace when dynamic method is gone
+			Assert.AreEqual (ExceptionHandling_Test_Support.CaughtStackTrace, st.ToString (), "#3");
+
+			// Catch handler stack trace inside dynamic method match
+			Assert.AreEqual (ExceptionHandling_Test_Support.Caught.StackTrace, st.ToString (), "#4");
+		}
+
+		class ExceptionHandlingWithExceptionDispatchInfo_Test_Support
+		{
+			public static Exception Caught;
+			public static string CaughtStackTrace;
+
+			public static void ThrowMe ()
+			{
+				Caught = null;
+				CaughtStackTrace = null;
+
+				Exception e;
+				try {
+					throw new Exception("test");
+				} catch (Exception e2) {
+					e = e2;
+				}
+
+				var edi = ExceptionDispatchInfo.Capture(e);
+
+				edi.Throw();
+			}
+
+			public static void Handler (Exception e)
+			{
+				var split = e.StackTrace.Split (new[] { Environment.NewLine }, StringSplitOptions.RemoveEmptyEntries);
+				Assert.AreEqual (5, split.Length, "#1");
+				Assert.IsTrue (split [1].Contains ("---"), "#2");
+			}
+		}
+
+		[Test]
+		public void ExceptionHandlingWithExceptionDispatchInfo ()
+		{
+			var method = new DynamicMethod ("", typeof(void), new[] { typeof(int) }, typeof (DynamicMethodTest));
+			var ig = method.GetILGenerator ();
+
+			ig.BeginExceptionBlock();
+			ig.Emit(OpCodes.Call, typeof(ExceptionHandlingWithExceptionDispatchInfo_Test_Support).GetMethod("ThrowMe"));
+
+			ig.BeginCatchBlock(typeof(Exception));
+			ig.Emit(OpCodes.Call, typeof(ExceptionHandlingWithExceptionDispatchInfo_Test_Support).GetMethod("Handler"));
+			ig.EndExceptionBlock();
+
+			ig.Emit(OpCodes.Ret);
+
+			var invoke = (Action<int>) method.CreateDelegate (typeof(Action<int>));
+			invoke (444);
+		}
+
 #if !MONODROID
 		// RUNTIME: crash
 		[Test]

mono/mini/mini-exceptions.c

@@ -620,50 +620,6 @@ mono_exception_walk_trace (MonoException *ex, MonoExceptionFrameWalk func, gpoin
 	return len > 0;
 }
 
-MonoString *
-ves_icall_System_Exception_get_trace (MonoException *ex)
-{
-	MonoDomain *domain = mono_domain_get ();
-	MonoString *res;
-	MonoArray *ta = ex->trace_ips;
-	int i, len;
-	GString *trace_str;
-
-	if (ta == NULL)
-		/* Exception is not thrown yet */
-		return NULL;
-
-	len = mono_array_length (ta) >> 1;
-	trace_str = g_string_new ("");
-	for (i = 0; i < len; i++) {
-		MonoJitInfo *ji;
-		gpointer ip = mono_array_get (ta, gpointer, i * 2 + 0);
-		gpointer generic_info = mono_array_get (ta, gpointer, i * 2 + 1);
-
-		ji = mono_jit_info_table_find_internal (domain, ip, TRUE, TRUE);
-		if (ji == NULL) {
-			/* Unmanaged frame */
-			g_string_append_printf (trace_str, "in (unmanaged) %p\n", ip);
-		} else if (!ji->is_trampoline) {
-			gchar *location;
-			gint32 address;
-			MonoMethod *method = get_method_from_stack_frame (ji, generic_info);
-
-			address = (char *)ip - (char *)ji->code_start;
-			location = mono_debug_print_stack_frame (
-				method, address, ex->object.vtable->domain);
-
-			g_string_append_printf (trace_str, "%s\n", location);
-			g_free (location);
-		}
-	}
-
-	res = mono_string_new (ex->object.vtable->domain, trace_str->str);
-	g_string_free (trace_str, TRUE);
-
-	return res;
-}
-
 MonoArray *
 ves_icall_get_trace (MonoException *exc, gint32 skip, MonoBoolean need_file_info)
 {
@@ -1199,9 +1155,6 @@ build_native_trace (void)
 		trace_ips = g_list_reverse (trace_ips);	\
 		MONO_OBJECT_SETREF (mono_ex, trace_ips, glist_to_array (trace_ips, mono_defaults.int_class));	\
 		MONO_OBJECT_SETREF (mono_ex, native_trace_ips, build_native_trace ());	\
-		if (has_dynamic_methods)	\
-			/* These methods could go away anytime, so compute the stack trace now */	\
-			MONO_OBJECT_SETREF (mono_ex, stack_trace, ves_icall_System_Exception_get_trace (mono_ex));	\
 	}	\
 	g_list_free (trace_ips);	\
 	trace_ips = NULL;	\

mono/mini/mini-runtime.c

@@ -3133,8 +3133,6 @@ register_icalls (void)
 				ves_icall_get_frame_info);
 	mono_add_internal_call ("System.Diagnostics.StackTrace::get_trace",
 				ves_icall_get_trace);
-	mono_add_internal_call ("System.Exception::get_trace",
-				ves_icall_System_Exception_get_trace);
 	mono_add_internal_call ("Mono.Runtime::mono_runtime_install_handlers",
 				mono_runtime_install_handlers);
 

mono/mini/mini.h

@@ -2675,7 +2675,6 @@ MonoBoolean ves_icall_get_frame_info            (gint32 skip, MonoBoolean need_f
 						 MonoReflectionMethod **method, 
 						 gint32 *iloffset, gint32 *native_offset,
 						 MonoString **file, gint32 *line, gint32 *column);
-MonoString *ves_icall_System_Exception_get_trace (MonoException *exc);
 void mono_set_cast_details                      (MonoClass *from, MonoClass *to);
 
 /* Installs a function which is called when the runtime encounters an unhandled exception.