Skip to content

Commit

Permalink
[mono] Include filename in Invalid image error messages
Browse files Browse the repository at this point in the history 
Fixes dotnet/runtime#31649
Fixes dotnet/runtime#31650

**Testing**:
Reproduced Error via
`<Runtime Repo Root>/build.sh /p:RuntimeFlavor=mono`
Removing `[ActiveIssue("dotnet/runtime#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.
  • Loading branch information
@mdh1418
mdh1418 committed Mar 9, 2020
1 parent 66545a2 commit 1e7adea
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 28 deletions.
13 changes: 8 additions & 5 deletions mono/metadata/appdomain.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -1489,6 +1491,7 @@ mono_try_assembly_resolve_handle (MonoAssemblyLoadContext *alc, MonoStringHandle
#endif

leave:
g_free (filename);
HANDLE_FUNCTION_RETURN_VAL (ret);
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion mono/metadata/exception.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
9 changes: 6 additions & 3 deletions mono/metadata/icall.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down
12 changes: 7 additions & 5 deletions mono/metadata/metadata.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
Expand Down
28 changes: 14 additions & 14 deletions mono/mini/aot-runtime.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}

Expand All @@ -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);
Expand Down Expand Up @@ -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)));
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 1e7adea

Please sign in to comment.