[mono] Include filename in Invalid image error messages (#33117)

* [mono] Include filename in Invalid Image message

* [mono] Rename Invalid Image to File Not Found where applicable

* [Mono] Remove ActiveIssue attributes 31649 and 31650

* Touch up callers of mono_error_set_simple_file_not_found in appdomain.c

* Specify file name in mono_error_set_simple_file_not_found reflection_only message

* Touch up mono_error_set_file_not_found callers in icall.c

* Include image name in mono_error_set_bad_image_by_name calls in metadata.c

* Include aot name in mono_error_set_bad_image_by_name calls in aot-runtime.c

Co-authored-by: Mitchell Hwang <mihw@microsoft.com>
Co-authored-by: Mitchell Hwang <mitchell.hwang@microsoft.com>

diff --git a/src/libraries/System.Reflection/tests/AssemblyTests.cs b/src/libraries/System.Reflection/tests/AssemblyTests.cs
index bf54c57..7d16ead 100644 (file)
--- a/src/libraries/System.Reflection/tests/AssemblyTests.cs
+++ b/src/libraries/System.Reflection/tests/AssemblyTests.cs
@@ -319,7 +319,6 @@ namespace System.Reflection.Tests
         }
 
         [Fact]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/31650", TestRuntimes.Mono)]
         public void LoadFile_NoSuchPath_ThrowsFileNotFoundException()
         {
             string rootedPath = Path.GetFullPath(Guid.NewGuid().ToString("N"));
@@ -348,7 +347,6 @@ namespace System.Reflection.Tests
         }
 
         [Theory]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/31649", TestRuntimes.Mono)]
         [InlineData(0)]
         [InlineData(5)]
         [InlineData(50)]

diff --git a/src/mono/mono/metadata/appdomain.c b/src/mono/mono/metadata/appdomain.c
index 6f06bf0..65354fd 100644 (file)
--- a/src/mono/mono/metadata/appdomain.c
+++ b/src/mono/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/src/mono/mono/metadata/exception.c b/src/mono/mono/metadata/exception.c
index 59712ba..400895d 100644 (file)
--- a/src/mono/mono/metadata/exception.c
+++ b/src/mono/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/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c
index 50c078c..9e20e8a 100644 (file)
--- a/src/mono/mono/metadata/icall.c
+++ b/src/mono/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/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c
index 8768511..ec2da10 100644 (file)
--- a/src/mono/mono/metadata/metadata.c
+++ b/src/mono/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/src/mono/mono/mini/aot-runtime.c b/src/mono/mono/mini/aot-runtime.c
index 4a44ad9..c5e4072 100644 (file)
--- a/src/mono/mono/mini/aot-runtime.c
+++ b/src/mono/mono/mini/aot-runtime.c
@@ -287,7 +287,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;
        }
 
@@ -318,14 +318,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;
        }
@@ -486,7 +486,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;
        }
 
@@ -509,7 +509,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);
@@ -633,7 +633,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)));
@@ -806,7 +806,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;
        }
 
@@ -993,7 +993,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;
@@ -1010,7 +1010,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;
@@ -1044,7 +1044,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;
@@ -1120,7 +1120,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;
@@ -1182,7 +1182,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;
@@ -1412,7 +1412,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 {
@@ -1458,7 +1458,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);