Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make normal statics simpler #99183

Merged
merged 103 commits into from
Jun 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
103 commits
Select commit Hold shift + click to select a range
b57c72b
Phase 1 of modernize our static variable handling
davidwrighton Feb 28, 2024
e47278b
Merge branch 'main' of github.com:dotnet/runtime into make_normal_sta…
davidwrighton Feb 28, 2024
34420e0
Followup commit
davidwrighton Feb 28, 2024
3cb0324
Fix merge conflict
davidwrighton Feb 28, 2024
37e2555
Tweak so R2R works for now. May need further adjustment.
davidwrighton Feb 29, 2024
78720e1
Enable using the dynamic and pinned helpers for jitted code. Should b…
davidwrighton Feb 29, 2024
c375996
Re-enable the helper expansions for statics
davidwrighton Mar 1, 2024
5ec61b2
Attempt to implement for X64 unix. We'll see if I guessed right
davidwrighton Mar 2, 2024
e88cda8
Windows Arm64 working, and further tweaks
davidwrighton Mar 5, 2024
2eb6eb8
Enable X86
davidwrighton Mar 5, 2024
aa741bc
Update X64 and Arm64 Unix helpers to new sequence
davidwrighton Mar 5, 2024
f805eda
Enable new model for Arm32 by removing all special cases for Arm32
davidwrighton Mar 5, 2024
d463f59
Remove DomainLocalBlock and StaticBoxInit crst types
davidwrighton Mar 5, 2024
6aed90a
Fix Tls index setting in R2R code
davidwrighton Mar 6, 2024
1bb643a
Save a little bit of code size on X64
davidwrighton Mar 6, 2024
02ee7a1
Fix build breaks on unix
davidwrighton Mar 6, 2024
e3e8cd6
Fix Unix X64 build break
davidwrighton Mar 6, 2024
92153b0
Merge with change introduced by #97590
davidwrighton Mar 6, 2024
a8f9b73
Fix allocation of thread static boxes
davidwrighton Mar 6, 2024
d37f612
Remove custom assembly helpers for statics access
davidwrighton Mar 6, 2024
0c0545e
Merge branch 'main' of github.com:dotnet/runtime into make_normal_sta…
davidwrighton Mar 6, 2024
6a9a062
Expose the ability to look at simplified statics through a new ISOSDa…
davidwrighton Mar 7, 2024
abe8a97
Fix missing helper, used by collectible scenario
davidwrighton Mar 7, 2024
3380e00
Delete ThreadStaticHandleTable
davidwrighton Mar 7, 2024
785f32f
Fixup collectible assembly static handling to work correctly again
davidwrighton Mar 11, 2024
504d93e
Missed allocation of LoaderAllocatorHandle for the collectible static…
davidwrighton Mar 11, 2024
8afc63e
Implement collectible thread statics
davidwrighton Mar 13, 2024
b8ef938
Handle scenario with a RVA statc field and a class constructor, but n…
davidwrighton Mar 13, 2024
fce7f9f
Fix static field reading for R2R
davidwrighton Mar 14, 2024
8bd309c
Attempt to fixup arm32 behavior
davidwrighton Mar 14, 2024
6cb70fa
Merge branch 'main' of github.com:dotnet/runtime into make_normal_sta…
davidwrighton Mar 14, 2024
7db100e
Merge branch 'main' of github.com:dotnet/runtime into make_normal_sta…
davidwrighton Mar 14, 2024
1595cf0
Continued fallout from my possibly ill considered decision to make th…
davidwrighton Mar 15, 2024
961744d
Fix InFlightTLSData so that values are properly kept for thread stati…
davidwrighton Mar 15, 2024
69de672
Fix oops in statics allocation
davidwrighton Mar 18, 2024
57c3025
Merge branch 'main' of github.com:dotnet/runtime into make_normal_sta…
davidwrighton Mar 19, 2024
94b58ed
Zero out cTLSData when cleaning up thread statics
davidwrighton Mar 19, 2024
170f74c
Fix static field alignment on 32bit platforms
davidwrighton Mar 19, 2024
87ae678
Fix handling of cleared markers for thread statics
davidwrighton Mar 20, 2024
a1b5e17
Merge branch 'main' of github.com:dotnet/runtime into make_normal_sta…
davidwrighton Mar 21, 2024
4543b03
Update comment
davidwrighton Mar 21, 2024
53a68a0
Fix TLS indices to be pointer indexed offsets
davidwrighton Mar 21, 2024
378ecc1
Finish updates that I meant to make in previous commit
davidwrighton Mar 22, 2024
f38ae83
Copy and zero memory correctly
davidwrighton Mar 22, 2024
66cd564
Fix thread static reporting when indices are per pointer offset value
davidwrighton Mar 25, 2024
0e7c1d4
Update R2R version and update comment on thread statics
davidwrighton Mar 25, 2024
446a60e
Add missing semicolon
davidwrighton Mar 25, 2024
b2850ea
Jit formatting round 1
davidwrighton Mar 25, 2024
d526181
Update InitClass helper data
davidwrighton Mar 25, 2024
e2aa42f
Revert "Update R2R version and update comment on thread statics"
davidwrighton Mar 26, 2024
24801db
Update thread statics comments
davidwrighton Mar 26, 2024
662579b
Just bump R2R headers
davidwrighton Mar 26, 2024
43e4e52
Update SOS_BREAKING_CHANGE_VERSION
davidwrighton Mar 28, 2024
37ee3e9
Move fast get hashcode flags near to each other.
davidwrighton Mar 28, 2024
07f7d14
Use a new HandleType for managing interior pointers instead of the pr…
davidwrighton Mar 28, 2024
cd157be
Merge branch 'main' of https://github.com/dotnet/runtime into make_no…
davidwrighton Mar 28, 2024
e8273e6
Fix build break
davidwrighton Mar 29, 2024
4b27a95
Rework Thread static changes to make non-collectible tls data have a …
davidwrighton Apr 18, 2024
f111ecd
Merge branch 'main' of github.com:dotnet/runtime into make_normal_sta…
davidwrighton Apr 18, 2024
f6aaa69
Update JIT interface guid again
davidwrighton Apr 18, 2024
61823a4
Fixed build breaks caused by merge, and code review comments
davidwrighton Apr 18, 2024
99c678a
Merge branch 'main' of github.com:dotnet/runtime into make_normal_sta…
davidwrighton Apr 18, 2024
5914558
Fix comment and version number
davidwrighton Apr 18, 2024
4563623
Fix issues noted in testing
davidwrighton Apr 19, 2024
11c671f
Fix another oops
davidwrighton Apr 19, 2024
bc16c62
Merge commit 'a344abdd6fa1283aba3b18c882fe780447b3bad2' into make_nor…
davidwrighton Apr 19, 2024
ec88375
Extra checks for thread cleanup paths operating in parallel
davidwrighton Apr 22, 2024
0ef9bb1
- Fix incorrect assert that fires in the presence of non-collectible …
davidwrighton Apr 22, 2024
53cd1f6
Add some checks to try to avoid deadlocks on shutdown
davidwrighton Apr 23, 2024
160acba
Merge branch 'main' of github.com:dotnet/runtime into make_normal_sta…
davidwrighton Apr 29, 2024
cacb466
Implement direct access to TLS data on the ThreadLocalData structure
davidwrighton Apr 29, 2024
1053165
Fix missing enum value
davidwrighton Apr 30, 2024
43abbf3
DynamicallyAllocatedThreadLocalData_data
davidwrighton Apr 30, 2024
42fffaa
Now it allocates at the byte level to be a bit more efficient, and re…
davidwrighton Apr 30, 2024
8d9ddba
Merge branch 'main' of github.com:dotnet/runtime into make_normal_sta…
davidwrighton Apr 30, 2024
35c935e
Jit format and GCC build changes
davidwrighton May 1, 2024
d48ef55
Try to fix the musl validation leg
davidwrighton May 1, 2024
638ea50
Attempt to fix even more build nits
davidwrighton May 1, 2024
bcff9aa
Merge branch 'main' of github.com:dotnet/runtime into make_normal_sta…
davidwrighton May 1, 2024
fc67ecd
Fix next set of musl validation errors
davidwrighton May 1, 2024
4e2e0c2
Add statics heap specifically for non-collectible non-gc statics
davidwrighton May 3, 2024
4e78250
Fix naming for thread local static field names to be something which …
davidwrighton May 3, 2024
02b5899
Merge branch 'main' of github.com:dotnet/runtime into make_normal_sta…
davidwrighton May 3, 2024
100c2ad
Delete Statics heap at correct time
davidwrighton May 3, 2024
3888af5
Merge branch 'main' of github.com:dotnet/runtime into make_normal_sta…
davidwrighton May 6, 2024
270e991
Code review updates
davidwrighton May 7, 2024
2db5ffd
Initial pass at a BoTR chapter update.
davidwrighton May 8, 2024
0be084d
Get rid of the complicated GC interaction model for collectible assem…
davidwrighton May 8, 2024
8d7243b
Fix contract violation
davidwrighton May 8, 2024
b08d8eb
Fix markdownlint
davidwrighton May 8, 2024
a94b00c
Merge branch 'main' of https://github.com/dotnet/runtime into make_no…
davidwrighton May 14, 2024
3b94fa0
Adjust to code review feedback
davidwrighton May 14, 2024
e702b91
Merge branch 'main' of https://github.com/dotnet/runtime into make_no…
davidwrighton May 24, 2024
ecd26bc
Code review comments
davidwrighton May 24, 2024
42f3eb9
Merge branch 'main' of github.com:dotnet/runtime into make_normal_sta…
davidwrighton May 25, 2024
a6225c1
Merge branch 'main' of github.com:dotnet/runtime into make_normal_sta…
davidwrighton May 28, 2024
548842e
Merge branch 'main' of github.com:dotnet/runtime into make_normal_sta…
davidwrighton May 31, 2024
e6f47d1
Fix details found while examining diagnostic tests with this change
davidwrighton Jun 3, 2024
973a0f8
t status
davidwrighton Jun 3, 2024
71a4ae9
Get rid of the asserts from the Profiler tls reading path when runnin…
davidwrighton Jun 4, 2024
5afc45a
Code review feedback
davidwrighton Jun 7, 2024
8ca9b7c
Fix test to actually be valid
davidwrighton Jun 12, 2024
07f1739
Merge branch 'main' of github.com:dotnet/runtime into make_normal_sta…
davidwrighton Jun 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 138 additions & 13 deletions docs/design/coreclr/botr/type-system.md
Original file line number Diff line number Diff line change
Expand Up @@ -193,19 +193,143 @@ This is enforced via an extensive and complicated set of enforcements within the
- **ISSUE:** The signature walks performed are done with the normal signature walking code. This code is designed to load types as it walks the signature, but in this case the type load functionality is used with the assumption that no type load will actually be triggered.
- **ISSUE:** Stackwalker requirements require support from not just the type system, but also the assembly loader. The Loader has had a number of issues meeting the needs of the type system here.

Type System and NGEN
--------------------

The type system data structures are a core part of what is saved into NGEN images. Unfortunately, these data structures logically have pointers within them that point to other NGEN images. In order to handle this situation, the type system data structures implement a concept known as restoration.

In restoration, when a type system data structure is first needed, the data structure is fixed up with correct pointers. This is tied into the type loading levels described in the [Type Loader](type-loader.md) Book of the Runtime chapter.

There also exists the concept of pre-restored data structures. This means that the data structure is sufficiently correct at NGEN image load time (after intra-module pointer fixups and eager load type fixups), that the data structure may be used as is. This optimization requires that the NGEN image be "hard bound" to its dependent assemblies. See NGEN documentation for further details.

Type System and Domain Neutral Loading
--------------------------------------

The type system is a core part of the implementation of domain neutral loading. This is exposed to customers through the LoaderOptimization options available at AppDomain creation. Mscorlib is always loaded as domain neutral. The core requirement of this feature is that the type system data structures must not require pointers to domain specific state. Primarily this manifests itself in requirements around static fields and class constructors. In particular, whether or not a class constructor has been run is not a part of the core MethodTable data structure for this reason, and there is a mechanism for storing static data attached to the DomainFile data structure instead of the MethodTable data structure.
## Static variables

Static variables in CoreCLR are handled by a combination of getting the "static base", and then adjusting it by an offset to get a pointer to the actual value.
We define the statics base as either non-gc or gc for each field.
Currently non-gc statics are any statics which are represented by primitive types (byte, sbyte, char, int, uint, long, ulong, float, double, pointers of various forms), and enums.
GC statics are any statics which are represented by classes or by non-primitive valuetypes.
Copy link
Member

Choose a reason for hiding this comment

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

Does "non-primitive" in this case mean non-enums or non-blittable or some other concept? I assume it isn't the same as C# unmanaged.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, it's any valuetype which when examined via GetVerifierCorElementType will return an CorElementType which is not ELEMENT_TYPE_VALUETYPE or a pointer type. Effectively, its defined as the set of valuetypes which are not eligible for being a non-gc static.

For valuetype statics which are GC statics, the static variable is actually a pointer to a boxed instance of the valuetype.

### Per type static variable information
As of .NET 9, the static variable bases are now all associated with their particular type.
As you can see from this diagram, the data for statics can be acquired by starting at a `MethodTable` and then getting either the `DynamicStaticsInfo` to get a statics pointer, or by getting a `ThreadStaticsInfo` to get a TLSIndex, which then can be used with the thread static variable system to get the actual thread static base.

```mermaid
classDiagram
MethodTable : MethodTableAuxiliaryData* m_pAuxData
MethodTable --> MethodTableAuxiliaryData
MethodTableAuxiliaryData --> DynamicStaticsInfo : If has static variables
MethodTableAuxiliaryData --> GenericStaticsInfo : If is generic and has static variables
MethodTableAuxiliaryData --> ThreadStaticsInfo : If has thread local static variables

DynamicStaticsInfo : StaticsPointer m_pGCStatics
DynamicStaticsInfo : StaticsPointer m_pNonGCStatics

GenericStaticsInfo : FieldDesc* m_pFieldDescs

ThreadStaticsInfo : TLSIndex NonGCTlsIndex
ThreadStaticsInfo : TLSIndex GCTlsIndex
```

```mermaid
classDiagram

note for StaticsPointer "StaticsPointer is a pointer sized integer"
StaticsPointer : void* PointerToStaticBase
StaticsPointer : bool HasClassConstructorBeenRun

note for TLSIndex "TLSIndex is a 32bit integer"
TLSIndex : TLSIndexType indexType
TLSIndex : 24bit int indexOffset
```

In the above diagram, you can see that we have separate fields for non-gc and gc statics, as well as thread and normal statics.
For normal statics, we use a single pointer sized field, which also happens to encode whether or not the class constructor has been run.
This is done to allow lock free atomic access to both get the static field address as well as determine if the class constructor needs to be triggered.
For TLS statics, handling of detecting whether or not the class constructor has been run is a more complex process described as part of the thread statics infrastructure.
The `DynamicStaticsInfo` and `ThreadStaticsInfo` structures are accessed without any locks, so it is important to ensure that access to fields on these structures can be done with a single memory access, to avoid memory order tearing issues.

Also, notably, for generic types, each field has a `FieldDesc` which is allocated per type instance, and is not shared by multiple canonical instances.

#### Lifetime management for collectible statics

Finally we have a concept of collectible assemblies in the CoreCLR runtime, so we need to handle lifetime management for static variables.
The approach chosen was to build a special GC handle type which will allow the runtime to have a pointer in the runtime data structures to the interior of a managed object on the GC heap.

The requirement of behavior here is that a static variable cannot keep its own collectible assembly alive, and so collectible statics have the peculiar property that they can exist and be finalized before the collectible assembly is finally collected.
If there is some resurrection scenario, this can lead to very surprising behavior.

### Thread Statics

Thread statics are static variables which have a lifetime which is defined to be the shorter of the lifetime of the type containing the static, and the lifetime of the thread on which the static variable is accessed.
They are created by having a static variable on a type which is attributed with `[System.Runtime.CompilerServices.ThreadStaticAttribute]`.
The general scheme of how this works is to assign an "index" to the type which is the same on all threads, and then on each thread hold a data structure which is efficiently accessed by means of this index.
However, we have a few peculiarities in our approach.

1. We segregate collectible and non-collectible thread statics (`TLSIndexType::NonCollectible` and `TLSIndexType::Collectible`)
2. We provide an ability to share a non-gc thread static between native CoreCLR code and managed code (Subset of `TLSIndexType::DirectOnThreadLocalData`)
3. We provide an extremely efficient means to access a small number of non-gc thread statics. (The rest of the usage of `TLSIndexType::DirectOnThreadLocalData`)

#### Per-Thread Statics Data structures
```mermaid
classDiagram

note for ThreadLocalInfo "There is 1 of these per thread, and it is managed by the C++ compiler/OS using standard mechanisms.
It can be found as the t_ThreadStatics variable in a C++ compiler, and is also pointed at by the native Thread class."
ThreadLocalInfo : int cNonCollectibleTlsData
ThreadLocalInfo : void** pNonCollectibleTlsArrayData
ThreadLocalInfo : int cCollectibleTlsData
ThreadLocalInfo : void** pCollectibleTlsArrayData
ThreadLocalInfo : InFlightTLSData *pInFightData
ThreadLocalInfo : Thread* pThread
ThreadLocalInfo : Special Thread Statics Shared Between Native and Managed code
ThreadLocalInfo : byte[N] ExtendedDirectThreadLocalTLSData

InFlightTLSData : InFlightTLSData* pNext
InFlightTLSData : TLSIndex tlsIndex
InFlightTLSData : OBJECTHANDLE hTLSData

ThreadLocalInfo --> InFlightTLSData : For TLS statics which have their memory allocated, but have not been accessed since the class finished running its class constructor
InFlightTLSData --> InFlightTLSData : linked list
```

#### Access patterns for getting the thread statics address

This is the pattern that the JIT will use to access a thread static which is not `DirectOnThreadLocalData`.

0. Get the TLS index somehow
1. Get TLS pointer to OS managed TLS block for the current thread ie. `pThreadLocalData = &t_ThreadStatics`
2. Read 1 integer value `pThreadLocalData->cCollectibleTlsData OR pThreadLocalData->cNonCollectibleTlsData`
3. Compare cTlsData against the index we're looking up `if (cTlsData < index.GetIndexOffset())`
4. If the index is not within range, jump to step 11.
5. Read 1 pointer value from TLS block `pThreadLocalData->pCollectibleTlsArrayData` OR `pThreadLocalData->pNonCollectibleTlsArrayData`
6. Read 1 pointer from within the TLS Array. `pTLSBaseAddress = *(intptr_t*)(((uint8_t*)pTlsArrayData) + index.GetIndexOffset()`
7. If pointer is NULL jump to step 11 `if pTLSBaseAddress == NULL`
8. If TLS index not a Collectible index, return pTLSBaseAddress
9. if `ObjectFromHandle((OBJECTHANDLE)pTLSBaseAddress)` is NULL, jump to step 11
10. Return `ObjectFromHandle((OBJECTHANDLE)pTLSBaseAddress)`
11. Tail-call a helper `return GetThreadLocalStaticBase(index)`

This is the pattern that the JIT will use to access a thread static which is on `DirectOnThreadLocalData`
0. Get the TLS index somehow
1. Get TLS pointer to OS managed TLS block for the current thread ie. `pThreadLocalData = &t_ThreadStatics`
2. Add the index offset to the start of the ThreadLocalData structure `pTLSBaseAddress = ((uint8_t*)pThreadLocalData) + index.GetIndexOffset()`

#### Lifetime management for thread static variables
We distinguish between collectible and non-collectible thread static variables for efficiency purposes.

A non-collectible thread static is a thread static defined on a type which cannot be collected by the runtime.
This describes most thread statics in actual observed practice.
The `DirectOnThreadLocalData` statics are a subset of this category which has a speical optimized form and does not need any GC reporting.
For non-collectible thread statics, the pointer (`pNonCollectibleTlsArrayData`) in the `ThreadLocalData` is a pointer to a managed `object[]` which points at either `object[]`, `byte[]`, or `double[]` arrays.
At GC scan time, the pointer to the initial object[] is the only detail which needs to be reported to the GC.

A collectible thread static is a thread static which can be collected by the runtime.
This describes the static variables defined on types which can be collected by the runtime.
The pointer (`pCollectibleTlsArrayData`) in the `ThreadLocalData` is a pointer to a chunk of memory allocated via `malloc`, and holds pointers to `object[]`, `byte[]`, or `double[]` arrays.
At GC scan time, each managed object must individually be kept alive only if the type and thread is still alive. This requires properly handling several situations.
1. If a collectible assembly becomes unreferenced, but a thread static variable associated with it has a finalizer, the object must move to the finalization queue.
2. If a thread static variable associated with a collectible assembly refers to the collectible assembly `LoaderAllocator` via a series of object references, it must not provide a reason for the collectible assembly to be considered referenced.
3. If a collectible assembly is collected, then the associated static variables no longer exist, and the TLSIndex values associated with that collectible assembly becomes re-useable.
4. If a thread is no longer executing, then all thread statics associated with that thread are no longer kept alive.

The approach chosen is to use a pair of different handle types.
For efficient access, the handle type stored in the dynamically adjusted array is a WeakTrackResurrection GCHandle.
This handle instance is associated with the slot in the TLS data, not with the exact instantiation, so it can be re-used when the if the associated collectible assembly is collected, and then the slot is re-used.
In addition, each slot that is in use will have a `LOADERHANDLE` which will keep the object alive until the `LoaderAllocator` is freed.
This `LOADERHANDLE` will be abandoned if the `LoaderAllocator` is collected, but that's ok, as `LOADERHANDLE` only needs to be cleaned up if the `LoaderAllocator` isn't collected.
On thread destroy, for each collectible slot in the tls array, we will explicitly free the `LOADERHANDLE` on the correct `LoaderAllocator`.

Physical Architecture
=====================
Expand All @@ -221,6 +345,7 @@ Major parts of the type system are found in:
- Array – Code for handling the special cases required for array processing
- VirtualStubDispatch.cpp/h/inl – Code for virtual stub dispatch
- VirtualCallStubCpu.hpp – Processor specific code for virtual stub dispatch.
- threadstatics.cpp/h - Handling for thread static variables.

Major entry points are BuildMethodTable, LoadTypeHandleThrowing, CanCastTo\*, GetMethodDescFromMemberDefOrRefOrSpecThrowing, GetFieldDescFromMemberRefThrowing, CompareSigs, and VirtualCallStubManager::ResolveWorkerStatic.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -693,8 +693,8 @@ internal unsafe struct MethodTableAuxiliaryData
[FieldOffset(0)]
private uint Flags;

private const uint enum_flag_CanCompareBitsOrUseFastGetHashCode = 0x0001; // Is any field type or sub field type overrode Equals or GetHashCode
private const uint enum_flag_HasCheckedCanCompareBitsOrUseFastGetHashCode = 0x0002; // Whether we have checked the overridden Equals or GetHashCode
private const uint enum_flag_CanCompareBitsOrUseFastGetHashCode = 0x0004; // Is any field type or sub field type overridden Equals or GetHashCode

public bool HasCheckedCanCompareBitsOrUseFastGetHashCode => (Flags & enum_flag_HasCheckedCanCompareBitsOrUseFastGetHashCode) != 0;

Expand Down
72 changes: 10 additions & 62 deletions src/coreclr/debug/daccess/request.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1715,8 +1715,8 @@ ClrDataAccess::GetModuleData(CLRDATA_ADDRESS addr, struct DacpModuleData *Module
ModuleData->bIsReflection = pModule->IsReflection();
ModuleData->bIsPEFile = pModule->IsPEFile();
ModuleData->Assembly = HOST_CDADDR(pModule->GetAssembly());
ModuleData->dwModuleID = pModule->GetModuleID();
ModuleData->dwModuleIndex = pModule->GetModuleIndex().m_dwIndex;
ModuleData->dwModuleID = 0; // CoreCLR no longer has this concept
ModuleData->dwModuleIndex = 0; // CoreCLR no longer has this concept
ModuleData->dwTransientFlags = pModule->m_dwTransientFlags;
ModuleData->LoaderAllocator = HOST_CDADDR(pModule->m_loaderAllocator);
ModuleData->ThunkHeap = HOST_CDADDR(pModule->m_pThunkHeap);
Expand Down Expand Up @@ -3246,47 +3246,16 @@ ClrDataAccess::GetNestedExceptionData(CLRDATA_ADDRESS exception, CLRDATA_ADDRESS
HRESULT
ClrDataAccess::GetDomainLocalModuleData(CLRDATA_ADDRESS addr, struct DacpDomainLocalModuleData *pLocalModuleData)
{
if (addr == 0 || pLocalModuleData == NULL)
return E_INVALIDARG;

SOSDacEnter();

DomainLocalModule* pLocalModule = PTR_DomainLocalModule(TO_TADDR(addr));

pLocalModuleData->pGCStaticDataStart = TO_CDADDR(PTR_TO_TADDR(pLocalModule->GetPrecomputedGCStaticsBasePointer()));
pLocalModuleData->pNonGCStaticDataStart = TO_CDADDR(pLocalModule->GetPrecomputedNonGCStaticsBasePointer());
pLocalModuleData->pDynamicClassTable = PTR_CDADDR(pLocalModule->m_pDynamicClassTable.Load());
pLocalModuleData->pClassData = (TADDR) (PTR_HOST_MEMBER_TADDR(DomainLocalModule, pLocalModule, m_pDataBlob));

SOSDacLeave();
return hr;
// CoreCLR does not use domain local modules anymore
Copy link
Member

@jkotas jkotas Mar 27, 2024

Choose a reason for hiding this comment

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

Does this need to come with SOS_BREAKING_CHANGE_VERSION bump?

Copy link
Member

Choose a reason for hiding this comment

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

The check/warning message in SOS is only done for commands that assume runtime data structure layout like dumplog, threads -special, printexception. These 2 APIs are called in dumpobj which doesn't do the check and will fail with no warning.

It looks like SOS should do the check/warning on every command. I didn't consider these type of API breaking changes just layout changes when I added the checks. I'll add a SOS tracking issue to address this.

In the end, yes, SOS_BREAKING_CHANGE_VERSION should be bumped.

Copy link
Member Author

@davidwrighton davidwrighton Mar 27, 2024

Choose a reason for hiding this comment

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

Interesting. I had interpreted the failures in !dumpobj as pretty reasonable and not breaking. The way it failed is fairly graceful, as instead of crashing, SOS would simply report "<no information>" for normal statics, and skip reporting for any TLS statics.

Copy link
Member

Choose a reason for hiding this comment

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

I'll add a SOS tracking issue to address this.

It impacts clrmd too.

SOS would simply report "" for normal statics, and skip reporting for any TLS statics.

I think it is hard for a random person out there to connect the dots between SOS failing to dump the statics and needing a newer SOS version to fix that.

return E_NOTIMPL;
}


HRESULT
ClrDataAccess::GetDomainLocalModuleDataFromModule(CLRDATA_ADDRESS addr, struct DacpDomainLocalModuleData *pLocalModuleData)
{
if (addr == 0 || pLocalModuleData == NULL)
return E_INVALIDARG;

SOSDacEnter();

Module* pModule = PTR_Module(TO_TADDR(addr));
DomainLocalModule* pLocalModule = PTR_DomainLocalModule(pModule->GetDomainLocalModule());
if (!pLocalModule)
{
hr = E_INVALIDARG;
}
else
{
pLocalModuleData->pGCStaticDataStart = TO_CDADDR(PTR_TO_TADDR(pLocalModule->GetPrecomputedGCStaticsBasePointer()));
pLocalModuleData->pNonGCStaticDataStart = TO_CDADDR(pLocalModule->GetPrecomputedNonGCStaticsBasePointer());
pLocalModuleData->pDynamicClassTable = PTR_CDADDR(pLocalModule->m_pDynamicClassTable.Load());
pLocalModuleData->pClassData = (TADDR) (PTR_HOST_MEMBER_TADDR(DomainLocalModule, pLocalModule, m_pDataBlob));
}

SOSDacLeave();
return hr;
// CoreCLR does not use domain local modules anymore
return E_NOTIMPL;
}

HRESULT
Expand All @@ -3299,31 +3268,8 @@ ClrDataAccess::GetDomainLocalModuleDataFromAppDomain(CLRDATA_ADDRESS appDomainAd
HRESULT
ClrDataAccess::GetThreadLocalModuleData(CLRDATA_ADDRESS thread, unsigned int index, struct DacpThreadLocalModuleData *pLocalModuleData)
{
if (pLocalModuleData == NULL)
return E_INVALIDARG;

SOSDacEnter();

pLocalModuleData->threadAddr = thread;
pLocalModuleData->ModuleIndex = index;

PTR_Thread pThread = PTR_Thread(TO_TADDR(thread));
PTR_ThreadLocalBlock pLocalBlock = ThreadStatics::GetCurrentTLB(pThread);
PTR_ThreadLocalModule pLocalModule = pLocalBlock->GetTLMIfExists(ModuleIndex(index));
if (!pLocalModule)
{
hr = E_INVALIDARG;
}
else
{
pLocalModuleData->pGCStaticDataStart = TO_CDADDR(PTR_TO_TADDR(pLocalModule->GetPrecomputedGCStaticsBasePointer()));
pLocalModuleData->pNonGCStaticDataStart = TO_CDADDR(pLocalModule->GetPrecomputedNonGCStaticsBasePointer());
pLocalModuleData->pDynamicClassTable = PTR_CDADDR(pLocalModule->m_pDynamicClassTable);
pLocalModuleData->pClassData = (TADDR) (PTR_HOST_MEMBER_TADDR(ThreadLocalModule, pLocalModule, m_pDataBlob));
}

SOSDacLeave();
return hr;
// CoreCLR does not use thread local modules anymore
return E_NOTIMPL;
}


Expand Down Expand Up @@ -3656,6 +3602,7 @@ static const char *LoaderAllocatorLoaderHeapNames[] =
{
"LowFrequencyHeap",
"HighFrequencyHeap",
"StaticsHeap",
"StubHeap",
"ExecutableHeap",
"FixupPrecodeHeap",
Expand Down Expand Up @@ -3690,6 +3637,7 @@ HRESULT ClrDataAccess::GetLoaderAllocatorHeaps(CLRDATA_ADDRESS loaderAllocatorAd
int i = 0;
pLoaderHeaps[i++] = HOST_CDADDR(pLoaderAllocator->GetLowFrequencyHeap());
pLoaderHeaps[i++] = HOST_CDADDR(pLoaderAllocator->GetHighFrequencyHeap());
pLoaderHeaps[i++] = HOST_CDADDR(pLoaderAllocator->GetStaticsHeap());
pLoaderHeaps[i++] = HOST_CDADDR(pLoaderAllocator->GetStubHeap());
pLoaderHeaps[i++] = HOST_CDADDR(pLoaderAllocator->GetExecutableHeap());
pLoaderHeaps[i++] = HOST_CDADDR(pLoaderAllocator->GetFixupPrecodeHeap());
Expand Down
Loading
Loading