Skip to content

Conversation

@davidwrighton
Copy link
Member

@davidwrighton davidwrighton commented Sep 10, 2025

  • When the this pointer is used as the generics context, if some code modifies the this pointer, do not allow that modification to affect the generics behavior of the function
  • Implement this by making a shadow copy of the this pointer in this situation
  • To avoid doing this ALL of the time, I've made a scheme where we reserve space for the shadow copy var during normal var processing, but we fill it only if needed, and otherwise bash the shadow copy var to point to its natural location.
  • Also handle the case of a legitimate null this pointer when doing EH type resolution, and have it throw a NullReferenceException.

This is a new PR replacing #119338, since I fouled up a merge in that PR.

davidwrighton and others added 6 commits September 10, 2025 13:38
…h use it as the generics context

- When the this pointer is used as the generics context, if some code modifies the this pointer, do not allow that modification to affect the generics behavior of the function
- Implement this by making a shadow copy of the this pointer in this situation
- To avoid doing this ALL of the time, I've added a scheme where we can restart the entire method compilation by setting a flag which describes the current set of retry rules.
Copilot AI review requested due to automatic review settings September 10, 2025 21:18
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Sep 10, 2025
Copy link
Contributor

Copilot AI left a 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 implements a fix for CLR interpreter's handling of generic methods that use the "this" pointer as the generics context. The change addresses a scenario where modifications to the "this" pointer during method execution could incorrectly affect the generics behavior.

Key Changes:

  • Creates a shadow copy of the "this" pointer when it's used as the generics context in generic methods
  • Implements lazy evaluation - reserves space for the shadow copy but only uses it if the "this" pointer is actually modified
  • Adds null reference checking for the generics context when derived from "this"

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/coreclr/vm/codeman.cpp Adds null reference checking for "this"-derived generics context
src/coreclr/interpreter/compiler.h Adds tracking variables for shadow copy state
src/coreclr/interpreter/compiler.cpp Implements shadow copy logic, variable management, and GC reporting

@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @BrzVlad, @janvorli, @kg
See info in area-owners.md if you want to be subscribed.

Address feedback from @kg
Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@teo-tsirpanis teo-tsirpanis removed the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Sep 11, 2025
@davidwrighton davidwrighton merged commit f7b310c into dotnet:main Sep 11, 2025
100 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Oct 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants