Fix when() clause cutting stack traces when filtering doesn't succeed (fix for 46661...
authormathieubourgeois <mathieu.bourgeois@gameloft.com>
Tue, 14 Feb 2017 02:53:05 +0000 (21:53 -0500)
committerZoltan Varga <vargaz@gmail.com>
Tue, 14 Feb 2017 02:53:05 +0000 (21:53 -0500)
* * [exceptions] When encountering a filter (when) clause, don't setup the stack trace before we know if the filter is going to work. Otherwise, any exception that isn't blocked by the when clause will have its stack trace cut at the point of the parent of the function containing the when clause
* Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=46661

* + Added test case for bug 46661

mono/mini/mini-exceptions.c
mono/tests/Makefile.am
mono/tests/bug-46661.cs [new file with mode: 0644]

index 1813122d609def11d098566043de73514a44e3ae..68427314ac5e3fef0b4526e3ae0f117652d5202d 100644 (file)
@@ -1589,18 +1589,9 @@ mono_handle_exception_internal_first_pass (MonoContext *ctx, MonoObject *obj, gi
                                        ex_obj = obj;
 
                                if (ei->flags == MONO_EXCEPTION_CLAUSE_FILTER) {
-                                       gboolean is_user_frame = method->wrapper_type == MONO_WRAPPER_NONE || method->wrapper_type == MONO_WRAPPER_DYNAMIC_METHOD;
 #ifndef DISABLE_PERFCOUNTERS
                                        mono_perfcounters->exceptions_filters++;
 #endif
-                                       /*
-                                       Here's the thing, if this is a filter clause done by a wrapper like runtime invoke, we don't want to
-                                       trim the stackframe since if it returns FALSE we lose information.
-
-                                       FIXME Not 100% sure if it's a good idea even with user clauses.
-                                       */
-                                       if (is_user_frame)
-                                               setup_stack_trace (mono_ex, dynamic_methods, &trace_ips);
 
 #ifndef MONO_CROSS_COMPILE
 #ifdef MONO_CONTEXT_SET_LLVM_EXC_REG
@@ -1637,8 +1628,7 @@ mono_handle_exception_internal_first_pass (MonoContext *ctx, MonoObject *obj, gi
                                        filter_idx ++;
 
                                        if (filtered) {
-                                               if (!is_user_frame)
-                                                       setup_stack_trace (mono_ex, dynamic_methods, &trace_ips);
+                                               setup_stack_trace (mono_ex, dynamic_methods, &trace_ips);
                                                g_slist_free (dynamic_methods);
                                                /* mono_debugger_agent_handle_exception () needs this */
                                                mini_set_abort_threshold (ctx);
index 98a48d654278c21915c0ee5f37aed1339660d651..58ca018ac9b8d426153dd6b2067a9c288f337cf5 100644 (file)
@@ -478,7 +478,8 @@ BASE_TEST_CS_SRC=           \
        namedmutex-destroy-race.cs      \
        thread6.cs      \
        appdomain-threadpool-unload.cs  \
-       process-unref-race.cs
+       process-unref-race.cs   \
+       bug-46661.cs
 
 TEST_CS_SRC_DIST=      \
        $(BASE_TEST_CS_SRC)     \
diff --git a/mono/tests/bug-46661.cs b/mono/tests/bug-46661.cs
new file mode 100644 (file)
index 0000000..fe88b57
--- /dev/null
@@ -0,0 +1,51 @@
+using System;
+using System.Linq;
+using System.IO;
+
+namespace ExceptionFilterTestLauncher
+{
+    class Program
+    {
+        public static object newObject = null;
+        public static bool SubTest(int i)
+        {
+            return newObject.GetHashCode() == i;
+        }
+
+        public static bool HandleException(Exception e)
+        {
+            return true;
+        }
+
+        public static bool Test2(int i)
+        {
+            bool test = true;
+            try
+            {
+                test = SubTest(i);
+            }
+            catch (Exception e) when (!HandleException(e))
+            {
+            }
+            return test;
+        }
+
+        static void Main(string[] args)
+        {
+            try
+            {
+                bool result = Test2(12345);
+            }
+            catch (Exception e)
+            {
+                // Before bug 46661 was fixed, the when would cut the stack trace, so Test(int) wouldn't show up
+                if(!e.StackTrace.Contains("SubTest"))
+                    throw new Exception("Stack trace doesn't reference SubTest function. Current stacktrace is " + e.StackTrace.ToString());
+                else
+                    // Correct result
+                    Environment.Exit(0);
+            }
+            throw new Exception("Exception should have been caught!");
+        }
+    }
+}
\ No newline at end of file