[runtime] Setup stack trace for use in exception filters too

[radical]

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 .

[radical]

I don't know how to have a test case for the mono_ex->dynamic_methods change (combine the new dynamic_methods with the existing one in the MonoException object). It isn't being used anywhere else or being surfaced. But the change makes sense to me. And I did try it with a test case where I had two dynamic methods and without this, at the end we have reference to only one dynamic method.

[radical]

Also, reading handle_exception_first_pass, https://github.com/mono/mono/blob/master/mono/mini/mini-exceptions.c#L1654 suggests that this function can be called with a MonoException object which already has traces (and hence possibly dynamic_methods?). I tried to get to this case by : catching an exception which had 2 dynamic methods in it's trace. And then throwing that same exception object in a different callstack but I found that this time the object had no trace_ips, but it still had the two dynamic_methods! So, somebody is clearing trace_ips for the rethrow but forgot to clean up dynamic_methods too?

I'm not very familiar with the code, so I might be making sense only in my head :)

[radical]

Linux x64 - all the tests passed but the build failed at csprojdiff step.

[luhenry]

@alexanderkyte @vargaz could you please review?

[lewurm]

why do you reuse the exception here? Also, you should mark Throw () with [MethodImpl(MethodImplOptions.NoInlining)], otherwise the framecount is 1 too very likely.

[radical]

That's a good question. This was based on exception19.cs where the exception was being reused too. I think maybe we can enhance this to test that the trace was indeed reset when the filter invoked. Currently, trace from both the original exception and the new trace here would have 1 frame, but if we change Throw to instead have two frames then we will be checking if the filter got a new trace or not. How does that sound?

+		} catch (Exception ex) when (CheckTrace (ex, 2, true)) {
+			CheckTrace (ex, 2);
+		}

[lewurm]

sounds good 👍

[lewurm]

Looks good to me, good job! I'm not 100% sure about the dynamic_methods stuff. It looks like it makes sense, and also doesn't break any tests.

[radical]

Updated the test with https://gist.github.com/radical/26d9b11e185353ea4f869ce3fbeaf61a

[radical]

The runtime crashed while running some System/System.Xml tests. Looking at the trace I don't think my patch caused it but I don't know what made it crash.

https://gist.github.com/radical/0996979b5fdd2dce6f1b0a460a0d6e27

.. for Linux AArch64

[lewurm]

@monojenkins build Linux ARMv7

[lewurm]

@monojenkins build Linux ARMv5

[lewurm]

@monojenkins build Linux AArch64

[luhenry]

@monojenkins build Linux ARMv7

[luhenry]

@monojenkins build Linux AArch64

[lewurm]

@monojenkins rebase