Fix debug assertion in FindTightlyBoundUnboxingStub_DEBUG / FindTightlyBoundWrappedMethodDesc_DEBUG#124377
Closed
pavelsavara wants to merge 1 commit intodotnet:mainfrom
Closed
Fix debug assertion in FindTightlyBoundUnboxingStub_DEBUG / FindTightlyBoundWrappedMethodDesc_DEBUG#124377pavelsavara wants to merge 1 commit intodotnet:mainfrom
pavelsavara wants to merge 1 commit intodotnet:mainfrom
Conversation
Contributor
|
Tagging subscribers to this area: @agocke |
529d576 to
7adcc0c
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes debug-only functions in genmeth.cpp that verify the correctness of optimized method descriptor lookup logic for generic methods on value types.
Changes:
- Replaced
MethodTable::MethodIteratorwithMethodTable::IntroducedMethodIteratorin two DEBUG helper functions - Removed incorrect virtual/non-virtual filtering logic that was using
IsVirtual()checks - Simplified iteration from backward (
MoveToEnd()+Prev()) to forward (Next())
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The debug-only verification functions
FindTightlyBoundUnboxingStub_DEBUGandFindTightlyBoundWrappedMethodDesc_DEBUGinsrc/coreclr/vm/genmeth.cppuse a different iteration strategy than their production counterparts, causing them to miss certain MethodDescs and trigger false assertion failures.The production functions (
FindTightlyBoundUnboxingStubandFindTightlyBoundWrappedMethodDesc) use chunk-based iteration viaMethodTable::IntroducedMethodIterator::GetNext(), which physically walks all MethodDescs in the MethodDescChunks owned by the MethodTable. This correctly enumerates every MethodDesc regardless of its slot assignment.The debug functions used slot-based iteration via
MethodTable::MethodIterator, which iterates slot indices 0..GetNumMethods()-1and resolves each slot to a MethodDesc viaGetMethodDescForSlot(). Additionally, each function applied a virtual/non-virtual filter:FindTightlyBoundUnboxingStub_DEBUGfiltered withIsVirtual()— only checked slots in the virtual range (index <GetNumVirtuals())FindTightlyBoundWrappedMethodDesc_DEBUGfiltered with!IsVirtual()— only checked slots in the non-virtual range (index >=GetNumVirtuals())This causes failures because slot-based iteration can miss MethodDescs that are not reachable through the slot table, and the virtual/non-virtual filters further restrict the search to the wrong range.
Root cause
For generic value types, the
MethodTableBuilderallocates tightly-bound unboxing stubs as follows (methodtablebuilder.cpp, around line 7113):For generic value types, the "wrapped" (non-unboxing) copy of each method is placed in a vtable slot (virtual range). But
FindTightlyBoundWrappedMethodDesc_DEBUGonly searched non-virtual slots via its!IsVirtual()filter, so it never found the wrapped method for generic value types and returned NULL.The assertion at the call site then fails:
_ASSERTE(pResultMD == pMDescInCanonMT || pResultMD == FindTightlyBoundWrappedMethodDesc_DEBUG(pMDescInCanonMT));Because the production function correctly found the wrapped method (via chunk walk), but the debug function returned NULL (due to the wrong slot filter).
Why is this latent
The assertion only fires when
FindOrCreateAssociatedMethodDescis called with specific parameters that reach theFindTightlyBoundWrappedMethodDescorFindTightlyBoundUnboxingStubcode paths. These paths are exercised during:getCallInforesolves a virtual method on a generic value type withallowInstParam=TRUE, it reachesFindTightlyBoundWrappedMethodDesc.FindOrCreateAssociatedMethodDescForReflectionresolves a virtual method on a value type withforceBoxedEntryPoint=TRUE, it reachesFindTightlyBoundUnboxingStub.Both paths are commonly exercised during normal execution, but the assertion failure only manifests in Debug builds. In Release builds, the
_ASSERTEmacros are no-ops, so the bug is invisible.The assertion fires when the specific method being resolved has an unboxing stub whose slot assignment falls outside the range searched by the debug function — this is the common case for generic value types.
Fix
Changed both debug functions to use
IntroducedMethodIterator(chunk-based iteration) instead ofMethodIterator(slot-based iteration), matching the approach used by the production functions. Removed theIsVirtual()/!IsVirtual()filters since chunk-based iteration enumerates all MethodDescs directly without relying on slot indices.Before (slot-based, with wrong filter)
After (chunk-based, no filter needed)
TODO Test - #124386
Added
src/tests/Loader/classloader/generics/VSD/Struct_UnboxingStubLookup.cswhich:Processor<T>implementing multiple interfaces (IProcessor<T>,IIdentity) with virtual methods and object overrides.[MethodImpl(NoInlining)]helper methods with various type instantiations (reference types and value types). This triggers JIT'sgetCallInfo→FindOrCreateAssociatedMethodDesc→FindTightlyBoundWrappedMethodDescon the canonical method table.GetInterfaceMapon generic struct instantiations) to triggerFindOrCreateAssociatedMethodDescForReflection→FindTightlyBoundUnboxingStub.In Debug builds without the fix, this test crashes with the assertion failure. With the fix, it passes.