From 1e7adea046507c400bba3da1e5489bc18c3f08a2 Mon Sep 17 00:00:00 2001 From: mdh1418 Date: Mon, 9 Mar 2020 17:19:40 +0000 Subject: [PATCH] [mono] Include filename in Invalid image error messages Fixes dotnet/runtime#31649 Fixes dotnet/runtime#31650 **Testing**: Reproduced Error via `/build.sh /p:RuntimeFlavor=mono` Removing `[ActiveIssue("https://github.com/dotnet/runtime/issues/31650", TestRuntimes.Mono)]` in AssemblyTests.cs Running `dotnet msbuild /t:RebuildAndTest /p:XunitMethodName=System.Reflection.Tests.AssemblyTests.LoadFile_NoSuchPath_ThrowsFileNotFoundException` Fixing issue resulted in no errors. --- mono/metadata/appdomain.c | 13 ++++++++----- mono/metadata/exception.c | 2 +- mono/metadata/icall.c | 9 ++++++--- mono/metadata/metadata.c | 12 +++++++----- mono/mini/aot-runtime.c | 28 ++++++++++++++-------------- 5 files changed, 36 insertions(+), 28 deletions(-) diff --git a/mono/metadata/appdomain.c b/mono/metadata/appdomain.c index 6f06bf02e1a7..65354fd96984 100644 --- a/mono/metadata/appdomain.c +++ b/mono/metadata/appdomain.c @@ -1415,6 +1415,7 @@ mono_try_assembly_resolve_handle (MonoAssemblyLoadContext *alc, MonoStringHandle HANDLE_FUNCTION_ENTER (); MonoAssembly *ret = NULL; MonoDomain *domain = mono_alc_domain (alc); + char *filename = NULL; if (mono_runtime_get_no_exec ()) goto leave; @@ -1454,7 +1455,8 @@ mono_try_assembly_resolve_handle (MonoAssemblyLoadContext *alc, MonoStringHandle if (ret && !refonly && mono_asmctx_get_kind (&ret->context) == MONO_ASMCTX_REFONLY) { /* .NET Framework throws System.IO.FileNotFoundException in this case */ - mono_error_set_file_not_found (error, NULL, "AssemblyResolveEvent handlers cannot return Assemblies loaded for reflection only"); + filename = mono_string_handle_to_utf8 (fname, error); + mono_error_set_file_not_found (error, filename, "AssemblyResolveEvent handlers cannot return Assemblies loaded for reflection only: %s", filename); ret = NULL; goto leave; } @@ -1489,6 +1491,7 @@ mono_try_assembly_resolve_handle (MonoAssemblyLoadContext *alc, MonoStringHandle #endif leave: + g_free (filename); HANDLE_FUNCTION_RETURN_VAL (ret); } @@ -2655,9 +2658,9 @@ ves_icall_System_Reflection_Assembly_LoadFrom (MonoStringHandle fname, MonoBoole if (!ass) { if (status == MONO_IMAGE_IMAGE_INVALID) - mono_error_set_bad_image_by_name (error, name, "Invalid Image"); + mono_error_set_bad_image_by_name (error, name, "Invalid Image: %s", name); else - mono_error_set_file_not_found (error, name, "Invalid Image"); + mono_error_set_simple_file_not_found (error, name, refOnly); goto leave; } @@ -2696,9 +2699,9 @@ mono_alc_load_file (MonoAssemblyLoadContext *alc, MonoStringHandle fname, MonoAs ass = mono_assembly_request_open (filename, &req, &status); if (!ass) { if (status == MONO_IMAGE_IMAGE_INVALID) - mono_error_set_bad_image_by_name (error, filename, "Invalid Image"); + mono_error_set_bad_image_by_name (error, filename, "Invalid Image: %s", filename); else - mono_error_set_file_not_found (error, filename, "Invalid Image"); + mono_error_set_simple_file_not_found (error, filename, asmctx == MONO_ASMCTX_REFONLY); } leave: diff --git a/mono/metadata/exception.c b/mono/metadata/exception.c index 59712ba9a609..400895d89094 100644 --- a/mono/metadata/exception.c +++ b/mono/metadata/exception.c @@ -1486,7 +1486,7 @@ void mono_error_set_simple_file_not_found (MonoError *error, const char *file_name, gboolean refection_only) { if (refection_only) - mono_error_set_file_not_found (error, file_name, "Cannot resolve dependency to assembly because it has not been preloaded. When using the ReflectionOnly APIs, dependent assemblies must be pre-loaded or loaded on demand through the ReflectionOnlyAssemblyResolve event."); + mono_error_set_file_not_found (error, file_name, "Cannot resolve dependency to assembly '%s' because it has not been preloaded. When using the ReflectionOnly APIs, dependent assemblies must be pre-loaded or loaded on demand through the ReflectionOnlyAssemblyResolve event.", file_name); else mono_error_set_file_not_found (error, file_name, "Could not load file or assembly '%s' or one of its dependencies.", file_name); } diff --git a/mono/metadata/icall.c b/mono/metadata/icall.c index 50c078c1cf85..9e20e8a1cc4b 100644 --- a/mono/metadata/icall.c +++ b/mono/metadata/icall.c @@ -5728,7 +5728,10 @@ add_file_to_modules_array (MonoDomain *domain, MonoArrayHandle dest, int dest_id goto_if_nok (error, leave); if (!m) { const char *filename = mono_metadata_string_heap (image, cols [MONO_FILE_NAME]); - mono_error_set_file_not_found (error, filename, "%s", ""); + gboolean refonly = FALSE; + if (image->assembly) + refonly = mono_asmctx_get_kind (&image->assembly->context) == MONO_ASMCTX_REFONLY; + mono_error_set_simple_file_not_found (error, filename, refonly); goto leave; } MonoReflectionModuleHandle rm = mono_module_get_object_handle (domain, m, error); @@ -6030,9 +6033,9 @@ ves_icall_System_Reflection_Assembly_InternalGetAssemblyName (MonoStringHandle f if (!image){ if (status == MONO_IMAGE_IMAGE_INVALID) - mono_error_set_bad_image_by_name (error, filename, "Invalid Image"); + mono_error_set_bad_image_by_name (error, filename, "Invalid Image: %s", filename); else - mono_error_set_file_not_found (error, filename, "%s", ""); + mono_error_set_simple_file_not_found (error, filename, FALSE); g_free (filename); return; } diff --git a/mono/metadata/metadata.c b/mono/metadata/metadata.c index 87685111e8ef..ec2da10faff6 100644 --- a/mono/metadata/metadata.c +++ b/mono/metadata/metadata.c @@ -1058,7 +1058,8 @@ const char * mono_metadata_string_heap_checked (MonoImage *meta, guint32 index, MonoError *error) { if (G_UNLIKELY (!(index < meta->heap_strings.size))) { - mono_error_set_bad_image_by_name (error, meta->name ? meta->name : "unknown image", "string heap index %ud out bounds %u", index, meta->heap_strings.size); + const char *image_name = meta && meta->name ? meta->name : "unknown image"; + mono_error_set_bad_image_by_name (error, image_name, "string heap index %ud out bounds %u: %s", index, meta->heap_strings.size, image_name); return NULL; } return meta->heap_strings.data + index; @@ -1127,7 +1128,8 @@ mono_metadata_blob_heap_checked (MonoImage *meta, guint32 index, MonoError *erro if (G_UNLIKELY (index == 0 && meta->heap_blob.size == 0)) return NULL; if (G_UNLIKELY (!(index < meta->heap_blob.size))) { - mono_error_set_bad_image_by_name (error, meta->name ? meta->name : "unknown image", "blob heap index %u out of bounds %u", index, meta->heap_blob.size); + const char *image_name = meta && meta->name ? meta->name : "unknown image"; + mono_error_set_bad_image_by_name (error, image_name, "blob heap index %u out of bounds %u: %s", index, meta->heap_blob.size, image_name); return NULL; } return meta->heap_blob.data + index; @@ -1216,13 +1218,13 @@ mono_metadata_decode_row_checked (const MonoImage *image, const MonoTableInfo *t const char *image_name = image && image->name ? image->name : "unknown image"; if (G_UNLIKELY (! (idx < t->rows && idx >= 0))) { - mono_error_set_bad_image_by_name (error, image_name, "row index %d out of bounds: %d rows", idx, t->rows); + mono_error_set_bad_image_by_name (error, image_name, "row index %d out of bounds: %d rows: %s", idx, t->rows, image_name); return FALSE; } const char *data = t->base + idx * t->row_size; if (G_UNLIKELY (res_size != count)) { - mono_error_set_bad_image_by_name (error, image_name, "res_size %d != count %d", res_size, count); + mono_error_set_bad_image_by_name (error, image_name, "res_size %d != count %d: %s", res_size, count, image_name); return FALSE; } @@ -1237,7 +1239,7 @@ mono_metadata_decode_row_checked (const MonoImage *image, const MonoTableInfo *t case 4: res [i] = read32 (data); break; default: - mono_error_set_bad_image_by_name (error, image_name, "unexpected table [%d] size %d", i, n); + mono_error_set_bad_image_by_name (error, image_name, "unexpected table [%d] size %d: %s", i, n, image_name); return FALSE; } data += n; diff --git a/mono/mini/aot-runtime.c b/mono/mini/aot-runtime.c index 58e923b34acb..75b97d23ea50 100644 --- a/mono/mini/aot-runtime.c +++ b/mono/mini/aot-runtime.c @@ -281,7 +281,7 @@ load_image (MonoAotModule *amodule, int index, MonoError *error) return amodule->image_table [index]; mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_AOT, "AOT: module %s wants to load image %d: %s", amodule->aot_name, index, amodule->image_names[index].name); if (amodule->out_of_date) { - mono_error_set_bad_image_by_name (error, amodule->aot_name, "Image out of date"); + mono_error_set_bad_image_by_name (error, amodule->aot_name, "Image out of date: %s", amodule->aot_name); return NULL; } @@ -312,14 +312,14 @@ load_image (MonoAotModule *amodule, int index, MonoError *error) } if (!assembly) { mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_AOT, "AOT: module %s is unusable because dependency %s is not found.", amodule->aot_name, amodule->image_names [index].name); - mono_error_set_bad_image_by_name (error, amodule->aot_name, "module is unusable because dependency %s is not found (error %d).\n", amodule->image_names [index].name, status); + mono_error_set_bad_image_by_name (error, amodule->aot_name, "module '%s' is unusable because dependency %s is not found (error %d).\n", amodule->aot_name, amodule->image_names [index].name, status); amodule->out_of_date = TRUE; return NULL; } if (strcmp (assembly->image->guid, amodule->image_guids [index])) { mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_AOT, "AOT: module %s is unusable (GUID of dependent assembly %s doesn't match (expected '%s', got '%s')).", amodule->aot_name, amodule->image_names [index].name, amodule->image_guids [index], assembly->image->guid); - mono_error_set_bad_image_by_name (error, amodule->aot_name, "module is unusable (GUID of dependent assembly %s doesn't match (expected '%s', got '%s')).", amodule->image_names [index].name, amodule->image_guids [index], assembly->image->guid); + mono_error_set_bad_image_by_name (error, amodule->aot_name, "module '%s' is unusable (GUID of dependent assembly %s doesn't match (expected '%s', got '%s')).", amodule->aot_name, amodule->image_names [index].name, amodule->image_guids [index], assembly->image->guid); amodule->out_of_date = TRUE; return NULL; } @@ -480,7 +480,7 @@ decode_klass_ref (MonoAotModule *module, guint8 *buf, guint8 **endbuf, MonoError reftype = decode_value (p, &p); if (reftype == 0) { *endbuf = p; - mono_error_set_bad_image_by_name (error, module->aot_name, "Decoding a null class ref"); + mono_error_set_bad_image_by_name (error, module->aot_name, "Decoding a null class ref: %s", module->aot_name); return NULL; } @@ -503,7 +503,7 @@ decode_klass_ref (MonoAotModule *module, guint8 *buf, guint8 **endbuf, MonoError token = decode_value (p, &p); image = module->assembly->image; if (!image) { - mono_error_set_bad_image_by_name (error, module->aot_name, "No image associated with the aot module"); + mono_error_set_bad_image_by_name (error, module->aot_name, "No image associated with the aot module: %s", module->aot_name); return NULL; } klass = mono_class_get_checked (image, token, error); @@ -627,7 +627,7 @@ decode_klass_ref (MonoAotModule *module, guint8 *buf, guint8 **endbuf, MonoError break; } default: - mono_error_set_bad_image_by_name (error, module->aot_name, "Invalid klass reftype %d", reftype); + mono_error_set_bad_image_by_name (error, module->aot_name, "Invalid klass reftype %d: %s", reftype, module->aot_name); } //g_assert (klass); //printf ("BLA: %s\n", mono_type_full_name (m_class_get_byval_arg (klass))); @@ -800,7 +800,7 @@ decode_type (MonoAotModule *module, guint8 *buf, guint8 **endbuf, MonoError *err break; } default: - mono_error_set_bad_image_by_name (error, module->aot_name, "Invalid encoded type %d", t->type); + mono_error_set_bad_image_by_name (error, module->aot_name, "Invalid encoded type %d: %s", t->type, module->aot_name); goto fail; } @@ -987,7 +987,7 @@ decode_method_ref_with_target (MonoAotModule *module, MethodRef *ref, MonoMethod else if (wrapper_type == MONO_WRAPPER_STFLD) ref->method = mono_marshal_get_stfld_wrapper (type); else { - mono_error_set_bad_image_by_name (error, module->aot_name, "Unknown AOT wrapper type %d", wrapper_type); + mono_error_set_bad_image_by_name (error, module->aot_name, "Unknown AOT wrapper type %d: %s", wrapper_type, module->aot_name); return FALSE; } break; @@ -1004,7 +1004,7 @@ decode_method_ref_with_target (MonoAotModule *module, MethodRef *ref, MonoMethod if (!ref->method) ref->method = mono_gc_get_managed_allocator_by_type (atype, MANAGED_ALLOCATOR_SLOW_PATH); if (!ref->method) { - mono_error_set_bad_image_by_name (error, module->aot_name, "Error: No managed allocator, but we need one for AOT.\nAre you using non-standard GC options?\n"); + mono_error_set_bad_image_by_name (error, module->aot_name, "Error: No managed allocator, but we need one for AOT.\nAre you using non-standard GC options?\n%s\n", module->aot_name); return FALSE; } break; @@ -1038,7 +1038,7 @@ decode_method_ref_with_target (MonoAotModule *module, MethodRef *ref, MonoMethod return FALSE; } } else { - mono_error_set_bad_image_by_name (error, module->aot_name, "Invalid STELEMREF subtype %d", subtype); + mono_error_set_bad_image_by_name (error, module->aot_name, "Invalid STELEMREF subtype %d: %s", subtype, module->aot_name); return FALSE; } break; @@ -1114,7 +1114,7 @@ decode_method_ref_with_target (MonoAotModule *module, MethodRef *ref, MonoMethod guint32 init_type = decode_value (p, &p); ref->method = mono_marshal_get_llvm_func_wrapper ((MonoLLVMFuncWrapperSubtype) init_type); } else { - mono_error_set_bad_image_by_name (error, module->aot_name, "Invalid UNKNOWN wrapper subtype %d", subtype); + mono_error_set_bad_image_by_name (error, module->aot_name, "Invalid UNKNOWN wrapper subtype %d: %s", subtype, module->aot_name); return FALSE; } break; @@ -1176,7 +1176,7 @@ decode_method_ref_with_target (MonoAotModule *module, MethodRef *ref, MonoMethod else if (subtype == WRAPPER_SUBTYPE_ISINST_WITH_CACHE) ref->method = mono_marshal_get_isinst_with_cache (); else { - mono_error_set_bad_image_by_name (error, module->aot_name, "Invalid CASTCLASS wrapper subtype %d", subtype); + mono_error_set_bad_image_by_name (error, module->aot_name, "Invalid CASTCLASS wrapper subtype %d: %s", subtype, module->aot_name); return FALSE; } break; @@ -1406,7 +1406,7 @@ decode_method_ref_with_target (MonoAotModule *module, MethodRef *ref, MonoMethod return_val_if_nok (error, FALSE); break; default: - mono_error_set_bad_image_by_name (error, module->aot_name, "Invalid METHODREF_ARRAY method type %d", method_type); + mono_error_set_bad_image_by_name (error, module->aot_name, "Invalid METHODREF_ARRAY method type %d: %s", method_type, module->aot_name); return FALSE; } } else { @@ -1452,7 +1452,7 @@ decode_resolve_method_ref_with_target (MonoAotModule *module, MonoMethod *target if (ref.method) return ref.method; if (!ref.image) { - mono_error_set_bad_image_by_name (error, module->aot_name, "No image found for methodref with target"); + mono_error_set_bad_image_by_name (error, module->aot_name, "No image found for methodref with target: %s", module->aot_name); return NULL; } return mono_get_method_checked (ref.image, ref.token, NULL, NULL, error);