Use CodeRangeMapRangeList for callcounting stubs and remove unneeded StubManagers#126521
Use CodeRangeMapRangeList for callcounting stubs and remove unneeded StubManagers#126521rcj1 wants to merge 4 commits intodotnet:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates call-counting stubs to be tracked via CodeRangeMapRangeList so the stub kind can be identified directly from the RangeSectionMap (including via cDAC), avoiding iteration over call-counting stub allocators. It also aligns call-counting stub enumeration behavior with other CodeRangeMapRangeList users by no longer explicitly EnumMem-enumerating the stub heap bytes.
Changes:
- Add a new stub kind (
STUB_CODE_BLOCK_CALLCOUNTING) and route RangeSection-based stub identification/tracing toCallCountingStubManager. - Switch call-counting stub allocation tracking from
RangeListtoCodeRangeMapRangeList. - Adjust includes/forward-decls to break prior header dependencies and support the new range list type.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/stubmgr.cpp | Recognizes call-counting stubs via RangeSectionStubManager and forwards tracing/name lookups accordingly. |
| src/coreclr/vm/loaderallocator.hpp | Removes direct include of callcounting.h; adds forward decl/DPTR for CallCountingManager. |
| src/coreclr/vm/common.h | Adds callcounting.h include to preserve transitive visibility of call-counting types. |
| src/coreclr/vm/codeman.h | Introduces STUB_CODE_BLOCK_CALLCOUNTING and string mapping for RangeSection-reported stub kinds. |
| src/coreclr/vm/callcounting.h | Switches allocator tracking to CodeRangeMapRangeList; adjusts visibility for RangeSection forwarding. |
| src/coreclr/vm/callcounting.cpp | Initializes CodeRangeMapRangeList for call-counting stub heaps; removes old stub-range checks and DAC heap range enumeration. |
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
|
Tagging subscribers to this area: @agocke |
| // LoaderHeap cannot be constructed when DACCESS_COMPILE is defined (at the time, its destructor was private). Working | ||
| // around that by controlling creation/destruction using a pointer. | ||
| InterleavedLoaderHeap *m_heap; | ||
| RangeList m_heapRangeList; |
There was a problem hiding this comment.
I am wondering what it would take to get rid of all RangeList uses and switch all of them to CodeRangeMapRangeList. (Outside the scope of this PR.)
There was a problem hiding this comment.
LockedRangeList is the other class that inherits from RangeList and it is used in m_memoryAssociations, cache_entry_rangeList, and indcell_rangeList. Don’t think CodeRangeMapRangeList is really appropriate for, say, m_memoryAssociations as it really has nothing to do with executable code ranges.
There was a problem hiding this comment.
The underlying implementation of CodeRangeMapRangeList is not specific to code. It allows quick mapping of an address to a RangeSection, with page granularity. I think it would be fine to use it instead of RangeList when we need to detect whether given address came from a loader heap or from a mapped image.
rcj1
changed the title
| return JumpStubStubManager::g_pManager->DoTraceStub(stubStartAddress, trace); | ||
| { | ||
| PCODE jumpTarget = decodeBackToBackJump(stubStartAddress); | ||
| trace->InitForStub(jumpTarget); |
There was a problem hiding this comment.
The inline JumpStub tracing no longer logs the resolved destination (JumpStubStubManager previously used LOG_TRACE_DESTINATION). Consider restoring the logging here for consistency with other stub manager trace paths and to keep existing debugging diagnostics intact.
| trace->InitForStub(jumpTarget); | |
| trace->InitForStub(jumpTarget); | |
| LOG_TRACE_DESTINATION("RangeSectionJumpStub", stubStartAddress, trace); |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
In order to figure out if a specific address is a callcounting stub, we currently have to iterate through the call counting stub allocators to figure out which one, if any, owns the address. If we switch call counting stubs to use CodeRangeMapRangeList, the type of the code can easily be looked up from the cDAC in the RangeSectionMap.
Removed EnumMem of the call counting stub heap. We now have the same enumeration as other CodeRangeMapRangeList users. That is, the RangeSection metadata allows lookup of the code type etc, but the actual bytes of the stubs are not explicitly included in a heap dump.
Relevant for
runtime/src/coreclr/debug/daccess/daccess.cpp
Line 5441 in 1d27484