-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[mono] Include filename in Invalid image error messages #32560
[mono] Include filename in Invalid image error messages #32560
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should use mono_error_set_simple_file_not_found
for some of these FileNotFoundExceptions. It seems to already include the file name in its error message and includes a nicer error message.
Additionally, we should make the change to include the filename with the error message in more than just the function calls failing the tests. There are some cases where it doesn't make sense to do so because we have an in-memory assembly (e.g. mono_error_set_bad_image_by_name (error, "In memory assembly", "0x%p", assembly_data);
) but otherwise we should try and include it across the board. For FileNotFound, the main candidates are the two calls in icall.c, but we have a bunch of invalid image calls in metadata.c and aot-runtime.c that seem like they would benefit from this.
Please add exactly one area label if the bot doesn't. It trains the bot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly looks good, just a few little things to fix up.
5357328
to
f15a4a5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So close! Watch out for whitespace changes.
f15a4a5
to
f57ff48
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks!
…)" This reverts commit 5371ef4.
Fixes #31649
Fixes #31650
Testing:
Reproduced Error via
<Runtime Repo Root>/build.sh /p:RuntimeFlavor=mono
Removing
[ActiveIssue("https://github.com/dotnet/runtime/issues/31650", TestRuntimes.Mono)]
in AssemblyTests.csRunning
dotnet msbuild /t:RebuildAndTest /p:XunitMethodName=System.Reflection.Tests.AssemblyTests.LoadFile_NoSuchPath_ThrowsFileNotFoundException
Fixing issue resulted in no errors.