[clr-interp] allow modification of the this pointer in functions which use it as the generics context#119338
Closed
davidwrighton wants to merge 93 commits intodotnet:mainfrom
Closed
[clr-interp] allow modification of the this pointer in functions which use it as the generics context#119338davidwrighton wants to merge 93 commits intodotnet:mainfrom
davidwrighton wants to merge 93 commits intodotnet:mainfrom
Conversation
Member
davidwrighton
commented
Sep 3, 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 added a scheme where we can restart the entire method compilation by setting a flag which describes the current set of retry rules.
…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.
69 tasks
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR implements a retry mechanism in the CLR interpreter to handle cases where the this pointer is modified in generic methods that use it as the generics context. The solution creates a shadow copy of the this pointer to prevent modifications from affecting generic behavior.
Key changes:
- Added a retry compilation loop that restarts method compilation when specific conditions are detected
- Introduced
InterpreterRetryFlagsto control retry behavior with a specific flag for this pointer mitigation - Modified variable creation and instruction emission to detect when the
thispointer might be modified and enable the shadow copy mechanism
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/coreclr/interpreter/eeinterp.cpp |
Added retry compilation loop with flag-based restart mechanism |
src/coreclr/interpreter/compiler.h |
Added InterpreterRetryFlags enum and updated constructor signature |
src/coreclr/interpreter/compiler.cpp |
Implemented shadow copy logic, retry detection, and variable management for this pointer mitigation |
…oading of assemblies cached by TypeDescriptor (dotnet#114619) * Use separate hashtables for collectible types to support assembly unloadability for the TypeDescriptor class * Addressed codereview feedback * Added a test for custom provider and updated WeakHashTable to use ConditionalWeakTable to ensure values can be also colelcted * Renamed ContextAwareHashtable to CollectibleKeyHashtable
…on the context param (dotnet#119336) - Add support to the InterpreterCodeManager for the ability to understand the generic context of a given frame - This is done by adding implementation for InterpreterCodeManager::GetInstance, InterpreterCodeManager::GetParamTypeArg and InterpreterCodeManager::GetParamContextType - In addition, provide an implementation of InterpreterJitManager::ResolveEHClause. The enhancement with this is that it will support doing shared generic catch of a generically dependent exception type. (The JIT implements this by translating these sorts of catches into filters, but that doesn't seem like a good approach for the interpreter. The existing function is noted as being sensitive to stack overflow issues, so I didn't update this for all scenarios) As a bonus, this work also enables !clrstack -i -a to understand the actual accurate generic types of all of the locals.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…dotnet#118778) Today physical promotion will create `FIELD_LIST` for returned struct values. However, arguments did not get the same treatment and had constraints motivated by old limitations of the backend. Since those limitations are gone now we can just indiscriminately produce `FIELD_LIST` in physical promotion.
Bumps [actions/setup-dotnet](https://github.com/actions/setup-dotnet) from 4 to 5. - [Release notes](https://github.com/actions/setup-dotnet/releases) - [Commits](actions/setup-dotnet@v4...v5) --- updated-dependencies: - dependency-name: actions/setup-dotnet dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Fix ProfileEnter when called on a foreign thread When ProfileEnter is called on a foreign thread that the runtime has not seen yet, which can happen for UnmanagedCallersOnly marked methods, it crashes in the GCX_COOP_THREAD_EXISTS(GET_THREAD()); That was left in by accident in my change I've made a long time ago and should not be there. This change removes that switch, we switch to cooperative mode later in the ProfileEnter and setup the thread for runtime before that. Close dotnet#115617 * Make ProfileLeave look the same
Porting feedback from dotnet#119195 to main.
* fix bug where waiting threads were not iterated
…otnet#119334) Windows builds now enable /guard:cf. Also pick up a risc-v enhancement.
* Limit doublemapper maximum size The double mapping currently sets the maximum size of the double mapped memory to 2TB. However, that causes the runtime to fail to start if there is a rlimit for max file size set to a smaller value. This change fixed the problem by clipping the max double mapped memory size by that rlimit and also uses much more conservative size even without the file size limit set. It is now set to the total amount of physical memory by default and also clipped by the virtual memory rlimit. Close dotnet#117819 * In case of VM limit, use just 1/4 of the space * Change physicalMemorySize to 64 bits Even on 32 bit targets, more than 4GB memory may be available.
…cCatalyst (dotnet#119177) * [clr-interp] Setup fully interpreted functional test on Apple mobile * Run tests in runtime.yml pipeline * Run arm64 architecture only * Fix build * Remove tvOS platform * Test linker option * Fix compiler flag * Link interpreter into coreclr library * Fix wasm build * Test maccatalyst build * Revery no-overriding-t-option flag * Test maccatalyst build * Revert no-overriding-t-option flag * Test maccatalyst build * Add SystemNative_Log and SystemNative_LogError declarations * Remove tvos_arm64 platform * Update APPLE_RUNTIME_IDENTIFIER to use placeholder * Update maccatalyst build args * Use checked configuration * Remove iOS device build from runtime pipeline * Fix interpreter build exports * Fix failing build * Fix R2R condition * Test debug config * Test checked build on MacCatalyst and simulator * Test debug config * Test checked configuration * Fix comment * Remove dependency on dn-containers * Define PAL_JitWriteProtect macro for platforms that don't allow JIT write protection * Update install_clr destination to sharedFramework * Fix build
* Backflow from https://github.com/dotnet/dotnet / 3e92251 build 281912 [[ commit created by automation ]] * Update dependencies from https://github.com/dotnet/dotnet build 281912 Updated Dependencies: Microsoft.CodeAnalysis, Microsoft.CodeAnalysis.Analyzers, Microsoft.CodeAnalysis.CSharp, Microsoft.Net.Compilers.Toolset (Version 5.0.0-2.25420.121 -> 5.0.0-2.25453.110) Microsoft.CodeAnalysis.NetAnalyzers, Microsoft.DotNet.ApiCompat.Task, Microsoft.NET.Workload.Emscripten.Current.Manifest-10.0.100.Transport (Version 10.0.100-rc.1.25420.121 -> 10.0.100-rc.1.25453.110) Microsoft.DotNet.Arcade.Sdk, Microsoft.DotNet.Build.Tasks.Archives, Microsoft.DotNet.Build.Tasks.Feed, Microsoft.DotNet.Build.Tasks.Installers, Microsoft.DotNet.Build.Tasks.Packaging, Microsoft.DotNet.Build.Tasks.TargetFramework, Microsoft.DotNet.Build.Tasks.Templating, Microsoft.DotNet.Build.Tasks.Workloads, Microsoft.DotNet.CodeAnalysis, Microsoft.DotNet.GenAPI, Microsoft.DotNet.GenFacades, Microsoft.DotNet.Helix.Sdk, Microsoft.DotNet.PackageTesting, Microsoft.DotNet.RemoteExecutor, Microsoft.DotNet.SharedFramework.Sdk, Microsoft.DotNet.XliffTasks, Microsoft.DotNet.XUnitExtensions (Version 10.0.0-beta.25420.121 -> 11.0.0-beta.25453.110) Microsoft.DotNet.Cecil (Version 0.11.5-alpha.25420.121 -> 0.11.5-alpha.25453.110) Microsoft.DotNet.XUnitAssert, Microsoft.DotNet.XUnitConsoleRunner (Version 2.9.3-beta.25420.121 -> 2.9.3-beta.25453.110) Microsoft.NET.Sdk.IL, Microsoft.NETCore.App.Ref, Microsoft.NETCore.ILAsm, runtime.native.System.IO.Ports, System.Reflection.Metadata, System.Reflection.MetadataLoadContext, System.Text.Json (Version 10.0.0-rc.1.25420.121 -> 10.0.0-rc.1.25453.110) NuGet.Frameworks, NuGet.Packaging, NuGet.ProjectModel, NuGet.Versioning (Version 7.0.0-preview.1.42121 -> 7.0.0-preview.1.45410) System.CommandLine (Version 2.0.0-rc.1.25420.121 -> 2.0.0-rc.1.25453.110) * Remove duplicate definitions from emitriscv64 * Make tests clear out DOTNET_ROOT_<arch> when clearing DOTNET_ROOT (cherry picked from commit 503100d) (cherry picked from commit d201857) * Fix environment variable casing (cherry picked from commit db0d37a) (cherry picked from commit d3ff91a) --------- Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Tanner Gooding <tagoo@outlook.com> Co-authored-by: Elinor Fung <elfung@microsoft.com>
* Re-enable some disabled crossgen2 tests that now pass * Broaden exclusion for DevDiv_255294 since it is not an ARM specific failure
…rkaround from Wasm.Build.Tests (dotnet#119494)
…119544) This prints "ERROR: The process "corerun.exe" not found" under normal circumstances that makes it look like an actual test failure. Fixes dotnet#119525
Member
Author
|
Ugh, something went wrong with a merge locally.... this needs to be cleaned up, as it shouldn't be doing this much in the change. |
This was referenced Sep 10, 2025
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.