Skip to content

Commit

Permalink
[runtime] Setup stack trace for use in exception filters too
Browse files Browse the repository at this point in the history
If we have
	`catch (Exception e) when (SomeMethod (e))`

.. then the exception object received in `SomeMethod` has an empty
trace. This is because in `handle_exception_first_pass` we populate the
trace in `MonoException` object only at the "end":

	- filter returns TRUE
	- exception caught
	- unhandled exception

But if we have a filter then the trace at that point needs to be
accessible. And if the filter fails then as we unwind more frames, the
earlier frames (`trace_ips`) need to be retained to get the correct full
trace.

`dynamic_methods` needs to be handled similarly and in
`setup_stack_trace` we need to add to the existing `dynamic_methods` in
the MonoException object.

Fixes #7649 .
  • Loading branch information
radical committed Apr 13, 2018
1 parent ec0406b commit 544209a
Show file tree
Hide file tree
Showing 3 changed files with 139 additions and 14 deletions.
34 changes: 20 additions & 14 deletions mono/mini/mini-exceptions.c
Expand Up @@ -1563,23 +1563,25 @@ build_native_trace (MonoError *error)
#endif
}

/* This can be called more than once on a MonoException. */
static void
setup_stack_trace (MonoException *mono_ex, GSList *dynamic_methods, GList **trace_ips)
setup_stack_trace (MonoException *mono_ex, GSList **dynamic_methods, GList *trace_ips)
{
if (mono_ex) {
*trace_ips = g_list_reverse (*trace_ips);
GList *trace_ips_copy = g_list_copy (trace_ips);
trace_ips_copy = g_list_reverse (trace_ips_copy);
ERROR_DECL (error);
MonoArray *ips_arr = mono_glist_to_array (*trace_ips, mono_defaults.int_class, error);
MonoArray *ips_arr = mono_glist_to_array (trace_ips_copy, mono_defaults.int_class, error);
mono_error_assert_ok (error);
MONO_OBJECT_SETREF (mono_ex, trace_ips, ips_arr);
MONO_OBJECT_SETREF (mono_ex, native_trace_ips, build_native_trace (error));
mono_error_assert_ok (error);
if (dynamic_methods) {
if (*dynamic_methods) {
/* These methods could go away anytime, so save a reference to them in the exception object */
GSList *l;
MonoMList *list = NULL;
MonoMList *list = (MonoMList*)mono_ex->dynamic_methods;

for (l = dynamic_methods; l; l = l->next) {
for (l = *dynamic_methods; l; l = l->next) {
guint32 dis_link;
MonoDomain *domain = mono_domain_get ();

Expand All @@ -1598,10 +1600,13 @@ setup_stack_trace (MonoException *mono_ex, GSList *dynamic_methods, GList **trac
}

MONO_OBJECT_SETREF (mono_ex, dynamic_methods, list);

g_slist_free (*dynamic_methods);
*dynamic_methods = NULL;
}

g_list_free (trace_ips_copy);
}
g_list_free (*trace_ips);
*trace_ips = NULL;
}

typedef enum {
Expand Down Expand Up @@ -1698,8 +1703,8 @@ handle_exception_first_pass (MonoContext *ctx, MonoObject *obj, gint32 *out_filt

unwind_res = unwinder_unwind_frame (&unwinder, domain, jit_tls, NULL, ctx, &new_ctx, NULL, &lmf, NULL, &frame);
if (!unwind_res) {
setup_stack_trace (mono_ex, dynamic_methods, &trace_ips);
g_slist_free (dynamic_methods);
setup_stack_trace (mono_ex, &dynamic_methods, trace_ips);
g_list_free (trace_ips);
return result;
}

Expand Down Expand Up @@ -1785,6 +1790,8 @@ handle_exception_first_pass (MonoContext *ctx, MonoObject *obj, gint32 *out_filt
ex_obj = obj;

if (ei->flags == MONO_EXCEPTION_CLAUSE_FILTER) {
setup_stack_trace (mono_ex, &dynamic_methods, trace_ips);

#ifndef DISABLE_PERFCOUNTERS
mono_atomic_inc_i32 (&mono_perfcounters->exceptions_filters);
#endif
Expand Down Expand Up @@ -1837,8 +1844,7 @@ handle_exception_first_pass (MonoContext *ctx, MonoObject *obj, gint32 *out_filt
filter_idx ++;

if (filtered) {
setup_stack_trace (mono_ex, dynamic_methods, &trace_ips);
g_slist_free (dynamic_methods);
g_list_free (trace_ips);
/* mono_debugger_agent_handle_exception () needs this */
mini_set_abort_threshold (&frame);
MONO_CONTEXT_SET_IP (ctx, ei->handler_start);
Expand All @@ -1852,8 +1858,8 @@ handle_exception_first_pass (MonoContext *ctx, MonoObject *obj, gint32 *out_filt
ERROR_DECL_VALUE (isinst_error);
error_init (&isinst_error);
if (ei->flags == MONO_EXCEPTION_CLAUSE_NONE && mono_object_isinst_checked (ex_obj, catch_class, error)) {
setup_stack_trace (mono_ex, dynamic_methods, &trace_ips);
g_slist_free (dynamic_methods);
setup_stack_trace (mono_ex, &dynamic_methods, trace_ips);
g_list_free (trace_ips);

if (out_ji)
*out_ji = ji;
Expand Down
1 change: 1 addition & 0 deletions mono/tests/Makefile.am
Expand Up @@ -306,6 +306,7 @@ TESTS_CS_SRC= \
exception17.cs \
exception18.cs \
exception19.cs \
exception20.cs \
typeload-unaligned.cs \
struct.cs \
valuetype-gettype.cs \
Expand Down
118 changes: 118 additions & 0 deletions mono/tests/exception20.cs
@@ -0,0 +1,118 @@
using System;
using System.Linq;
using System.Runtime.CompilerServices;
using System.IO;

class CustomException : Exception
{
public int Value;
public CustomException (int value) => this.Value = value;
}

class C
{
static CustomException e;

static void Throw ()
{
try {
throw new CustomException(1);
} catch (CustomException ex) {
e = ex;
}
}

static int FrameCount (Exception ex)
{
string fullTrace = ex.StackTrace;
if (fullTrace == null)
throw new Exception ("Empty trace found!");

string[] frames = fullTrace.Split(new string[] { Environment.NewLine }, StringSplitOptions.None);

// Ignore metadata
frames = frames.Where (l => !l.StartsWith ("[")).ToArray ();

return frames.Length;
}

// throws on false anyway
[MethodImpl(MethodImplOptions.NoInlining)]
static bool CheckTrace (Exception ex, int numFramesToExpect, bool whatToReturn = true)
{
int frames = FrameCount (ex);
if (frames != numFramesToExpect) {
throw new Exception ($"Exception carried {frames} frames along with it when it should have reported {numFramesToExpect}. Full trace:\n---\n{ex.StackTrace}\n---");
}

return whatToReturn;
}

[MethodImpl(MethodImplOptions.NoInlining)]
static void ThrowCustomExceptionWithValueButFailsToCatchWithFilter ()
{
try {
ThrowCustomException (1);
} catch (CustomException ce) when (ce.Value == 9999) {
throw new Exception ("This should NEVER be hit");
}
}

[MethodImpl(MethodImplOptions.NoInlining)]
static void ThrowCustomException (int value)
{
throw new CustomException (value);
}

[MethodImpl(MethodImplOptions.NoInlining)]
static void ThrowFileNotFoundException ()
{
throw new FileNotFoundException ();
}

public static void Main ()
{
Throw ();

// Single frame
// Filter returns false
try {
throw e;
} catch (Exception ex) when (CheckTrace (ex, 1, true)) {
CheckTrace (ex, 1);
}

// Throw + filter fails, then filter matches at next level
try {
ThrowCustomExceptionWithValueButFailsToCatchWithFilter ();
} catch (CustomException ex) when (CheckTrace (ex, 3, true)) {
CheckTrace (ex, 3);
}

// Throw + filter fails, then filter matches, exception re-thrown and caught
try {
try {
ThrowCustomExceptionWithValueButFailsToCatchWithFilter ();
} catch (CustomException ex) when (CheckTrace (ex, 3, true)) {
CheckTrace (ex, 3);

// this will truncate the trace now
throw ex;
}
} catch (Exception ex) {
CheckTrace (ex, 1);
}

// Throw, filter matches, throw exception as-is, caught
try {
try {
ThrowFileNotFoundException ();
} catch (Exception e) when (CheckTrace (e, 2)) {
// trace should remain as is
throw;
}
} catch (Exception ex) {
CheckTrace (ex, 2);
}
}
}

0 comments on commit 544209a

Please sign in to comment.