Skip to content

Remove TEB field from Thread, DAC, and cDAC#126902

Merged
max-charlamb merged 2 commits intodotnet:mainfrom
max-charlamb:dev/maxcharlamb/remove-teb-from-getthread
Apr 15, 2026
Merged

Remove TEB field from Thread, DAC, and cDAC#126902
max-charlamb merged 2 commits intodotnet:mainfrom
max-charlamb:dev/maxcharlamb/remove-teb-from-getthread

Conversation

@max-charlamb
Copy link
Copy Markdown
Member

@max-charlamb max-charlamb commented Apr 14, 2026

Note

This PR was generated with the assistance of GitHub Copilot.

Summary

Remove the TEB (Thread Environment Block) pointer from the DAC/cDAC Thread data contract. Consumers should look up the TEB from the OS thread ID via the debugger's native API instead.

Background

The Thread::m_pTEB field was exposed through DacpThreadData.teb and through the cDAC ThreadData.TEB contract field. Analysis of all consumers shows:

  • SOS reads DacpThreadData.teb in one place (strike.cpp:4460) to display COM apartment state in !Threads. A companion PR in dotnet/diagnostics switches this to use IDebuggerServices::GetThreadTeb(osThreadId) instead.
  • ClrMD reads DacpThreadData.Teb to derive StackBase/StackLimit. A companion PR in microsoft/clrmd switches this to use IThreadReader.GetThreadTeb(osThreadId) instead.
  • NativeAOT already returns TargetPointer.Null for TEB.
  • Non-Windows platforms already return NULL for TEB.

Changes

  • request.cpp: Always set threadData->teb = NULL (field retained for binary layout compatibility)
  • datadescriptor.inc: Remove TEB field from Thread type descriptor
  • threads.h: Remove TEB from cdac_data<Thread>
  • Data/Thread.cs: Remove TEB property and reading logic
  • Thread_1.cs: Remove TEB from ThreadData construction
  • IThread.cs: Remove TEB from ThreadData record
  • SOSDacImpl.cs: Set data->teb = 0, remove debug assertion
  • Thread.md: Remove TEB from data contract documentation
  • Test mocks: Remove TEB from mock thread descriptors

Companion PRs

Validation

  • cDAC build: 0 warnings, 0 errors
  • cDAC tests: 1586/1586 passed
  • cdb verification: !Threads output identical between legacy DAC and cDAC (Release)
  • Manual cdb verification: STA/MTA apartment state displays correctly with both old and new SOS

@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

Copy link
Copy Markdown
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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@max-charlamb max-charlamb force-pushed the dev/maxcharlamb/remove-teb-from-getthread branch from aa402c9 to d0e418d Compare April 14, 2026 20:07
Copilot AI review requested due to automatic review settings April 14, 2026 20:07
@max-charlamb max-charlamb marked this pull request as ready for review April 14, 2026 20:08
max-charlamb and others added 2 commits April 14, 2026 16:10
The TEB (Thread Environment Block) pointer was stored in Thread::m_pTEB and
exposed through the DAC via DacpThreadData.teb and through the cDAC via the
Thread data contract. Consumers (SOS, ClrMD) can instead look up the TEB
from the OS thread ID via the debugger's native API, which does not depend
on the runtime carrying this value.

Changes:
- request.cpp: Always set threadData->teb to NULL (field retained for
  binary layout compatibility of DacpThreadData)
- datadescriptor.inc: Remove TEB field from Thread type descriptor
- threads.h: Remove TEB from cdac_data<Thread>
- Data/Thread.cs: Remove TEB property and reading logic
- Thread_1.cs: Remove TEB from ThreadData construction
- IThread.cs: Remove TEB from ThreadData record
- SOSDacImpl.cs: Set data->teb = 0, remove debug assertion
- Thread.md: Remove TEB from data contract documentation
- Test mocks: Remove TEB from mock thread descriptors

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The m_pTEB field cached a pointer to the thread's TEB (_NT_TIB) that was
only used in three places:
- GetTEB() accessor - no longer called by any consumer
- GetExceptionListPtr() - dead code, never called
- Debug-only stack bounds check in RedirectThreadAtHandledJITCase

The debug check now uses GetCachedStackBase/Limit which are already
cached on the Thread object. The cached stack limit (from VirtualQuery)
is slightly more permissive than the TEB's committed stack limit, which
is acceptable for a diagnostic assert.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@max-charlamb max-charlamb force-pushed the dev/maxcharlamb/remove-teb-from-getthread branch from d0e418d to 7a7bfc2 Compare April 14, 2026 20:10
Copy link
Copy Markdown
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thanks!

@max-charlamb max-charlamb changed the title Remove TEB field from Thread data contract and DAC Remove TEB field from Thread, DAC, and cDAC Apr 14, 2026
Copy link
Copy Markdown
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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@hoyosjs
Copy link
Copy Markdown
Member

hoyosjs commented Apr 15, 2026

We don't usually get to do this on cross-OS DAC - usually things like DSO use it for things like alternate stack probing. I see that CLRMD readers preserve the linux 0 return

@max-charlamb max-charlamb merged commit f37ab00 into dotnet:main Apr 15, 2026
123 of 126 checks passed
max-charlamb added a commit to dotnet/diagnostics that referenced this pull request Apr 16, 2026
> [!NOTE]
> This PR was generated with the assistance of GitHub Copilot.

## Summary

Stop reading the TEB from `DacpThreadData.teb` in the `!Threads` command
and instead look it up via
`IDebuggerServices::GetThreadTeb(osThreadId)`. This decouples SOS from a
runtime-internal field that is being removed from the DAC/cDAC Thread
data contract.

## Changes

### strike.cpp
The `!Threads` command reads the COM apartment state (STA/MTA/NTA) by
dereferencing `TEB.ReservedForOle`. Previously it used `Thread.teb` from
`DacpThreadData`; now it calls
`GetDebuggerServices()->GetThreadTeb(Thread.osThreadId)` to get the TEB
address.

### New test: ThreadApartment
Added a new SOS integration test that validates `clrthreads` properly
displays COM apartment state:
- **Debuggee**: Creates one STA thread (`SetApartmentState(STA)`) and
one MTA thread before breaking
- **Script**: Verifies the Apt column contains both STA and MTA values
- **Windows-only**: Guarded in both the test method (`OS.Kind !=
OSKind.Windows`) and the script (`IFDEF:WINDOWS`)

## Verification

Manually verified with cdb against a dump containing STA/MTA threads:
- Original SOS: STA 1, MTA 4
- New SOS: STA 1, MTA 4 (identical output)

## Related PRs

- dotnet/runtime#126902 — Removes TEB from the Thread data contract and
DAC (always returns 0)

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants