From 3e78ee8defec1dbf8987504f93d9f6e1eb448b85 Mon Sep 17 00:00:00 2001 From: Zoltan Varga Date: Sat, 28 Jan 2023 04:53:46 -0500 Subject: [PATCH] [mono] Clean up StackTrace/StackFrame icalls. (#81303) * 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/System/Diagnostics/StackFrame.Mono.cs | 16 +++++-- .../src/System/Diagnostics/StackTrace.Mono.cs | 7 +-- .../src/System/Exception.Mono.cs | 6 ++- src/mono/mono/metadata/icall-decl.h | 7 +++ src/mono/mono/metadata/icall-def.h | 6 +++ src/mono/mono/metadata/icall.c | 53 ++++++++++++++++++++++ src/mono/mono/metadata/object-internals.h | 5 ++ src/mono/mono/mini/mini-exceptions.c | 44 ++++-------------- src/mono/mono/mini/mini-runtime.c | 9 +--- src/mono/mono/mini/mini.h | 12 ++--- 10 files changed, 107 insertions(+), 58 deletions(-) diff --git a/src/mono/System.Private.CoreLib/src/System/Diagnostics/StackFrame.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Diagnostics/StackFrame.Mono.cs index 0251fe2..188babf 100644 --- a/src/mono/System.Private.CoreLib/src/System/Diagnostics/StackFrame.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Diagnostics/StackFrame.Mono.cs @@ -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); } } diff --git a/src/mono/System.Private.CoreLib/src/System/Diagnostics/StackTrace.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Diagnostics/StackTrace.Mono.cs index 14d6ff0..49b4da7 100644 --- a/src/mono/System.Private.CoreLib/src/System/Diagnostics/StackTrace.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Diagnostics/StackTrace.Mono.cs @@ -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; diff --git a/src/mono/System.Private.CoreLib/src/System/Exception.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Exception.Mono.cs index 3602c89..acaf925 100644 --- a/src/mono/System.Private.CoreLib/src/System/Exception.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Exception.Mono.cs @@ -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; diff --git a/src/mono/mono/metadata/icall-decl.h b/src/mono/mono/metadata/icall-decl.h index 1eb6a7e..205a703 100644 --- a/src/mono/mono/metadata/icall-decl.h +++ b/src/mono/mono/metadata/icall-decl.h @@ -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__ diff --git a/src/mono/mono/metadata/icall-def.h b/src/mono/mono/metadata/icall-def.h index 34b0c5d..813fbe3 100644 --- a/src/mono/mono/metadata/icall-def.h +++ b/src/mono/mono/metadata/icall-def.h @@ -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)) diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index 7c2d7ec..41d4d94 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -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 */ diff --git a/src/mono/mono/metadata/object-internals.h b/src/mono/mono/metadata/object-internals.h index 1a6d61e3..0a50a64 100644 --- a/src/mono/mono/metadata/object-internals.h +++ b/src/mono/mono/metadata/object-internals.h @@ -17,6 +17,7 @@ #include #include #include +#include #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); diff --git a/src/mono/mono/mini/mini-exceptions.c b/src/mono/mono/mini/mini-exceptions.c index f948c57..5693527 100644 --- a/src/mono/mono/mini/mini-exceptions.c +++ b/src/mono/mono/mini/mini-exceptions.c @@ -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; } diff --git a/src/mono/mono/mini/mini-runtime.c b/src/mono/mono/mini/mini-runtime.c index 14a3cca..ce664ed 100644 --- a/src/mono/mono/mini/mini-runtime.c +++ b/src/mono/mono/mini/mini-runtime.c @@ -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 diff --git a/src/mono/mono/mini/mini.h b/src/mono/mono/mini/mini.h index 3e92f2a..d4de135 100644 --- a/src/mono/mono/mini/mini.h +++ b/src/mono/mono/mini/mini.h @@ -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); -- 2.7.4