Type verification in tests #40186
Type verification in tests #40186
Conversation
|
Why do we need to introduce |
| { | ||
| public class FieldFixupSignature : Signature | ||
| { | ||
| public const int MaxCheckableOffset = 0x1FFFFFFF; |
There was a problem hiding this comment.
What was the issue with larger offsets? Are they now unchecked?
There was a problem hiding this comment.
They are now unchecked. The problem is that the encoding uses the compressed uint encoding from metadata which cannot encode a number greater than 0x1FFFFFFF.
In practice, the need to do this is so rare, that it isn't worth changing the encoding.
There was a problem hiding this comment.
Shouldn't we fail compilation for large offsets then? I see throwing a RequiresRuntimeJitException in one if branch, but not in others.
src/coreclr/src/tools/Common/Internal/Runtime/ReadyToRunConstants.cs
Outdated
Show resolved
Hide resolved
…sistent between runtime and crossgen2
All contents of the ReadyToRun file format should be either ECMA 335 constants, or defined as part of the ready to run headers. |
jkoritzinsky
left a comment
jkoritzinsky
left a comment
There was a problem hiding this comment.
fieldmarshaler.cpp changes look good to me!
|
@jkoritzinsky I see some validation here for crossgen2, what about the runtime itself? I would assume the runtime needs a new test or to update one. |
|
I don't believe we have a type layout test in the runtime for this case. The crossgen2 test will catch any unintentional layout changes in this area since crossgen2 has a parallel implementation, so a runtime test isn't as immediately important. It would still be worth adding though I think. |
- Enable type layout verification in crossgen2 test passes Product bugs found/fixed - Fix type blittability issue found during enabling this test. - HFA encoding was not correct for Arm64, and did not have R2R defined constants - Fix type layout verification infrastructure issues - Do not generate type layout checks or verification if the offset is 24 bits or larger - Only load approx enclosing type when verifying, instead of enclosing type (this avoids asserts)
ghost
locked as resolved and limited conversation to collaborators
7 participants
Do not merge until this has passed manual checks against all of the various crossgen2 test pipelines