[WIP] Use CodeVersioning for EnC#130159
Conversation
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
There was a problem hiding this comment.
Pull request overview
This PR extends the existing code-versioning infrastructure so Edit-and-Continue (EnC) edits are represented as explicit IL code versions (distinct from ReJIT), and updates DAC/cDAC and debugger plumbing to treat only true ReJIT versions as ReJIT.
Changes:
- Adds a “source” discriminator for IL code versions (ReJIT vs EnC) plus an EnC version counter, and threads this through CoreCLR, data descriptors, and cDAC contracts.
- Updates ReJIT enumeration / DAC-facing ReJIT APIs to exclude EnC code versions (and excludes the synthetic default version from ReJIT ID enumeration).
- Adjusts IL retrieval paths in legacy cDAC/DAC DBI paths to read IL from the active EnC code version where appropriate; updates/extends unit tests and contract docs.
Show a summary per file
| File | Description |
|---|---|
| src/native/managed/cdac/tests/UnitTests/ReJITTests.cs | Updates expected ReJIT ID enumeration behavior and mocks ICodeVersions.IsReJIT. |
| src/native/managed/cdac/tests/UnitTests/MockDescriptors/MockDescriptors.CodeVersions.cs | Extends mock IL code version node layout with Source/EnCVersion; updates builder helpers. |
| src/native/managed/cdac/tests/UnitTests/CodeVersionsTests.cs | Adds coverage asserting EnC/default versions are excluded from ReJIT detection and enumeration. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs | Filters ReJIT-related legacy SOS DAC behaviors using ICodeVersions.IsReJIT. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs | Fetches IL from active EnC code version when available; filters IL code version node APIs to ReJIT only. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs | Uses active EnC IL for local signature parsing when applicable. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/ILCodeVersionNode.cs | Adds Source and EnCVersion fields to the ILCodeVersionNode contract data type. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/CodeVersions_1.cs | Implements ICodeVersions.IsReJIT based on the new ILCodeVersionNode.Source. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ICodeVersions.cs | Introduces public CodeVersionSource and new ICodeVersions.IsReJIT API. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/Extensions/IReJITExtensions.cs | Filters ReJIT ID enumeration to true ReJIT versions via cv.IsReJIT. |
| src/coreclr/vm/rejit.cpp | Tags configured ReJIT IL code versions with CodeVersionSource::kReJIT; filters ReJIT ID queries. |
| src/coreclr/vm/method.hpp | Makes EnC IL methods versionable via CodeVersionManager; enforces EnC/ReJIT mutual exclusion. |
| src/coreclr/vm/method.cpp | Returns active EnC IL from CodeVersionManager in MethodDesc::GetILHeader. |
| src/coreclr/vm/encee.cpp | Creates/activates EnC IL code versions via CodeVersionManager; records source and EnC version. |
| src/coreclr/vm/datadescriptor/datadescriptor.inc | Adds ILCodeVersionNode.Source and ILCodeVersionNode.EnCVersion to the CDAC descriptor. |
| src/coreclr/vm/codeversion.h | Adds CodeVersionSource enum and stores Source/EnCVersion on ILCodeVersionNode. |
| src/coreclr/vm/codeversion.cpp | Implements ILCodeVersion::IsReJIT / GetEnCVersion, plus node accessors/mutators. |
| src/coreclr/debug/ee/functioninfo.cpp | Treats only ReJIT (not EnC) explicit versions as “rejitted” for boundary mapping behavior. |
| src/coreclr/debug/ee/debugger.cpp | Updates debugger ReJIT-related checks to ignore EnC versions; tags created versions as ReJIT. |
| src/coreclr/debug/daccess/request.cpp | Filters DAC ReJIT data paths to only ReJIT IL code versions. |
| src/coreclr/debug/daccess/dacdbiimpl.cpp | Uses CodeVersionManager to resolve EnC versions and EnC IL in DBI helper paths. |
| docs/design/datacontracts/CodeVersions.md | Documents the new CodeVersionSource/IsReJIT concepts (but needs an EnCVersion row added). |
Copilot's findings
- Files reviewed: 22/22 changed files
- Comments generated: 4
| public MockILCodeVersionNode AddILCodeVersionNode(ulong versionId, uint rejitFlags, bool deoptimized = false, uint source = 1 /* CodeVersionSource.ReJIT */, ulong encVersion = 0) | ||
| { |
There was a problem hiding this comment.
Copilot's findings
Comments suppressed due to low confidence (1)
src/coreclr/debug/daccess/request.cpp:4693
- GetPendingReJITID now rejects the active ILCodeVersion unless ilVersion.IsReJIT() is true. However newly-requested ReJIT versions start in kStateRequested with CodeVersionSource still kUnknown (it isn't set until later in ConfigureILCodeVersion), so pending ReJIT queries will incorrectly return E_INVALIDARG. Reorder the checks to treat kStateRequested as a ReJIT-pending version even before Source is populated, and only use IsReJIT() to reject non-requested non-ReJIT (e.g., EnC) versions.
CodeVersionManager::LockHolder codeVersioningLockHolder;
ILCodeVersion ilVersion = pCodeVersionManager->GetActiveILCodeVersion(pMD);
if (ilVersion.IsNull() || !ilVersion.IsReJIT())
{
hr = E_INVALIDARG;
}
else if (ilVersion.GetRejitState() == RejitFlags::kStateRequested)
{
- Files reviewed: 24/24 changed files
- Comments generated: 3
| public enum CodeVersionSource : uint | ||
| { | ||
| Unknown = 0, | ||
| ReJIT = 1, | ||
| EnC = 2, | ||
| } |
| _codeVersionsAllocator.Allocate((ulong)ILCodeVersioningStateLayout.Size, "ILCodeVersioningState")); | ||
|
|
||
| public MockILCodeVersionNode AddILCodeVersionNode(ulong versionId, uint rejitFlags, bool deoptimized = false, ulong ilAddress = 0) | ||
| public MockILCodeVersionNode AddILCodeVersionNode(ulong versionId, uint rejitFlags, bool deoptimized = false, uint source = 1 /* CodeVersionSource.ReJIT */, ulong encVersion = 0, ulong ilAddress = 0) |
| | ILCodeVersionNode | Source | a `CodeVersionSource` value indicating what produced this version (ReJIT, EnC, or unknown) | | ||
| | ILCodeVersionNode | InstrumentedILMap | Embedded `InstrumentedILOffsetMapping` describing the instrumented IL offset mapping | |
| { | ||
| if (!ilCodeVersionHandle.IsExplicit) | ||
| return false; | ||
| return // node Source is ReJIT |
| Module *pModule = GetModule(); | ||
|
|
||
| #ifdef FEATURE_CODE_VERSIONING | ||
| if (InEnCEnabledModule()) |
There was a problem hiding this comment.
Should both EnC and ReJIT be on the same plan? Either keep using dynamic IL to track last IL for both. Or drop dynamic IL for both and use dynamic IL for reflection emit only.
There was a problem hiding this comment.
ReJIT already uses the code version to store IL. It looks like we had just never captured ReJIT IL here. We probably should
Follow-up on #128338 (comment).
Sourcefield toILCodeVersionNodeto distinguish the sources.