[mono] Clean up StackTrace/StackFrame icalls. (#81303)
authorZoltan Varga <vargaz@gmail.com>
Sat, 28 Jan 2023 09:53:46 +0000 (04:53 -0500)
committerGitHub <noreply@github.com>
Sat, 28 Jan 2023 09:53:46 +0000 (04:53 -0500)
* Register them normally in icall-def.h instead of calling
  mono_add_internal_call ().
* Remove unused Mono.Runtime::mono_runtime_install_handlers
  icall registration.
* Use ObjectHandleOnStack to pass/return objects.
* Move some of the logic to icall.c.

src/mono/System.Private.CoreLib/src/System/Diagnostics/StackFrame.Mono.cs
src/mono/System.Private.CoreLib/src/System/Diagnostics/StackTrace.Mono.cs
src/mono/System.Private.CoreLib/src/System/Exception.Mono.cs
src/mono/mono/metadata/icall-decl.h
src/mono/mono/metadata/icall-def.h
src/mono/mono/metadata/icall.c
src/mono/mono/metadata/object-internals.h
src/mono/mono/mini/mini-exceptions.c
src/mono/mono/mini/mini-runtime.c
src/mono/mono/mini/mini.h

index 0251fe2..188babf 100644 (file)
@@ -30,7 +30,15 @@ namespace System.Diagnostics
         {
             const int SystemDiagnosticsStackDepth = 3;
 
-            if (skipFrames + SystemDiagnosticsStackDepth < 0 || !get_frame_info(skipFrames + SystemDiagnosticsStackDepth, needFileInfo, out MethodBase? method, out int ilOffset, out int nativeOffset, out string? fileName, out int line, out int column))
+            if (skipFrames + SystemDiagnosticsStackDepth < 0)
+                return;
+
+            MethodBase? method = null;
+            string? fileName = null;
+            bool success = GetFrameInfo(skipFrames + SystemDiagnosticsStackDepth, needFileInfo,
+                                        ObjectHandleOnStack.Create (ref method), ObjectHandleOnStack.Create (ref fileName),
+                                        out int ilOffset, out int nativeOffset, out int line, out int column);
+            if (!success)
                 return;
 
             _method = method;
@@ -50,8 +58,8 @@ namespace System.Diagnostics
 #pragma warning restore IDE0060
 
         [MethodImplAttribute(MethodImplOptions.InternalCall)]
-        private static extern bool get_frame_info(int skipFrames, bool needFileInfo,
-            out MethodBase method, out int ilOffset, out int nativeOffset, out string file, out int line, out int column);
-
+        private static extern bool GetFrameInfo(int skipFrames, bool needFileInfo,
+                                                ObjectHandleOnStack out_method, ObjectHandleOnStack out_file,
+                                                out int ilOffset, out int nativeOffset, out int line, out int column);
     }
 }
index 14d6ff0..49b4da7 100644 (file)
@@ -34,7 +34,7 @@ namespace System.Diagnostics
     public partial class StackTrace
     {
         [MethodImplAttribute(MethodImplOptions.InternalCall)]
-        internal static extern MonoStackFrame[] get_trace(Exception e, int skipFrames, bool needFileInfo);
+        internal static extern void GetTrace(ObjectHandleOnStack ex, ObjectHandleOnStack res, int skipFrames, bool needFileInfo);
 
         [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
             Justification = "StackFrame.GetMethod is getting compared to null but nothing else on it is touched.")]
@@ -62,8 +62,9 @@ namespace System.Diagnostics
 
         private void InitializeForException(Exception e, int skipFrames, bool needFileInfo)
         {
-            MonoStackFrame[] frames = get_trace(e, skipFrames, needFileInfo);
-            _numOfFrames = frames.Length;
+            MonoStackFrame[]? frames = null;
+            GetTrace (ObjectHandleOnStack.Create (ref e), ObjectHandleOnStack.Create (ref frames), skipFrames, needFileInfo);
+            _numOfFrames = frames!.Length;
 
             int foreignFrames;
             MonoStackFrame[]? foreignExceptions = e.foreignExceptionsFrames;
index 3602c89..acaf925 100644 (file)
@@ -6,6 +6,7 @@ using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Reflection;
 using System.Runtime.InteropServices;
+using System.Runtime.CompilerServices;
 using System.Diagnostics.Tracing;
 
 namespace System
@@ -68,7 +69,10 @@ namespace System
 
             if (_traceIPs != null)
             {
-                stackFrames = Diagnostics.StackTrace.get_trace(this, 0, true);
+                Exception self = this;
+                MonoStackFrame[]? frames = null;
+                Diagnostics.StackTrace.GetTrace (ObjectHandleOnStack.Create (ref self), ObjectHandleOnStack.Create (ref frames), 0, true);
+                stackFrames = frames!;
                 if (stackFrames.Length > 0)
                     stackFrames[stackFrames.Length - 1].isLastFrameFromForeignException = true;
 
index 1eb6a7e..205a703 100644 (file)
@@ -210,4 +210,11 @@ ICALL_EXPORT MonoBoolean ves_icall_System_Array_FastCopy (MonoObjectHandleOnStac
 
 ICALL_EXPORT MonoBoolean ves_icall_System_Reflection_LoaderAllocatorScout_Destroy (gpointer native);
 
+ICALL_EXPORT void ves_icall_System_Diagnostics_StackTrace_GetTrace (MonoObjectHandleOnStack ex_handle, MonoObjectHandleOnStack res, int skip_frames, MonoBoolean need_file_info);
+
+ICALL_EXPORT MonoBoolean ves_icall_System_Diagnostics_StackFrame_GetFrameInfo (gint32 skip, MonoBoolean need_file_info,
+                                                                                                                                                          MonoObjectHandleOnStack out_method, MonoObjectHandleOnStack file,
+                                                                                                                                                          gint32 *iloffset, gint32 *native_offset,
+                                                                                                                                                          gint32 *line, gint32 *column);
+
 #endif // __MONO_METADATA_ICALL_DECL_H__
index 34b0c5d..813fbe3 100644 (file)
@@ -156,6 +156,12 @@ NOHANDLES(ICALL(DEBUGR_1, "IsAttached_internal", ves_icall_System_Diagnostics_De
 NOHANDLES(ICALL(DEBUGR_2, "IsLogging", ves_icall_System_Diagnostics_Debugger_IsLogging))
 NOHANDLES(ICALL(DEBUGR_3, "Log_icall", ves_icall_System_Diagnostics_Debugger_Log))
 
+ICALL_TYPE(STACKFRAME, "System.Diagnostics.StackFrame", STACKFRAME_1)
+NOHANDLES(ICALL(STACKFRAME_1, "GetFrameInfo", ves_icall_System_Diagnostics_StackFrame_GetFrameInfo))
+
+ICALL_TYPE(STACKTRACE, "System.Diagnostics.StackTrace", STACKTRACE_1)
+NOHANDLES(ICALL(STACKTRACE_1, "GetTrace", ves_icall_System_Diagnostics_StackTrace_GetTrace))
+
 ICALL_TYPE(EVENTPIPE, "System.Diagnostics.Tracing.EventPipeInternal", EVENTPIPE_1)
 HANDLES(EVENTPIPE_1, "CreateProvider", ves_icall_System_Diagnostics_Tracing_EventPipeInternal_CreateProvider, gconstpointer, 3, (MonoString, gpointer, gpointer))
 NOHANDLES(ICALL(EVENTPIPE_2, "DefineEvent", ves_icall_System_Diagnostics_Tracing_EventPipeInternal_DefineEvent))
index 7c2d7ec..41d4d94 100644 (file)
@@ -7304,6 +7304,59 @@ ves_icall_System_Environment_get_ProcessorCount (void)
        return mono_cpu_limit ();
 }
 
+void
+ves_icall_System_Diagnostics_StackTrace_GetTrace (MonoObjectHandleOnStack ex_handle, MonoObjectHandleOnStack res, int skip_frames, MonoBoolean need_file_info)
+{
+       MonoArray *trace = mono_get_runtime_callbacks ()->get_trace ((MonoException*)*ex_handle, skip_frames, need_file_info);
+       HANDLE_ON_STACK_SET (res, trace);
+}
+
+MonoBoolean
+ves_icall_System_Diagnostics_StackFrame_GetFrameInfo (gint32 skip, MonoBoolean need_file_info,
+                                                                                                         MonoObjectHandleOnStack out_method, MonoObjectHandleOnStack out_file,
+                                                                                                         gint32 *iloffset, gint32 *native_offset,
+                                                                                                         gint32 *line, gint32 *column)
+{
+       MonoMethod *method = NULL;
+       MonoDebugSourceLocation *location;
+       ERROR_DECL (error);
+
+       gboolean res = mono_get_runtime_callbacks ()->get_frame_info (skip, &method, &location, iloffset, native_offset);
+       if (!res)
+               return FALSE;
+
+       if (location)
+               *iloffset = location->il_offset;
+       else
+               *iloffset = 0;
+
+       if (need_file_info) {
+               if (location) {
+                       MonoString *filename = mono_string_new_checked (location->source_file, error);
+                       if (!is_ok (error)) {
+                               mono_error_set_pending_exception (error);
+                               return FALSE;
+                       }
+                       HANDLE_ON_STACK_SET (out_file, filename);
+                       *line = location->row;
+                       *column = location->column;
+               } else {
+                       *line = *column = 0;
+               }
+       }
+
+       mono_debug_free_source_location (location);
+
+       MonoReflectionMethod *rm = mono_method_get_object_checked (method, NULL, error);
+       if (!is_ok (error)) {
+               mono_error_set_pending_exception (error);
+               return FALSE;
+       }
+       HANDLE_ON_STACK_SET (out_method, rm);
+
+       return TRUE;
+}
+
 // Generate wrappers.
 
 #define ICALL_TYPE(id,name,first) /* nothing */
index 1a6d61e..0a50a64 100644 (file)
@@ -17,6 +17,7 @@
 #include <mono/metadata/threads-types.h>
 #include <mono/metadata/handle.h>
 #include <mono/metadata/abi-details.h>
+#include <mono/metadata/mono-debug.h>
 #include "mono/utils/mono-compiler.h"
 #include "mono/utils/mono-error.h"
 #include "mono/utils/mono-error-internals.h"
@@ -699,6 +700,10 @@ typedef struct {
        void (*interp_jit_info_foreach)(InterpJitInfoFunc func, gpointer user_data);
        gboolean (*interp_sufficient_stack)(gsize size);
        void (*init_class) (MonoClass *klass);
+       MonoArray *(*get_trace) (MonoException *exc, gint32 skip, MonoBoolean need_file_info);
+       MonoBoolean (*get_frame_info) (gint32 skip, MonoMethod **out_method,
+                                                                  MonoDebugSourceLocation **out_location,
+                                                                  gint32 *iloffset, gint32 *native_offset);
 } MonoRuntimeCallbacks;
 
 typedef gboolean (*MonoInternalStackWalk) (MonoStackFrameInfo *frame, MonoContext *ctx, gpointer data);
index f948c57..5693527 100644 (file)
@@ -1057,8 +1057,8 @@ mono_exception_walk_trace_internal (MonoException *ex, MonoExceptionFrameWalk fu
        return captured_has_traces || otherwise_has_traces;
 }
 
-MonoArray *
-ves_icall_get_trace (MonoException *exc, gint32 skip, MonoBoolean need_file_info)
+MonoArray*
+mono_get_trace (MonoException *exc, gint32 skip, MonoBoolean need_file_info)
 {
        ERROR_DECL (error);
        MonoArray *res;
@@ -1414,12 +1414,11 @@ next:
 }
 
 MonoBoolean
-ves_icall_get_frame_info (gint32 skip, MonoBoolean need_file_info,
-                         MonoReflectionMethod **method,
-                         gint32 *iloffset, gint32 *native_offset,
-                         MonoString **file, gint32 *line, gint32 *column)
+mono_get_frame_info (gint32 skip,
+                                        MonoMethod **out_method,
+                                        MonoDebugSourceLocation **out_location,
+                                        gint32 *iloffset, gint32 *native_offset)
 {
-       ERROR_DECL (error);
        MonoJitTlsData *jit_tls = mono_tls_get_jit_tls ();
        MonoLMF *lmf = mono_get_lmf ();
        MonoJitInfo *ji = NULL;
@@ -1465,7 +1464,7 @@ ves_icall_get_frame_info (gint32 skip, MonoBoolean need_file_info,
                *native_offset = GPTRDIFF_TO_INT32 (frame_ip - (guint8*)ji->code_start);
        } else {
                mono_arch_flush_register_windows ();
-               MONO_INIT_CONTEXT_FROM_FUNC (&ctx, ves_icall_get_frame_info);
+               MONO_INIT_CONTEXT_FROM_FUNC (&ctx, mono_get_frame_info);
 
                unwinder_init (&unwinder);
 
@@ -1508,40 +1507,15 @@ ves_icall_get_frame_info (gint32 skip, MonoBoolean need_file_info,
                }
        }
 
-       MonoReflectionMethod *rm = mono_method_get_object_checked (actual_method, NULL, error);
-       if (!is_ok (error)) {
-               mono_error_set_pending_exception (error);
-               return FALSE;
-       }
-       mono_gc_wbarrier_generic_store_internal (method, (MonoObject*) rm);
+       *out_method = actual_method;
 
        if (il_offset != -1) {
                location = mono_debug_lookup_source_location_by_il (jmethod, il_offset, NULL);
        } else {
                location = mono_debug_lookup_source_location (jmethod, *native_offset, NULL);
        }
-       if (location)
-               *iloffset = location->il_offset;
-       else
-               *iloffset = 0;
-
-       if (need_file_info) {
-               if (location) {
-                       MonoString *filename = mono_string_new_checked (location->source_file, error);
-                       if (!is_ok (error)) {
-                               mono_error_set_pending_exception (error);
-                               return FALSE;
-                       }
-                       mono_gc_wbarrier_generic_store_internal (file, (MonoObject*)filename);
-                       *line = location->row;
-                       *column = location->column;
-               } else {
-                       *file = NULL;
-                       *line = *column = 0;
-               }
-       }
 
-       mono_debug_free_source_location (location);
+       *out_location = location;
 
        return TRUE;
 }
index 14a3cca..ce664ed 100644 (file)
@@ -4571,6 +4571,8 @@ mini_init (const char *filename)
        callbacks.get_jit_stats = get_jit_stats;
        callbacks.get_exception_stats = get_exception_stats;
        callbacks.init_class = init_class;
+       callbacks.get_trace = mono_get_trace;
+       callbacks.get_frame_info = mono_get_frame_info;
 
        mono_install_callbacks (&callbacks);
 
@@ -4768,13 +4770,6 @@ mini_init (const char *filename)
 static void
 register_icalls (void)
 {
-       mono_add_internal_call_internal ("System.Diagnostics.StackFrame::get_frame_info",
-                               ves_icall_get_frame_info);
-       mono_add_internal_call_internal ("System.Diagnostics.StackTrace::get_trace",
-                               ves_icall_get_trace);
-       mono_add_internal_call_internal ("Mono.Runtime::mono_runtime_install_handlers",
-                               mono_runtime_install_handlers);
-
        /*
         * It's important that we pass `TRUE` as the last argument here, as
         * it causes the JIT to omit a wrapper for these icalls. If the JIT
index 3e92f2a..d4de135 100644 (file)
@@ -2595,14 +2595,10 @@ gpointer mono_get_restore_context               (void);
 gpointer mono_get_throw_corlib_exception        (void);
 gpointer mono_get_throw_exception_addr          (void);
 gpointer mono_get_rethrow_preserve_exception_addr          (void);
-ICALL_EXPORT
-MonoArray *ves_icall_get_trace                  (MonoException *exc, gint32 skip, MonoBoolean need_file_info);
-
-ICALL_EXPORT
-MonoBoolean ves_icall_get_frame_info            (gint32 skip, MonoBoolean need_file_info,
-                                                MonoReflectionMethod **method,
-                                                gint32 *iloffset, gint32 *native_offset,
-                                                MonoString **file, gint32 *line, gint32 *column);
+MonoArray* mono_get_trace (MonoException *exc, gint32 skip, MonoBoolean need_file_info);
+MonoBoolean mono_get_frame_info            (gint32 skip, MonoMethod **out_method,
+                                                                                       MonoDebugSourceLocation **out_location,
+                                                                                       gint32 *iloffset, gint32 *native_offset);
 void mono_set_cast_details                      (MonoClass *from, MonoClass *to);
 
 void mono_decompose_typechecks (MonoCompile *cfg);