-
Notifications
You must be signed in to change notification settings - Fork 5.1k
JIT: fix issue with EH clause class types for fault/filters from C++/CLI #118870
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
Conversation
C++/CLI appears to leave the CORINFO_EH_CLAUSE ClassToken/FilterOffset union set to some nonzero value for fault/filter clauses. The JIT currently just passes this value along to the runtime. If a method with such a nonzero field is inlined into a dynamic method, this trips up a check in the runtime where a nonzero entry for such a field is interpreted as a class handle, even for fault/filter clauses where it should be ignored. Tolerate this by zeroing the field in the JIT. Note this could not have happened in pre .NET10 as methods with EH could not be inlined, so a dynamic method would never see such an EH clause, and in non-dynamic methods this field is ignored for faults and filters. Fixes #118837.
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.
Pull Request Overview
This PR fixes an issue where C++/CLI generated code leaves nonzero values in the ClassToken/FilterOffset union field for fault/filter exception handling clauses. When such methods are inlined into dynamic methods in .NET 10+, the runtime incorrectly interprets these nonzero values as class handles, causing validation failures.
- Adds explicit zeroing of the
ebdTyp
field for finally and fault exception handling clauses - Prevents runtime validation errors when C++/CLI methods with EH are inlined into dynamic methods
- Addresses a new issue introduced in .NET 10 when method inlining with exception handling was enabled
@jakobbotsch PTAL I could (and perhaps should) also fix this in the jit interface (to ignore the value), and/or get the C++/CLI output fixed. We will want to propose this for a .NET 10 backport. Also I haven't added a test case; if anyone can point me at the right way to have a dependent C++/CLI library I will do so (many JIT tests just check in IL instead, which I could also do). |
I assume that it happened here: https://github.com/dotnet/runtime/blob/main/src/coreclr/vm/jitinterface.cpp#L12736-L12738 . We can change the condition to |
And also in |
I am not sure you can convince ilasm to leave bogus value in unused EHInfo fields. If you wanted to deterministically exercise a situation with a bogus value in unused EHInfo fields, I think you would have to emit the binary using System.Reflection.Metadata. I do not think it is it worth it. |
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
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.
Can we always zero out the entirety of compHndBBtab
when we allocate/reallocate it?
/ba-g System.Security.Cryptography.Tests dead-lettered |
Seems prudent. Let me add that. |
Seeing unrelated failures in OSX host activation tests
Opened #118904 |
/backport to release/10.0-rc1 |
Started backporting to release/10.0-rc1: https://github.com/dotnet/runtime/actions/runs/17081234351 |
C++/CLI appears to leave the CORINFO_EH_CLAUSE ClassToken/FilterOffset union set to some nonzero value for fault/filter clauses. The JIT currently just passes this value along to the runtime.
If a method with such a nonzero field is inlined into a dynamic method, this trips up a check in the runtime where a nonzero entry for such a field is interpreted as a class handle, even for fault/filter clauses where it should be ignored.
Tolerate this by zeroing the field in the JIT.
Note this could not have happened in pre .NET10 as methods with EH could not be inlined, so a dynamic method would never see such an EH clause, and in non-dynamic methods this field is ignored for faults and filters.
Fixes #118837.