cDAC: dereference OBJECTHANDLE in collectible thread-statics#129291
Open
leculver wants to merge 4 commits into
Open
cDAC: dereference OBJECTHANDLE in collectible thread-statics#129291leculver wants to merge 4 commits into
leculver wants to merge 4 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates collectible TLS static-base lookup to treat TLS slots as OBJECTHANDLEs and resolve the underlying object rather than treating the slot value as the object pointer.
Changes:
- Replaces direct pointer read from the collectible TLS array with
ObjectHandledereference viaProcessedData. - Adds clarifying comments describing native behavior alignment (
ObjectFromHandleUnchecked).
Show a summary per file
| File | Description |
|---|---|
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Thread_1.cs | Adjusts collectible TLS static base resolution to dereference an OBJECTHANDLE via contract processing. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 2
rcj1
reviewed
Jun 11, 2026
| { | ||
| TargetPointer collectibleArray = threadLocalData.CollectibleTlsArrayData; | ||
| threadLocalStaticBase = _target.ReadPointer(collectibleArray + (ulong)(indexOffset * _target.PointerSize)); | ||
| // The collectible TLS array slot holds an OBJECTHANDLE. Constructing an |
Contributor
There was a problem hiding this comment.
Please delete the comment and update docs/design/datacontracts/Thread.md
…lution The cDAC IThread.GetThreadLocalStaticBase collectible-TLS branch returned the collectible TLS array slot contents directly as the static base. That slot holds an OBJECTHANDLE, not the object: native GetThreadLocalStaticBaseNoCreate (threadstatics.cpp) reads the handle from the slot and then calls ObjectFromHandleUnchecked to get the object. cDAC skipped the dereference, so collectible (unloadable-ALC) thread-static base resolution returned the handle cell address instead of the object base, silently mis-resolving every subsequent field read. Dereference the handle via the ObjectHandle data type (which reads the handle then the object, matching ObjectFromHandleUnchecked and the in-flight TLS path that already uses ObjectHandle.Object). The NonCollectible branch is unchanged: that array stores direct object refs. Flows through ISOSDacInterface14::GetThreadStaticBaseAddress, affecting cDAC-backed SOS thread-static field display and ClrMD ClrThreadStaticField.GetAddress. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rename handleAddress -> handleSlotAddress and reword the comment to make explicit that the value is the address of the TLS array slot containing the OBJECTHANDLE (not the handle/object), and that ObjectHandle reads the handle from that slot and dereferences it. No behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
leculver
force-pushed
the
cDac-threadstatic-fix
branch
from
4314b1a to
67fea10
Compare
Comment on lines
+305
to
+307
| // The collectible TLS array slot holds an OBJECTHANDLE; dereference the handle to the object | ||
| TargetPointer handleSlotAddress = collectibleArray + (ulong)(indexOffset * target.PointerSize); | ||
| threadLocalStaticBase = target.ReadPointer(target.ReadPointer(handleSlotAddress)); |
Comment on lines
+219
to
+220
| TargetPointer handleSlotAddress = collectibleArray + (ulong)(indexOffset * _target.PointerSize); | ||
| threadLocalStaticBase = _target.ProcessedData.GetOrAdd<Data.ObjectHandle>(handleSlotAddress).Object; |
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.
The collectable branch for thread statics currently returns the slot of an object, whereas the NonCollectible branch returns the object itself. Changing this matches the ObjectFromHandleUnchecked in request.cpp (and what clrmd expects on the other end).