From 78ce32e14888844e832f4c14c6bb688ae1c28999 Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Mon, 28 Aug 2023 13:45:30 +0200 Subject: [PATCH 01/21] Enable tests. --- src/tests/issues.targets | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index e142ff1292411..efcd8d4628e89 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -2405,15 +2405,6 @@ expected failure: overlapped structs fail at AOT compile time, not runtime - - expected failure: overlapped structs fail at AOT compile time, not runtime - - - expected failure: overlapped structs fail at AOT compile time, not runtime - - - expected failure: overlapped structs fail at AOT compile time, not runtime - expected failure: unsupported type with ref field fails at AOT compile time, not runtime From 3c32b67fef4323c6f9df1c66e5993b42d8035b81 Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Tue, 29 Aug 2023 14:00:15 +0200 Subject: [PATCH 02/21] When AOTing, type checks do not fail compilation but create a runtime exception. --- src/mono/mono/metadata/jit-icall-reg.h | 1 + src/mono/mono/mini/aot-compiler.c | 8 +++++- src/mono/mono/mini/jit-icalls.c | 8 ++++++ src/mono/mono/mini/jit-icalls.h | 2 ++ src/mono/mono/mini/method-to-ir.c | 34 ++++++++++++++++++++++---- src/mono/mono/mini/mini-runtime.c | 1 + src/mono/mono/mini/mini.c | 6 +++++ 7 files changed, 54 insertions(+), 6 deletions(-) diff --git a/src/mono/mono/metadata/jit-icall-reg.h b/src/mono/mono/metadata/jit-icall-reg.h index 02e30426578fb..2ac1bc9fb05d7 100644 --- a/src/mono/mono/metadata/jit-icall-reg.h +++ b/src/mono/mono/metadata/jit-icall-reg.h @@ -312,6 +312,7 @@ MONO_JIT_ICALL (mono_throw_bad_image) \ MONO_JIT_ICALL (mono_throw_not_supported) \ MONO_JIT_ICALL (mono_throw_platform_not_supported) \ MONO_JIT_ICALL (mono_throw_invalid_program) \ +MONO_JIT_ICALL (mono_throw_type_load) \ MONO_JIT_ICALL (mono_trace_enter_method) \ MONO_JIT_ICALL (mono_trace_leave_method) \ MONO_JIT_ICALL (mono_trace_tail_method) \ diff --git a/src/mono/mono/mini/aot-compiler.c b/src/mono/mono/mini/aot-compiler.c index 1944a77d2c00b..710e8a5f16194 100644 --- a/src/mono/mono/mini/aot-compiler.c +++ b/src/mono/mono/mini/aot-compiler.c @@ -9580,7 +9580,13 @@ compile_method (MonoAotCompile *acfg, MonoMethod *method) mono_atomic_inc_i32 (&acfg->stats.genericcount); return; } - if (cfg->exception_type != MONO_EXCEPTION_NONE) { + if (cfg->exception_type == MONO_EXCEPTION_TYPE_LOAD) { + // Clear the exception, method-to-ir has already replaced the INITOBJ of the invalid + // type with a THROW. + cfg->exception_type = MONO_EXCEPTION_NONE; + // TODO: log this? + } + else if (cfg->exception_type != MONO_EXCEPTION_NONE) { /* Some instances cannot be JITted due to constraints etc. */ if (!method->is_inflated) report_loader_error (acfg, cfg->error, FALSE, "Unable to compile method '%s' due to: '%s'.\n", mono_method_get_full_name (method), mono_error_get_message (cfg->error)); diff --git a/src/mono/mono/mini/jit-icalls.c b/src/mono/mono/mini/jit-icalls.c index 628d18abe35cf..3a2a1c1df55a5 100644 --- a/src/mono/mono/mini/jit-icalls.c +++ b/src/mono/mono/mini/jit-icalls.c @@ -1688,6 +1688,14 @@ mono_throw_invalid_program (const char *msg) mono_error_set_pending_exception (error); } +void +mono_throw_type_load (void) +{ + ERROR_DECL (error); + mono_error_set_generic_error (error, "System", "TypeLoadException", ""); + mono_error_set_pending_exception (error); +} + void mono_dummy_jit_icall (void) { diff --git a/src/mono/mono/mini/jit-icalls.h b/src/mono/mono/mini/jit-icalls.h index db59a7608c8da..845b9ca476744 100644 --- a/src/mono/mono/mini/jit-icalls.h +++ b/src/mono/mono/mini/jit-icalls.h @@ -234,6 +234,8 @@ ICALL_EXPORT void mono_throw_platform_not_supported (void); ICALL_EXPORT void mono_throw_invalid_program (const char *msg); +ICALL_EXPORT void mono_throw_type_load (void); + ICALL_EXPORT void mono_dummy_jit_icall (void); ICALL_EXPORT void mono_dummy_jit_icall_val (gpointer ptr); diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index e3605981bafcd..212612fe37395 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -166,8 +166,12 @@ mono_tailcall_print (const char *format, ...) } while (0) #define TYPE_LOAD_ERROR(klass) do { \ - cfg->exception_ptr = klass; \ - LOAD_ERROR; \ + if (cfg->compile_aot) { \ + mono_emit_jit_icall (cfg, mono_throw_type_load, NULL); \ + } else { \ + cfg->exception_ptr = klass; \ + LOAD_ERROR; \ + } \ } while (0) #define CHECK_CFG_ERROR do {\ @@ -9898,8 +9902,16 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b else { klass = NULL; field = mono_field_from_token_checked (image, token, &klass, generic_context, cfg->error); - if (!field) - CHECK_TYPELOAD (klass); + if (!field) { + if (cfg->compile_aot && (!klass || mono_class_has_failure (klass))) { + clear_cfg_error (cfg); + cfg->exception_type = MONO_EXCEPTION_NONE; + mono_emit_jit_icall (cfg, mono_throw_type_load, NULL); + break; + } else { + CHECK_TYPELOAD (klass); + } + } CHECK_CFG_ERROR; } if (!dont_verify && !cfg->skip_visibility && !mono_method_can_access_field (method, field)) @@ -11855,11 +11867,23 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b case MONO_CEE_INITOBJ: --sp; klass = mini_get_class (method, token, generic_context); - CHECK_TYPELOAD (klass); + + if (cfg->compile_aot) { + // In AOT we replace initobj of invalid types with a throw. + if (!klass || mono_class_has_failure (klass)) { + mono_emit_jit_icall (cfg, mono_throw_type_load, NULL); + inline_costs += 10; + break; + } + } else { + CHECK_TYPELOAD (klass); + } + if (mini_class_is_reference (klass)) MONO_EMIT_NEW_STORE_MEMBASE_IMM (cfg, OP_STORE_MEMBASE_IMM, sp [0]->dreg, 0, 0); else mini_emit_initobj (cfg, *sp, NULL, klass); + inline_costs += 1; break; case MONO_CEE_CONSTRAINED_: diff --git a/src/mono/mono/mini/mini-runtime.c b/src/mono/mono/mini/mini-runtime.c index d075623de8cf7..6f45106c2fe9c 100644 --- a/src/mono/mono/mini/mini-runtime.c +++ b/src/mono/mono/mini/mini-runtime.c @@ -5139,6 +5139,7 @@ register_icalls (void) register_icall (mono_throw_not_supported, mono_icall_sig_void, FALSE); register_icall (mono_throw_platform_not_supported, mono_icall_sig_void, FALSE); register_icall (mono_throw_invalid_program, mono_icall_sig_void_ptr, FALSE); + register_icall (mono_throw_type_load, mono_icall_sig_void, FALSE); register_icall_no_wrapper (mono_dummy_jit_icall, mono_icall_sig_void); //register_icall_no_wrapper (mono_dummy_jit_icall_val, mono_icall_sig_void_ptr); register_icall_no_wrapper (mini_init_method_rgctx, mono_icall_sig_void_ptr_ptr); diff --git a/src/mono/mono/mini/mini.c b/src/mono/mono/mini/mini.c index f96f13e32cc7c..f3508f98dc221 100644 --- a/src/mono/mono/mini/mini.c +++ b/src/mono/mono/mini/mini.c @@ -3491,6 +3491,9 @@ mini_method_compile (MonoMethod *method, guint32 opts, JitFlags flags, int parts */ mono_compile_create_vars (cfg); + if (cfg->exception_type == MONO_EXCEPTION_TYPE_LOAD) + cfg->exception_type = MONO_EXCEPTION_NONE; + mono_cfg_dump_create_context (cfg); mono_cfg_dump_begin_group (cfg); @@ -3706,6 +3709,9 @@ mini_method_compile (MonoMethod *method, guint32 opts, JitFlags flags, int parts } #endif + if (cfg->exception_type == MONO_EXCEPTION_TYPE_LOAD) + cfg->exception_type = MONO_EXCEPTION_NONE; + /* after SSA translation */ if (parts == 2) { if (MONO_METHOD_COMPILE_END_ENABLED ()) From 7676e00bfc7b4acabb09c8c2a72d34b348abcdda Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Wed, 30 Aug 2023 14:12:35 +0200 Subject: [PATCH 03/21] Cleaned up type load error cleaning. TypeLoadException icall now has a message with type name. --- src/mono/mono/mini/jit-icalls.c | 4 +- src/mono/mono/mini/jit-icalls.h | 2 +- src/mono/mono/mini/method-to-ir.c | 116 ++++++++++++++++++++---------- src/mono/mono/mini/mini-runtime.c | 2 +- src/mono/mono/mini/mini.c | 15 ++-- 5 files changed, 91 insertions(+), 48 deletions(-) diff --git a/src/mono/mono/mini/jit-icalls.c b/src/mono/mono/mini/jit-icalls.c index 3a2a1c1df55a5..5b447d3e4f159 100644 --- a/src/mono/mono/mini/jit-icalls.c +++ b/src/mono/mono/mini/jit-icalls.c @@ -1689,10 +1689,10 @@ mono_throw_invalid_program (const char *msg) } void -mono_throw_type_load (void) +mono_throw_type_load (const char* msg) { ERROR_DECL (error); - mono_error_set_generic_error (error, "System", "TypeLoadException", ""); + mono_error_set_generic_error (error, "System", "TypeLoadException", "Attempting to load invalid type '%s'.", msg); mono_error_set_pending_exception (error); } diff --git a/src/mono/mono/mini/jit-icalls.h b/src/mono/mono/mini/jit-icalls.h index 845b9ca476744..7a4ea025501f7 100644 --- a/src/mono/mono/mini/jit-icalls.h +++ b/src/mono/mono/mini/jit-icalls.h @@ -234,7 +234,7 @@ ICALL_EXPORT void mono_throw_platform_not_supported (void); ICALL_EXPORT void mono_throw_invalid_program (const char *msg); -ICALL_EXPORT void mono_throw_type_load (void); +ICALL_EXPORT void mono_throw_type_load (const char* msg); ICALL_EXPORT void mono_dummy_jit_icall (void); diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index 212612fe37395..7edbdecd58183 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -166,12 +166,8 @@ mono_tailcall_print (const char *format, ...) } while (0) #define TYPE_LOAD_ERROR(klass) do { \ - if (cfg->compile_aot) { \ - mono_emit_jit_icall (cfg, mono_throw_type_load, NULL); \ - } else { \ - cfg->exception_ptr = klass; \ - LOAD_ERROR; \ - } \ + cfg->exception_ptr = klass; \ + LOAD_ERROR; \ } while (0) #define CHECK_CFG_ERROR do {\ @@ -2307,6 +2303,20 @@ emit_not_supported_failure (MonoCompile *cfg) mono_emit_jit_icall (cfg, mono_throw_not_supported, NULL); } +static void +emit_type_load_failure (MonoCompile* cfg, MonoClass* klass) +{ + char* class_name; + if (G_UNLIKELY (!klass)) + class_name = mono_mem_manager_strdup (cfg->mem_manager, "Unknown class" ); + else + class_name = mono_mem_manager_strdup (cfg->mem_manager, mono_class_full_name (klass)); + + MonoInst* iargs[1]; + EMIT_NEW_LDSTRLITCONST (cfg, iargs [0], class_name); + mono_emit_jit_icall (cfg, mono_throw_type_load, iargs); +} + static void emit_invalid_program_with_msg (MonoCompile *cfg, MonoError *error_msg, MonoMethod *caller, MonoMethod *callee) { @@ -4967,6 +4977,11 @@ inline_method (MonoCompile *cfg, MonoMethod *cmethod, MonoMethodSignature *fsig, #define CHECK_UNVERIFIABLE(cfg) if (cfg->unverifiable) UNVERIFIED #define CHECK_TYPELOAD(klass) if (!(klass) || mono_class_has_failure (klass)) TYPE_LOAD_ERROR ((klass)) +#define CLEAR_TYPELOAD_EXCEPTION(cfg) if (cfg->exception_type == MONO_EXCEPTION_TYPE_LOAD) { clear_cfg_error (cfg); cfg->exception_type = MONO_EXCEPTION_NONE; } +#define IF_TYPELOAD_ERROR(klass) if (!(klass) || mono_class_has_failure (klass)) +#define HANDLE_TYPELOAD_ERROR(cfg,klass) if (!cfg->compile_aot) TYPE_LOAD_ERROR ((klass)); emit_type_load_failure (cfg, klass); CLEAR_TYPELOAD_EXCEPTION (cfg); + + /* offset from br.s -> br like opcodes */ #define BIG_BRANCH_OFFSET 13 @@ -5442,7 +5457,10 @@ emit_optimized_ldloca_ir (MonoCompile *cfg, guchar *ip, guchar *end, int local) if ((ip = il_read_initobj (ip, end, &token)) && ip_in_bb (cfg, cfg->cbb, start + 1)) { /* From the INITOBJ case */ klass = mini_get_class (cfg->current_method, token, cfg->generic_context); - CHECK_TYPELOAD (klass); + IF_TYPELOAD_ERROR (klass) { + HANDLE_TYPELOAD_ERROR (cfg, klass); + return ip; + } type = mini_get_underlying_type (m_class_get_byval_arg (klass)); emit_init_local (cfg, local, type, TRUE); return ip; @@ -9387,7 +9405,10 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b case MONO_CEE_ISINST: { --sp; klass = mini_get_class (method, token, generic_context); - CHECK_TYPELOAD (klass); + IF_TYPELOAD_ERROR (klass) { + HANDLE_TYPELOAD_ERROR (cfg, klass); + break; // reached only in AOT + } if (sp [0]->type != STACK_OBJ) UNVERIFIED; @@ -9409,7 +9430,10 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b --sp; klass = mini_get_class (method, token, generic_context); - CHECK_TYPELOAD (klass); + IF_TYPELOAD_ERROR (klass) { + HANDLE_TYPELOAD_ERROR (cfg, klass); + break; // reached in AOT only + } mono_save_token_info (cfg, image, token, klass); @@ -9841,7 +9865,10 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b case MONO_CEE_UNBOX: { --sp; klass = mini_get_class (method, token, generic_context); - CHECK_TYPELOAD (klass); + IF_TYPELOAD_ERROR (klass) { + HANDLE_TYPELOAD_ERROR (cfg, klass); + break; // reached in AOT only + } mono_save_token_info (cfg, image, token, klass); @@ -9903,13 +9930,9 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b klass = NULL; field = mono_field_from_token_checked (image, token, &klass, generic_context, cfg->error); if (!field) { - if (cfg->compile_aot && (!klass || mono_class_has_failure (klass))) { - clear_cfg_error (cfg); - cfg->exception_type = MONO_EXCEPTION_NONE; - mono_emit_jit_icall (cfg, mono_throw_type_load, NULL); - break; - } else { - CHECK_TYPELOAD (klass); + IF_TYPELOAD_ERROR (klass) { + HANDLE_TYPELOAD_ERROR (cfg, klass); + break; // reached only in AOT } } CHECK_CFG_ERROR; @@ -10397,7 +10420,10 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b case MONO_CEE_STOBJ: sp -= 2; klass = mini_get_class (method, token, generic_context); - CHECK_TYPELOAD (klass); + IF_TYPELOAD_ERROR (klass) { + HANDLE_TYPELOAD_ERROR (cfg, klass); + break; + } /* FIXME: should check item at sp [1] is compatible with the type of the store. */ mini_emit_memory_store (cfg, m_class_get_byval_arg (klass), sp [0], sp [1], ins_flag); @@ -10417,7 +10443,10 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b --sp; klass = mini_get_class (method, token, generic_context); - CHECK_TYPELOAD (klass); + IF_TYPELOAD_ERROR (klass) { + HANDLE_TYPELOAD_ERROR (cfg, klass); + break; + } if (m_class_get_byval_arg (klass)->type == MONO_TYPE_VOID) UNVERIFIED; @@ -10543,7 +10572,10 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b cfg->flags |= MONO_CFG_HAS_LDELEMA; klass = mini_get_class (method, token, generic_context); - CHECK_TYPELOAD (klass); + IF_TYPELOAD_ERROR (klass) { + HANDLE_TYPELOAD_ERROR (cfg, klass); + break; // reached only in AOT + } /* we need to make sure that this array is exactly the type it needs * to be for correctness. the wrappers are lax with their usage * so we need to ignore them here @@ -10576,7 +10608,10 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b if (il_op == MONO_CEE_LDELEM) { klass = mini_get_class (method, token, generic_context); - CHECK_TYPELOAD (klass); + IF_TYPELOAD_ERROR (klass) { + HANDLE_TYPELOAD_ERROR (cfg, klass); + break; // reached only in AOT + } mono_class_init_internal (klass); } else @@ -10624,7 +10659,10 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b if (il_op == MONO_CEE_STELEM) { klass = mini_get_class (method, token, generic_context); - CHECK_TYPELOAD (klass); + IF_TYPELOAD_ERROR (klass) { + HANDLE_TYPELOAD_ERROR (cfg, klass); + break; // reached only in AOT + } mono_class_init_internal (klass); } else @@ -10671,7 +10709,10 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b MONO_INST_NEW (cfg, ins, il_op); --sp; klass = mini_get_class (method, token, generic_context); - CHECK_TYPELOAD (klass); + IF_TYPELOAD_ERROR (klass) { + HANDLE_TYPELOAD_ERROR (cfg, klass); + break; // reached only in AOT + } context_used = mini_class_check_context_used (cfg, klass); @@ -10708,7 +10749,10 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b MONO_INST_NEW (cfg, ins, il_op); --sp; klass = mini_get_class (method, token, generic_context); - CHECK_TYPELOAD (klass); + IF_TYPELOAD_ERROR (klass) { + HANDLE_TYPELOAD_ERROR (cfg, klass); + break; // reached only in AOT + } context_used = mini_class_check_context_used (cfg, klass); @@ -11868,17 +11912,12 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b --sp; klass = mini_get_class (method, token, generic_context); - if (cfg->compile_aot) { - // In AOT we replace initobj of invalid types with a throw. - if (!klass || mono_class_has_failure (klass)) { - mono_emit_jit_icall (cfg, mono_throw_type_load, NULL); - inline_costs += 10; - break; - } - } else { - CHECK_TYPELOAD (klass); + IF_TYPELOAD_ERROR (klass) { + HANDLE_TYPELOAD_ERROR (cfg, klass); + inline_costs += 10; + break; // reached only in AOT } - + if (mini_class_is_reference (klass)) MONO_EMIT_NEW_STORE_MEMBASE_IMM (cfg, OP_STORE_MEMBASE_IMM, sp [0]->dreg, 0, 0); else @@ -11888,7 +11927,9 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b break; case MONO_CEE_CONSTRAINED_: constrained_class = mini_get_class (method, token, generic_context); - CHECK_TYPELOAD (constrained_class); + IF_TYPELOAD_ERROR (constrained_class) { + HANDLE_TYPELOAD_ERROR (cfg, klass); + } ins_has_side_effect = FALSE; break; case MONO_CEE_CPBLK: @@ -11973,7 +12014,10 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b EMIT_NEW_ICONST (cfg, ins, val); } else { klass = mini_get_class (method, token, generic_context); - CHECK_TYPELOAD (klass); + IF_TYPELOAD_ERROR (klass) { + HANDLE_TYPELOAD_ERROR (cfg, klass); + break; + } if (mini_is_gsharedvt_klass (klass)) { ins = mini_emit_get_gsharedvt_info_klass (cfg, klass, MONO_RGCTX_INFO_CLASS_SIZEOF); diff --git a/src/mono/mono/mini/mini-runtime.c b/src/mono/mono/mini/mini-runtime.c index 6f45106c2fe9c..5298e1b492336 100644 --- a/src/mono/mono/mini/mini-runtime.c +++ b/src/mono/mono/mini/mini-runtime.c @@ -5139,7 +5139,7 @@ register_icalls (void) register_icall (mono_throw_not_supported, mono_icall_sig_void, FALSE); register_icall (mono_throw_platform_not_supported, mono_icall_sig_void, FALSE); register_icall (mono_throw_invalid_program, mono_icall_sig_void_ptr, FALSE); - register_icall (mono_throw_type_load, mono_icall_sig_void, FALSE); + register_icall (mono_throw_type_load, mono_icall_sig_void_ptr, FALSE); register_icall_no_wrapper (mono_dummy_jit_icall, mono_icall_sig_void); //register_icall_no_wrapper (mono_dummy_jit_icall_val, mono_icall_sig_void_ptr); register_icall_no_wrapper (mini_init_method_rgctx, mono_icall_sig_void_ptr_ptr); diff --git a/src/mono/mono/mini/mini.c b/src/mono/mono/mini/mini.c index f3508f98dc221..4674a669b1478 100644 --- a/src/mono/mono/mini/mini.c +++ b/src/mono/mono/mini/mini.c @@ -660,7 +660,9 @@ mono_compile_create_var_for_vreg (MonoCompile *cfg, MonoType *type, int opcode, inst->backend.is_pinvoke = 0; inst->dreg = vreg; - if (mono_class_has_failure (inst->klass)) + // In AOT, we do not set the exception so that the compilation can succeed. To indicate + // the error, an exception is thrown in run-time. + if (!cfg->compile_aot && mono_class_has_failure (inst->klass)) mono_cfg_set_exception (cfg, MONO_EXCEPTION_TYPE_LOAD); if (cfg->compute_gc_maps) { @@ -1351,7 +1353,10 @@ mono_allocate_stack_slots (MonoCompile *cfg, gboolean backward, guint32 *stack_s size = mini_type_stack_size (t, &ialign); align = ialign; - if (mono_class_has_failure (mono_class_from_mono_type_internal (t))) + // In AOT, we do not set the exception but allow the compilation to succeed. The error will be + // indicated in runtime by throwing an exception when an operation with the invalid object is + // attempted. + if (!cfg->compile_aot && mono_class_has_failure (mono_class_from_mono_type_internal (t))) mono_cfg_set_exception (cfg, MONO_EXCEPTION_TYPE_LOAD); if (mini_class_is_simd (cfg, mono_class_from_mono_type_internal (t))) @@ -3491,9 +3496,6 @@ mini_method_compile (MonoMethod *method, guint32 opts, JitFlags flags, int parts */ mono_compile_create_vars (cfg); - if (cfg->exception_type == MONO_EXCEPTION_TYPE_LOAD) - cfg->exception_type = MONO_EXCEPTION_NONE; - mono_cfg_dump_create_context (cfg); mono_cfg_dump_begin_group (cfg); @@ -3709,9 +3711,6 @@ mini_method_compile (MonoMethod *method, guint32 opts, JitFlags flags, int parts } #endif - if (cfg->exception_type == MONO_EXCEPTION_TYPE_LOAD) - cfg->exception_type = MONO_EXCEPTION_NONE; - /* after SSA translation */ if (parts == 2) { if (MONO_METHOD_COMPILE_END_ENABLED ()) From 611888bc845197f6dcb8a0c4e109413742dde543 Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Wed, 30 Aug 2023 14:16:36 +0200 Subject: [PATCH 04/21] Removed another instance of indiscriminate exception clearing. --- src/mono/mono/mini/aot-compiler.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/mono/mono/mini/aot-compiler.c b/src/mono/mono/mini/aot-compiler.c index 710e8a5f16194..0ef29cf43de77 100644 --- a/src/mono/mono/mini/aot-compiler.c +++ b/src/mono/mono/mini/aot-compiler.c @@ -9580,13 +9580,8 @@ compile_method (MonoAotCompile *acfg, MonoMethod *method) mono_atomic_inc_i32 (&acfg->stats.genericcount); return; } - if (cfg->exception_type == MONO_EXCEPTION_TYPE_LOAD) { - // Clear the exception, method-to-ir has already replaced the INITOBJ of the invalid - // type with a THROW. - cfg->exception_type = MONO_EXCEPTION_NONE; - // TODO: log this? - } - else if (cfg->exception_type != MONO_EXCEPTION_NONE) { + + if (cfg->exception_type != MONO_EXCEPTION_NONE) { /* Some instances cannot be JITted due to constraints etc. */ if (!method->is_inflated) report_loader_error (acfg, cfg->error, FALSE, "Unable to compile method '%s' due to: '%s'.\n", mono_method_get_full_name (method), mono_error_get_message (cfg->error)); From 15b7aa5aaa30aa6aff51c1c718e4044110ec378e Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Wed, 30 Aug 2023 14:44:31 +0200 Subject: [PATCH 05/21] Fixed build warning. --- src/mono/mono/mini/method-to-ir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index 7edbdecd58183..73eea9f2404d4 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -11928,7 +11928,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b case MONO_CEE_CONSTRAINED_: constrained_class = mini_get_class (method, token, generic_context); IF_TYPELOAD_ERROR (constrained_class) { - HANDLE_TYPELOAD_ERROR (cfg, klass); + HANDLE_TYPELOAD_ERROR (cfg, constrained_class); } ins_has_side_effect = FALSE; break; From 90aa674841ada8fc2d9dfee22d05ae071a2c9939 Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Thu, 31 Aug 2023 14:43:26 +0200 Subject: [PATCH 06/21] Using class const instead of string const. Reverted some compile to runtime errors that were not necessary for the unit tests. --- src/mono/mono/mini/jit-icalls.c | 7 ++- src/mono/mono/mini/jit-icalls.h | 2 +- src/mono/mono/mini/method-to-ir.c | 72 +++++++++---------------------- 3 files changed, 27 insertions(+), 54 deletions(-) diff --git a/src/mono/mono/mini/jit-icalls.c b/src/mono/mono/mini/jit-icalls.c index 5b447d3e4f159..b4eadd6fc2574 100644 --- a/src/mono/mono/mini/jit-icalls.c +++ b/src/mono/mono/mini/jit-icalls.c @@ -1689,11 +1689,14 @@ mono_throw_invalid_program (const char *msg) } void -mono_throw_type_load (const char* msg) +mono_throw_type_load (MonoClass* klass) { ERROR_DECL (error); - mono_error_set_generic_error (error, "System", "TypeLoadException", "Attempting to load invalid type '%s'.", msg); + char* klass_name = mono_type_get_full_name (klass); + mono_error_set_type_load_class (error, klass, "Attempting to load invalid type '%s'.", klass_name); mono_error_set_pending_exception (error); + + g_free (klass_name); } void diff --git a/src/mono/mono/mini/jit-icalls.h b/src/mono/mono/mini/jit-icalls.h index 7a4ea025501f7..6e9ea58124e1e 100644 --- a/src/mono/mono/mini/jit-icalls.h +++ b/src/mono/mono/mini/jit-icalls.h @@ -234,7 +234,7 @@ ICALL_EXPORT void mono_throw_platform_not_supported (void); ICALL_EXPORT void mono_throw_invalid_program (const char *msg); -ICALL_EXPORT void mono_throw_type_load (const char* msg); +ICALL_EXPORT void mono_throw_type_load (MonoClass* klass); ICALL_EXPORT void mono_dummy_jit_icall (void); diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index 73eea9f2404d4..4c6e59001d3c3 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -2306,14 +2306,12 @@ emit_not_supported_failure (MonoCompile *cfg) static void emit_type_load_failure (MonoCompile* cfg, MonoClass* klass) { - char* class_name; - if (G_UNLIKELY (!klass)) - class_name = mono_mem_manager_strdup (cfg->mem_manager, "Unknown class" ); - else - class_name = mono_mem_manager_strdup (cfg->mem_manager, mono_class_full_name (klass)); + MonoClass* failed_class = klass; + if (G_UNLIKELY (!failed_class)) + failed_class = mono_defaults.object_class; MonoInst* iargs[1]; - EMIT_NEW_LDSTRLITCONST (cfg, iargs [0], class_name); + EMIT_NEW_CLASSCONST (cfg, iargs [0], failed_class); mono_emit_jit_icall (cfg, mono_throw_type_load, iargs); } @@ -4979,8 +4977,12 @@ inline_method (MonoCompile *cfg, MonoMethod *cmethod, MonoMethodSignature *fsig, #define CLEAR_TYPELOAD_EXCEPTION(cfg) if (cfg->exception_type == MONO_EXCEPTION_TYPE_LOAD) { clear_cfg_error (cfg); cfg->exception_type = MONO_EXCEPTION_NONE; } #define IF_TYPELOAD_ERROR(klass) if (!(klass) || mono_class_has_failure (klass)) -#define HANDLE_TYPELOAD_ERROR(cfg,klass) if (!cfg->compile_aot) TYPE_LOAD_ERROR ((klass)); emit_type_load_failure (cfg, klass); CLEAR_TYPELOAD_EXCEPTION (cfg); - +#define HANDLE_TYPELOAD_ERROR(cfg,klass) do { \ + if (!cfg->compile_aot) \ + TYPE_LOAD_ERROR ((klass)); \ + emit_type_load_failure (cfg, klass); \ + CLEAR_TYPELOAD_EXCEPTION (cfg); \ + } while (0) /* offset from br.s -> br like opcodes */ #define BIG_BRANCH_OFFSET 13 @@ -9405,10 +9407,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b case MONO_CEE_ISINST: { --sp; klass = mini_get_class (method, token, generic_context); - IF_TYPELOAD_ERROR (klass) { - HANDLE_TYPELOAD_ERROR (cfg, klass); - break; // reached only in AOT - } + CHECK_TYPELOAD (klass); if (sp [0]->type != STACK_OBJ) UNVERIFIED; @@ -9430,10 +9429,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b --sp; klass = mini_get_class (method, token, generic_context); - IF_TYPELOAD_ERROR (klass) { - HANDLE_TYPELOAD_ERROR (cfg, klass); - break; // reached in AOT only - } + CHECK_TYPELOAD (klass); mono_save_token_info (cfg, image, token, klass); @@ -9865,10 +9861,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b case MONO_CEE_UNBOX: { --sp; klass = mini_get_class (method, token, generic_context); - IF_TYPELOAD_ERROR (klass) { - HANDLE_TYPELOAD_ERROR (cfg, klass); - break; // reached in AOT only - } + CHECK_TYPELOAD (klass); mono_save_token_info (cfg, image, token, klass); @@ -10420,10 +10413,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b case MONO_CEE_STOBJ: sp -= 2; klass = mini_get_class (method, token, generic_context); - IF_TYPELOAD_ERROR (klass) { - HANDLE_TYPELOAD_ERROR (cfg, klass); - break; - } + CHECK_TYPELOAD (klass); /* FIXME: should check item at sp [1] is compatible with the type of the store. */ mini_emit_memory_store (cfg, m_class_get_byval_arg (klass), sp [0], sp [1], ins_flag); @@ -10443,10 +10433,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b --sp; klass = mini_get_class (method, token, generic_context); - IF_TYPELOAD_ERROR (klass) { - HANDLE_TYPELOAD_ERROR (cfg, klass); - break; - } + CHECK_TYPELOAD (klass); if (m_class_get_byval_arg (klass)->type == MONO_TYPE_VOID) UNVERIFIED; @@ -10572,10 +10559,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b cfg->flags |= MONO_CFG_HAS_LDELEMA; klass = mini_get_class (method, token, generic_context); - IF_TYPELOAD_ERROR (klass) { - HANDLE_TYPELOAD_ERROR (cfg, klass); - break; // reached only in AOT - } + CHECK_TYPELOAD (klass); /* we need to make sure that this array is exactly the type it needs * to be for correctness. the wrappers are lax with their usage * so we need to ignore them here @@ -10608,10 +10592,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b if (il_op == MONO_CEE_LDELEM) { klass = mini_get_class (method, token, generic_context); - IF_TYPELOAD_ERROR (klass) { - HANDLE_TYPELOAD_ERROR (cfg, klass); - break; // reached only in AOT - } + CHECK_TYPELOAD (klass); mono_class_init_internal (klass); } else @@ -10659,10 +10640,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b if (il_op == MONO_CEE_STELEM) { klass = mini_get_class (method, token, generic_context); - IF_TYPELOAD_ERROR (klass) { - HANDLE_TYPELOAD_ERROR (cfg, klass); - break; // reached only in AOT - } + CHECK_TYPELOAD (klass); mono_class_init_internal (klass); } else @@ -10709,10 +10687,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b MONO_INST_NEW (cfg, ins, il_op); --sp; klass = mini_get_class (method, token, generic_context); - IF_TYPELOAD_ERROR (klass) { - HANDLE_TYPELOAD_ERROR (cfg, klass); - break; // reached only in AOT - } + CHECK_TYPELOAD (klass); context_used = mini_class_check_context_used (cfg, klass); @@ -10749,10 +10724,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b MONO_INST_NEW (cfg, ins, il_op); --sp; klass = mini_get_class (method, token, generic_context); - IF_TYPELOAD_ERROR (klass) { - HANDLE_TYPELOAD_ERROR (cfg, klass); - break; // reached only in AOT - } + CHECK_TYPELOAD (klass); context_used = mini_class_check_context_used (cfg, klass); @@ -11927,9 +11899,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b break; case MONO_CEE_CONSTRAINED_: constrained_class = mini_get_class (method, token, generic_context); - IF_TYPELOAD_ERROR (constrained_class) { - HANDLE_TYPELOAD_ERROR (cfg, constrained_class); - } + CHECK_TYPELOAD (klass); ins_has_side_effect = FALSE; break; case MONO_CEE_CPBLK: From d4b6c9fbcf2c7311a85b509a598ca4e6110b4a27 Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Thu, 31 Aug 2023 14:51:10 +0200 Subject: [PATCH 07/21] White space. --- src/mono/mono/mini/aot-compiler.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/mono/mono/mini/aot-compiler.c b/src/mono/mono/mini/aot-compiler.c index 0ef29cf43de77..1944a77d2c00b 100644 --- a/src/mono/mono/mini/aot-compiler.c +++ b/src/mono/mono/mini/aot-compiler.c @@ -9580,7 +9580,6 @@ compile_method (MonoAotCompile *acfg, MonoMethod *method) mono_atomic_inc_i32 (&acfg->stats.genericcount); return; } - if (cfg->exception_type != MONO_EXCEPTION_NONE) { /* Some instances cannot be JITted due to constraints etc. */ if (!method->is_inflated) From bda12b3916e1e1a8c5fb788f4d55e2fc1275a3e2 Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Thu, 31 Aug 2023 15:22:58 +0200 Subject: [PATCH 08/21] Fixed build warning. --- src/mono/mono/mini/method-to-ir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index 4c6e59001d3c3..b68f32dd65d2c 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -11899,7 +11899,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b break; case MONO_CEE_CONSTRAINED_: constrained_class = mini_get_class (method, token, generic_context); - CHECK_TYPELOAD (klass); + CHECK_TYPELOAD (constrained_class); ins_has_side_effect = FALSE; break; case MONO_CEE_CPBLK: From fbd361462670393c1d34696e72bebb941a2bdbb0 Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Thu, 31 Aug 2023 16:45:39 +0200 Subject: [PATCH 09/21] Trying to fix weird AOT errors, fixed type load throw function. --- src/mono/mono/mini/jit-icalls.c | 13 +++++++++---- src/mono/mono/mini/method-to-ir.c | 9 +++------ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/mono/mono/mini/jit-icalls.c b/src/mono/mono/mini/jit-icalls.c index b4eadd6fc2574..611877a182f11 100644 --- a/src/mono/mono/mini/jit-icalls.c +++ b/src/mono/mono/mini/jit-icalls.c @@ -1692,11 +1692,16 @@ void mono_throw_type_load (MonoClass* klass) { ERROR_DECL (error); - char* klass_name = mono_type_get_full_name (klass); - mono_error_set_type_load_class (error, klass, "Attempting to load invalid type '%s'.", klass_name); - mono_error_set_pending_exception (error); - g_free (klass_name); + if (G_UNLIKELY(!klass)) { + mono_error_set_type_load_class (error, klass, "Attempting to load an invalid type."); + } else { + char* klass_name = mono_type_get_full_name (klass); + mono_error_set_type_load_class (error, klass, "Attempting to load invalid type '%s'.", klass_name); + g_free (klass_name); + } + + mono_error_set_pending_exception (error); } void diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index b68f32dd65d2c..07cf1f5807878 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -2306,12 +2306,8 @@ emit_not_supported_failure (MonoCompile *cfg) static void emit_type_load_failure (MonoCompile* cfg, MonoClass* klass) { - MonoClass* failed_class = klass; - if (G_UNLIKELY (!failed_class)) - failed_class = mono_defaults.object_class; - MonoInst* iargs[1]; - EMIT_NEW_CLASSCONST (cfg, iargs [0], failed_class); + EMIT_NEW_CLASSCONST (cfg, iargs [0], klass); mono_emit_jit_icall (cfg, mono_throw_type_load, iargs); } @@ -5461,7 +5457,6 @@ emit_optimized_ldloca_ir (MonoCompile *cfg, guchar *ip, guchar *end, int local) klass = mini_get_class (cfg->current_method, token, cfg->generic_context); IF_TYPELOAD_ERROR (klass) { HANDLE_TYPELOAD_ERROR (cfg, klass); - return ip; } type = mini_get_underlying_type (m_class_get_byval_arg (klass)); emit_init_local (cfg, local, type, TRUE); @@ -11986,6 +11981,8 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b klass = mini_get_class (method, token, generic_context); IF_TYPELOAD_ERROR (klass) { HANDLE_TYPELOAD_ERROR (cfg, klass); + EMIT_NEW_ICONST(cfg, ins, val, 0); + *sp++ = ins; break; } From 805d6c44992c77c574f8d1ac6f6330b0a304c565 Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Thu, 31 Aug 2023 16:57:53 +0200 Subject: [PATCH 10/21] Fixed build error. --- src/mono/mono/mini/method-to-ir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index 07cf1f5807878..21d4262ccd9ab 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -11981,7 +11981,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b klass = mini_get_class (method, token, generic_context); IF_TYPELOAD_ERROR (klass) { HANDLE_TYPELOAD_ERROR (cfg, klass); - EMIT_NEW_ICONST(cfg, ins, val, 0); + EMIT_NEW_ICONST(cfg, ins, 0); *sp++ = ins; break; } From 0b50974a033f2e96ef0333ab9e5de85399a95627 Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Fri, 1 Sep 2023 12:31:52 +0200 Subject: [PATCH 11/21] Special handling for classes that are NULL. --- src/mono/mono/mini/method-to-ir.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index 21d4262ccd9ab..afe3a85f6a384 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -2307,7 +2307,11 @@ static void emit_type_load_failure (MonoCompile* cfg, MonoClass* klass) { MonoInst* iargs[1]; - EMIT_NEW_CLASSCONST (cfg, iargs [0], klass); + if (G_LIKELY (klass)) { + EMIT_NEW_CLASSCONST (cfg, iargs [0], klass); + } else { + EMIT_NEW_PCONST (cfg, iargs [0], NULL); + } mono_emit_jit_icall (cfg, mono_throw_type_load, iargs); } From 19c5155fbe0d8a4146a07278b34649bd9bb2d3e9 Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Wed, 6 Sep 2023 11:01:49 +0200 Subject: [PATCH 12/21] Providing for a null klass when generating exception. --- src/mono/mono/mini/jit-icalls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/mono/mini/jit-icalls.c b/src/mono/mono/mini/jit-icalls.c index 611877a182f11..15d78d56a271e 100644 --- a/src/mono/mono/mini/jit-icalls.c +++ b/src/mono/mono/mini/jit-icalls.c @@ -1694,7 +1694,7 @@ mono_throw_type_load (MonoClass* klass) ERROR_DECL (error); if (G_UNLIKELY(!klass)) { - mono_error_set_type_load_class (error, klass, "Attempting to load an invalid type."); + mono_error_set_generic_error (error, "System", "TypeLoadException", ""); } else { char* klass_name = mono_type_get_full_name (klass); mono_error_set_type_load_class (error, klass, "Attempting to load invalid type '%s'.", klass_name); From 83bcc30d12113f57d962412753c0e1c9177d8c28 Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Wed, 6 Sep 2023 11:07:49 +0200 Subject: [PATCH 13/21] Removed flow control directive from macro. --- src/mono/mono/mini/method-to-ir.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index afe3a85f6a384..b659b539da68d 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -4976,7 +4976,7 @@ inline_method (MonoCompile *cfg, MonoMethod *cmethod, MonoMethodSignature *fsig, #define CHECK_TYPELOAD(klass) if (!(klass) || mono_class_has_failure (klass)) TYPE_LOAD_ERROR ((klass)) #define CLEAR_TYPELOAD_EXCEPTION(cfg) if (cfg->exception_type == MONO_EXCEPTION_TYPE_LOAD) { clear_cfg_error (cfg); cfg->exception_type = MONO_EXCEPTION_NONE; } -#define IF_TYPELOAD_ERROR(klass) if (!(klass) || mono_class_has_failure (klass)) +#define CLASS_HAS_FAILURE(klass) (!(klass) || mono_class_has_failure (klass)) #define HANDLE_TYPELOAD_ERROR(cfg,klass) do { \ if (!cfg->compile_aot) \ TYPE_LOAD_ERROR ((klass)); \ @@ -5459,7 +5459,7 @@ emit_optimized_ldloca_ir (MonoCompile *cfg, guchar *ip, guchar *end, int local) if ((ip = il_read_initobj (ip, end, &token)) && ip_in_bb (cfg, cfg->cbb, start + 1)) { /* From the INITOBJ case */ klass = mini_get_class (cfg->current_method, token, cfg->generic_context); - IF_TYPELOAD_ERROR (klass) { + if (CLASS_HAS_FAILURE (klass)) { HANDLE_TYPELOAD_ERROR (cfg, klass); } type = mini_get_underlying_type (m_class_get_byval_arg (klass)); @@ -9921,11 +9921,9 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b else { klass = NULL; field = mono_field_from_token_checked (image, token, &klass, generic_context, cfg->error); - if (!field) { - IF_TYPELOAD_ERROR (klass) { + if (!field && CLASS_HAS_FAILURE (klass)) { HANDLE_TYPELOAD_ERROR (cfg, klass); break; // reached only in AOT - } } CHECK_CFG_ERROR; } @@ -11883,7 +11881,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b --sp; klass = mini_get_class (method, token, generic_context); - IF_TYPELOAD_ERROR (klass) { + if (CLASS_HAS_FAILURE (klass)) { HANDLE_TYPELOAD_ERROR (cfg, klass); inline_costs += 10; break; // reached only in AOT @@ -11983,7 +11981,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b EMIT_NEW_ICONST (cfg, ins, val); } else { klass = mini_get_class (method, token, generic_context); - IF_TYPELOAD_ERROR (klass) { + if (CLASS_HAS_FAILURE (klass)) { HANDLE_TYPELOAD_ERROR (cfg, klass); EMIT_NEW_ICONST(cfg, ins, 0); *sp++ = ins; From c6fe63b20fbd967356db2be13d9ed9b61ffcf1cf Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Wed, 6 Sep 2023 13:16:14 +0200 Subject: [PATCH 14/21] Fixed stack corruption. --- src/mono/mono/mini/method-to-ir.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index b659b539da68d..99cd12c1ef547 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -9921,9 +9921,20 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b else { klass = NULL; field = mono_field_from_token_checked (image, token, &klass, generic_context, cfg->error); - if (!field && CLASS_HAS_FAILURE (klass)) { + if (!field || CLASS_HAS_FAILURE (klass)) { HANDLE_TYPELOAD_ERROR (cfg, klass); - break; // reached only in AOT + + // Reached only in AOT. Cannot turn a token into a class. We silence the compilation error + // and generate a runtime exception. + if (cfg->error->error_code == MONO_ERROR_BAD_IMAGE) + clear_cfg_error (cfg); + + // We need to push something to the stack for these opcodes. + if (il_op == MONO_CEE_LDFLD || il_op == MONO_CEE_LDSFLD || il_op == MONO_CEE_LDFLDA || il_op == MONO_CEE_LDSFLDA) { + EMIT_NEW_PCONST (cfg, *sp, NULL); + sp++; + } + break; } CHECK_CFG_ERROR; } @@ -11878,15 +11889,15 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b inline_costs += 100000; break; case MONO_CEE_INITOBJ: - --sp; klass = mini_get_class (method, token, generic_context); - if (CLASS_HAS_FAILURE (klass)) { HANDLE_TYPELOAD_ERROR (cfg, klass); inline_costs += 10; break; // reached only in AOT } + --sp; + if (mini_class_is_reference (klass)) MONO_EMIT_NEW_STORE_MEMBASE_IMM (cfg, OP_STORE_MEMBASE_IMM, sp [0]->dreg, 0, 0); else @@ -11983,8 +11994,8 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b klass = mini_get_class (method, token, generic_context); if (CLASS_HAS_FAILURE (klass)) { HANDLE_TYPELOAD_ERROR (cfg, klass); - EMIT_NEW_ICONST(cfg, ins, 0); - *sp++ = ins; + //EMIT_NEW_ICONST(cfg, ins, 0); + //*sp++ = ins; break; } From 9cb3d1ab888875d3a4e023dd9a659b6555c9d0ba Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Fri, 8 Sep 2023 12:59:58 +0200 Subject: [PATCH 15/21] Attempt to push the correct type onto the stack. --- src/mono/mono/metadata/class.c | 4 ++-- src/mono/mono/mini/method-to-ir.c | 24 +++++++++++++++++++++--- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index 284e3153a1a25..94eff9802c924 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -2392,8 +2392,8 @@ static MonoClassField * mono_class_get_field_idx (MonoClass *klass, int idx) { mono_class_setup_fields (klass); - if (mono_class_has_failure (klass)) - return NULL; + //if (mono_class_has_failure (klass)) + //return NULL; while (klass) { int first_field_idx = mono_class_get_first_field_idx (klass); diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index 99cd12c1ef547..0ec814dd378e2 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -9929,12 +9929,28 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b if (cfg->error->error_code == MONO_ERROR_BAD_IMAGE) clear_cfg_error (cfg); - // We need to push something to the stack for these opcodes. - if (il_op == MONO_CEE_LDFLD || il_op == MONO_CEE_LDSFLD || il_op == MONO_CEE_LDFLDA || il_op == MONO_CEE_LDSFLDA) { + // We need to push a dummy value onto the stack, respecting the intended type. + if (il_op == MONO_CEE_LDFLDA || il_op == MONO_CEE_LDSFLDA) { + // Address is expected, push a null pointer. EMIT_NEW_PCONST (cfg, *sp, NULL); sp++; + } else if (il_op == MONO_CEE_LDFLD || il_op == MONO_CEE_LDSFLD) { + // An object is expected. Make best effort to find its type and push that. If that + // attempt fails, push a pointer to balance the stack. + mono_class_init_internal (klass); + field = mono_class_get_field (klass, token); + + if (field == NULL) { + // FIXME: this may be incorrect. Either find the correct type to push or skip the rest of BB. + EMIT_NEW_PCONST (cfg, *sp, NULL); + sp++; + } else { + ftype = mono_field_get_type_internal (field); + goto fld_emit_stage; + } } - break; + + break; } CHECK_CFG_ERROR; } @@ -10329,6 +10345,8 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b int ro_type = ftype->type; if (!addr) addr = mono_static_field_get_addr (vtable, field); + +fld_emit_stage: if (ro_type == MONO_TYPE_VALUETYPE && m_class_is_enumtype (ftype->data.klass)) { ro_type = mono_class_enum_basetype_internal (ftype->data.klass)->type; } From 025ed7aaf6c3263b896df4d6eb5b69db29656f77 Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Fri, 8 Sep 2023 13:52:33 +0200 Subject: [PATCH 16/21] Fixing uninitialized ins. --- src/mono/mono/mini/method-to-ir.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index 0ec814dd378e2..943b53a9f5184 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -10419,8 +10419,11 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b } if (!is_const) { - MonoInst *load; + // This can happen in case of type load error. + if (!ins) + EMIT_NEW_PCONST (cfg, ins, 0); + MonoInst *load; EMIT_NEW_LOAD_MEMBASE_TYPE (cfg, load, field->type, ins->dreg, 0); load->flags |= ins_flag; *sp++ = load; From 4633336efad3a3540494720b5a786c2e904b4350 Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Fri, 8 Sep 2023 14:01:27 +0200 Subject: [PATCH 17/21] Fixing ro_type. --- src/mono/mono/mini/method-to-ir.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index 943b53a9f5184..9e4bcb251986b 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -9894,6 +9894,8 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b MonoType *ftype; MonoInst *store_val = NULL; MonoInst *thread_ins; + int ro_type; + gboolean is_typeload_failure = FALSE; is_instance = (il_op == MONO_CEE_LDFLD || il_op == MONO_CEE_LDFLDA || il_op == MONO_CEE_STFLD); if (is_instance) { @@ -9923,6 +9925,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b field = mono_field_from_token_checked (image, token, &klass, generic_context, cfg->error); if (!field || CLASS_HAS_FAILURE (klass)) { HANDLE_TYPELOAD_ERROR (cfg, klass); + is_typeload_failure = TRUE; // Reached only in AOT. Cannot turn a token into a class. We silence the compilation error // and generate a runtime exception. @@ -10333,7 +10336,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b } else { gboolean is_const = FALSE; MonoVTable *vtable = NULL; - + addr = NULL; if (!context_used) { vtable = mono_class_vtable_checked (klass, cfg->error); @@ -10342,15 +10345,19 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b } if ((ftype->attrs & FIELD_ATTRIBUTE_INIT_ONLY) && (((addr = mono_aot_readonly_field_override (field)) != NULL) || (!context_used && !cfg->compile_aot && vtable->initialized))) { - int ro_type = ftype->type; if (!addr) addr = mono_static_field_get_addr (vtable, field); fld_emit_stage: + ro_type = ftype->type; if (ro_type == MONO_TYPE_VALUETYPE && m_class_is_enumtype (ftype->data.klass)) { ro_type = mono_class_enum_basetype_internal (ftype->data.klass)->type; } + gint64 dummy_zero = 0; + if (is_typeload_failure && !addr) + addr = &dummy_zero; + GSHAREDVT_FAILURE (il_op); /* printf ("RO-FIELD %s.%s:%s\n", klass->name_space, klass->name, mono_field_get_name (field));*/ From 55db3c6490937fa308388dff1e68f040b2b30fde Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Fri, 8 Sep 2023 14:33:25 +0200 Subject: [PATCH 18/21] Initializing ins. --- src/mono/mono/mini/method-to-ir.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index 9e4bcb251986b..7f01a4f19e3f3 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -9896,6 +9896,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b MonoInst *thread_ins; int ro_type; gboolean is_typeload_failure = FALSE; + ins = NULL; is_instance = (il_op == MONO_CEE_LDFLD || il_op == MONO_CEE_LDFLDA || il_op == MONO_CEE_STFLD); if (is_instance) { From 4b35af0f4ff9ccbe4f01aab2f61f3673272765fd Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Wed, 20 Sep 2023 13:26:48 +0200 Subject: [PATCH 19/21] Complex cases with type load failures replace method body with a throw. --- src/mono/mono/mini/method-to-ir.c | 62 ++++++++++++++++++++++++------- 1 file changed, 48 insertions(+), 14 deletions(-) diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index 7f01a4f19e3f3..c5ec60b12d4cc 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -6193,6 +6193,49 @@ emit_llvmonly_interp_entry (MonoCompile *cfg, MonoMethodHeader *header) link_bblock (cfg, cfg->cbb, cfg->bb_exit); } +static void +method_make_alwaysthrow_typeloadfailure (MonoCompile* cfg, MonoClass* klass) +{ + // Get rid of all out-BBs from the entry BB. (all but init BB) + for (gint16 i = cfg->bb_entry->out_count - 1; i >= 0; i--) { + if (cfg->bb_entry->out_bb [i] != cfg->bb_init) { + mono_unlink_bblock (cfg, cfg->bb_entry, cfg->bb_entry->out_bb [i]); + mono_remove_bblock (cfg, cfg->bb_entry->out_bb [i]); + } + } + + // Discard all out-BBs from the init BB. + for (gint16 i = cfg->bb_init->out_count - 1; i >= 0; i--) { + if (cfg->bb_init->out_bb [i] != cfg->bb_exit) { + mono_unlink_bblock (cfg, cfg->bb_init, cfg->bb_init->out_bb [i]); + mono_remove_bblock (cfg, cfg->bb_init->out_bb [i]); + } + } + + // Maintain linked list consistency. This BB should have been added as the last, + // ignoring the ones that held actual method code. + cfg->cbb = cfg->bb_init; + + // Create a new BB that only throws, link it after the entry. + MonoBasicBlock* bb; + NEW_BBLOCK (cfg, bb); + bb->cil_code = NULL; + bb->cil_length = 0; + cfg->cbb->next_bb = bb; + cfg->cbb = bb; + + emit_type_load_failure (cfg, klass); + MonoInst* ins; + MONO_INST_NEW (cfg, ins, OP_NOT_REACHED); + MONO_ADD_INS (cfg->cbb, ins); + + ADD_BBLOCK (cfg, bb); + mono_link_bblock (cfg, cfg->bb_init, bb); + mono_link_bblock (cfg, bb, cfg->bb_exit); + + cfg->disable_inline = TRUE; +} + typedef union _MonoOpcodeParameter { gint32 i32; gint64 i64; @@ -9939,19 +9982,10 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b EMIT_NEW_PCONST (cfg, *sp, NULL); sp++; } else if (il_op == MONO_CEE_LDFLD || il_op == MONO_CEE_LDSFLD) { - // An object is expected. Make best effort to find its type and push that. If that - // attempt fails, push a pointer to balance the stack. - mono_class_init_internal (klass); - field = mono_class_get_field (klass, token); - - if (field == NULL) { - // FIXME: this may be incorrect. Either find the correct type to push or skip the rest of BB. - EMIT_NEW_PCONST (cfg, *sp, NULL); - sp++; - } else { - ftype = mono_field_get_type_internal (field); - goto fld_emit_stage; - } + // An object is expected here. It may be impossible to correctly infer its type, + // we turn this entire method into a throw. + method_make_alwaysthrow_typeloadfailure (cfg, klass); + goto all_bbs_done; } break; @@ -10349,7 +10383,6 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b if (!addr) addr = mono_static_field_get_addr (vtable, field); -fld_emit_stage: ro_type = ftype->type; if (ro_type == MONO_TYPE_VALUETYPE && m_class_is_enumtype (ftype->data.klass)) { ro_type = mono_class_enum_basetype_internal (ftype->data.klass)->type; @@ -12079,6 +12112,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b } if (start_new_bblock != 1) UNVERIFIED; +all_bbs_done: cfg->cbb->cil_length = GPTRDIFF_TO_INT32 (ip - cfg->cbb->cil_code); if (cfg->cbb->next_bb) { From ca7530d666c419867306a1fc12261beee0ce8a47 Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Thu, 21 Sep 2023 12:26:16 +0200 Subject: [PATCH 20/21] Cleaning up superfluous code changes. --- src/mono/mono/metadata/class.c | 4 ++-- src/mono/mono/mini/method-to-ir.c | 12 ++---------- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index 94eff9802c924..284e3153a1a25 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -2392,8 +2392,8 @@ static MonoClassField * mono_class_get_field_idx (MonoClass *klass, int idx) { mono_class_setup_fields (klass); - //if (mono_class_has_failure (klass)) - //return NULL; + if (mono_class_has_failure (klass)) + return NULL; while (klass) { int first_field_idx = mono_class_get_first_field_idx (klass); diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index c5ec60b12d4cc..14df6c10ee0a3 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -9937,8 +9937,6 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b MonoType *ftype; MonoInst *store_val = NULL; MonoInst *thread_ins; - int ro_type; - gboolean is_typeload_failure = FALSE; ins = NULL; is_instance = (il_op == MONO_CEE_LDFLD || il_op == MONO_CEE_LDFLDA || il_op == MONO_CEE_STFLD); @@ -9969,7 +9967,6 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b field = mono_field_from_token_checked (image, token, &klass, generic_context, cfg->error); if (!field || CLASS_HAS_FAILURE (klass)) { HANDLE_TYPELOAD_ERROR (cfg, klass); - is_typeload_failure = TRUE; // Reached only in AOT. Cannot turn a token into a class. We silence the compilation error // and generate a runtime exception. @@ -10371,7 +10368,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b } else { gboolean is_const = FALSE; MonoVTable *vtable = NULL; - + addr = NULL; if (!context_used) { vtable = mono_class_vtable_checked (klass, cfg->error); @@ -10380,18 +10377,13 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b } if ((ftype->attrs & FIELD_ATTRIBUTE_INIT_ONLY) && (((addr = mono_aot_readonly_field_override (field)) != NULL) || (!context_used && !cfg->compile_aot && vtable->initialized))) { + int ro_type = ftype->type; if (!addr) addr = mono_static_field_get_addr (vtable, field); - - ro_type = ftype->type; if (ro_type == MONO_TYPE_VALUETYPE && m_class_is_enumtype (ftype->data.klass)) { ro_type = mono_class_enum_basetype_internal (ftype->data.klass)->type; } - gint64 dummy_zero = 0; - if (is_typeload_failure && !addr) - addr = &dummy_zero; - GSHAREDVT_FAILURE (il_op); /* printf ("RO-FIELD %s.%s:%s\n", klass->name_space, klass->name, mono_field_get_name (field));*/ From 9709a897891a7636555dadcdc790dc9ffb81e13a Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Thu, 21 Sep 2023 12:31:49 +0200 Subject: [PATCH 21/21] Restored sizeof cosntant on failed types. --- src/mono/mono/mini/method-to-ir.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index 14df6c10ee0a3..346cea073533f 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -12048,8 +12048,8 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b klass = mini_get_class (method, token, generic_context); if (CLASS_HAS_FAILURE (klass)) { HANDLE_TYPELOAD_ERROR (cfg, klass); - //EMIT_NEW_ICONST(cfg, ins, 0); - //*sp++ = ins; + EMIT_NEW_ICONST(cfg, ins, 0); + *sp++ = ins; break; }