Reduce risk of the Volatile<T>/VolatilePtr<T,P> structures by davidwrighton · Pull Request #124106 · dotnet/runtime

davidwrighton · 2026-02-06T20:52:16Z

Explicitly delete the copy/move constructors and assignment operators which did not store/load using barriers
Make the value based constructors explicit (since those didn't store with barriers)
Convert assignments of NULL into VolatilePtr to use nullptr so we can go down a consistent path
Adjust the codebase to work with these changes

Also remove the other operator overloads on Volatile<T>.

These changes are a less suitable for backport version of the work in #124096 These changes reduce the risky surface area of Volatile to a small set of explicit constructors and the GetPointer, RawValue and operator& apis.

Notable discoveries during this work. There is a small race condition around handling the DebuggableAttribute during Reflection.Emit. There were a few small issues in the profiler api. And this made it a bit more clear that the handling of counting the number of alloc_contexts associated with a given gc heap during heap balancing was non-atomic. This is a know characteristic of the GC, and does not affect correctness of the overall GC, but if it is wrong might cause very short term performance issues. Notably, this is self-correcting, as after each GC, it will reset the counters, so this cannot cause any significant long term impacts.

- Explicitly delete the copy constructors and equals operators which did not store/load using barriers - Make the value based constructors explicit (since those didn't store with barriers) - Convert assignments of NULL into VolatilePtr to use nullptr so we can go down a consistent path - Adjust the codebase to work with these changes

dotnet-policy-service · 2026-02-06T20:53:42Z

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

Copilot

Pull request overview

This PR hardens CoreCLR’s Volatile<T> / VolatilePtr<T,P> wrappers by preventing implicit/copy construction and by ensuring assignments between volatile wrappers go through Load/Store paths (and thus respect the intended barrier semantics). It also updates a broad set of call sites to use explicit construction and nullptr to match the new API surface.

Changes:

Make Volatile<T> / VolatilePtr<T,P> value-based constructors explicit, delete copy/move constructors, and add explicit copy-assignment operators that route through Load/Store.
Update many static initializers and assignments across VM/GC/utilcode/JIT to avoid copy-initialization and prefer direct-init + nullptr.
Introduce VOLATILE_INIT(val) to allow consistent initialization across configurations where VOLATILE(T) is either T volatile or Volatile<T>.

Reviewed changes

Copilot reviewed 37 out of 37 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/coreclr/vm/vars.cpp	Switch `Volatile<LONG>` statics to direct-init.
src/coreclr/vm/util.cpp	Switch `Volatile<double>` static to direct-init.
src/coreclr/vm/threads.h	Use `nullptr` for `VolatilePtr` assignment.
src/coreclr/vm/threads.cpp	Use `nullptr` for several pointer members / fields.
src/coreclr/vm/syncclean.cpp	Update `VolatilePtr` statics to explicit direct-init with `nullptr`.
src/coreclr/vm/runtimecallablewrapper.h	Update macro-local `Volatile<LPVOID>` to direct-init.
src/coreclr/vm/loaderallocator.hpp	Use `nullptr` for `VolatilePtr`/pointer reset.
src/coreclr/vm/loaderallocator.cpp	Use `nullptr` for member initialization/reset paths.
src/coreclr/vm/jitinterface.cpp	Switch `Volatile<int64_t>` globals to direct-init.
src/coreclr/vm/interoplibinterface_java.cpp	Switch `Volatile<bool>` global to direct-init.
src/coreclr/vm/hosting.cpp	Update debug statics (`VolatilePtr`/`Volatile`) to direct-init.
src/coreclr/vm/finalizerthread.cpp	Update `VolatilePtr` static to explicit direct-init.
src/coreclr/vm/excep.cpp	Switch `Volatile<BOOL>` static to direct-init.
src/coreclr/vm/eventtrace.cpp	Switch `Volatile<LONGLONG>` static to direct-init.
src/coreclr/vm/eehash.inl	Use `nullptr` on volatile bucket table reset.
src/coreclr/vm/eehash.h	Use `nullptr` on volatile bucket table initialization.
src/coreclr/vm/crst.cpp	Switch `Volatile<LONG>` global to direct-init.
src/coreclr/vm/comcallablewrapper.h	Use `nullptr` when clearing auxiliary pointers.
src/coreclr/vm/comcallablewrapper.cpp	Use `nullptr` after deleting auxiliary data.
src/coreclr/vm/ceemain.cpp	Switch `Volatile<BOOL>` globals to direct-init.
src/coreclr/vm/appdomain.cpp	Use `nullptr` for member init in initializer list.
src/coreclr/utilcode/utsem.cpp	Switch `Volatile<BOOL>` global to direct-init.
src/coreclr/utilcode/util.cpp	Switch `Volatile<uint32_t>` static to direct-init.
src/coreclr/utilcode/stresslog.cpp	Update aggregate init to explicitly construct `Volatile<>` members.
src/coreclr/utilcode/debug.cpp	Switch `Volatile<LONG>` global to direct-init.
src/coreclr/jit/compiler.hpp	Switch local `Volatile<>` temps to direct-init.
src/coreclr/interop/trackerobjectmanager.cpp	Switch `Volatile<bool>` static to direct-init.
src/coreclr/inc/volatile.h	Core change: make constructors explicit, delete copy/move ctors, add assignment ops, add `VOLATILE_INIT`.
src/coreclr/inc/profilepriv.inl	Use `nullptr` for `VolatilePtr` reset.
src/coreclr/inc/daccess.h	Update `VOLATILE_SVAL_IMPL_INIT` to use direct-init.
src/coreclr/gc/objecthandle.cpp	Use `VOLATILE_INIT` for static initialization.
src/coreclr/gc/gcscan.cpp	Use `VOLATILE_INIT` for static initialization.
src/coreclr/gc/gcpriv.h	Add `gc_mechanisms` constructors to support new volatile wrapper restrictions.
src/coreclr/gc/gceventstatus.cpp	Update array initialization to explicitly construct `Volatile<>` elements.
src/coreclr/gc/gccommon.cpp	Use `VOLATILE_INIT` for global initialization.
src/coreclr/gc/gc.cpp	Update many `VOLATILE(...)` statics to direct-init and adjust `gc_mechanisms` copy path.
src/coreclr/gc/env/volatile.h	Mirror volatile-wrapper hardening in GC env header and add `VOLATILE_INIT`.

src/coreclr/inc/volatile.h

src/coreclr/gc/env/volatile.h

Copilot

Pull request overview

Copilot reviewed 44 out of 44 changed files in this pull request and generated 1 comment.

Copilot · 2026-02-06T23:10:31Z

src/coreclr/gc/env/volatile.h

+    //
+    inline VolatilePtr<T, P>& operator=(P val) {this->Store(val); return *this;}
+    inline VolatilePtr<T, P>& operator=(const VolatilePtr<T, P>& val) {this->Store(val.Load()); return *this;}
+    inline VolatilePtr<T, P>& operator=(VolatilePtr<T, P>&& val) = delete;
+    // nullptr is assigned via nullptr_t
+    inline VolatilePtr<T, P>& operator=(std::nullptr_t val) {this->Store((P)val); return *this;}


std::nullptr_t is used here but this header doesn’t include <cstddef>, so compilation can fail depending on include order. Add #include <cstddef> (or use decltype(nullptr) instead of std::nullptr_t) to make the header self-contained.

Suggested change

//

inline VolatilePtr<T, P>& operator=(P val) {this->Store(val); return *this;}

inline VolatilePtr<T, P>& operator=(const VolatilePtr<T, P>& val) {this->Store(val.Load()); return *this;}

inline VolatilePtr<T, P>& operator=(VolatilePtr<T, P>&& val) = delete;

// nullptr is assigned via nullptr_t

inline VolatilePtr<T, P>& operator=(std::nullptr_t val) {this->Store((P)val); return *this;}

// Assignment from P

//

inline VolatilePtr<T, P>& operator=(P val) {this->Store(val); return *this;}

inline VolatilePtr<T, P>& operator=(const VolatilePtr<T, P>& val) {this->Store(val.Load()); return *this;}

inline VolatilePtr<T, P>& operator=(VolatilePtr<T, P>&& val) = delete;

// nullptr is assigned via nullptr_t

inline VolatilePtr<T, P>& operator=(decltype(nullptr) val) {this->Store((P)val); return *this;}

- Minor changes to profiler/Module behavior. Technically this is a fix for the Reflection.Emit scenario, but its unlikely it will impact well written code - The usage in the GC is much more interesting. The rewritten logic here is equivalent to what we had before, but it may be a bug that alloc_context_count is not actually maintained in an accurate fashion.

…n/runtime into AdjustVolatileUsage

- synccache.hpp logic had m_iDepth and m_pHead as volatile variables, but they are never actually accessed while not under a lock, so they do not need to be volatile. - synchobject.hpp had m_lLocalSynchLockCount as Volatile<T>, but it was never mutated on a anything other than the owning thread. The fix was to make this a normal field. - init_count was using Volatile<T> appropriately, but changing the arithmetic to use a + 1 model instead of ++ was simple.

…olatileUsage

Copilot

Pull request overview

Copilot reviewed 49 out of 49 changed files in this pull request and generated 7 comments.

Copilot · 2026-02-10T00:00:18Z

src/coreclr/gc/env/volatile.h

+    inline VolatilePtr<T, P>& operator=(const VolatilePtr<T, P>& val) {this->Store(val.Load()); return *this;}
+    inline VolatilePtr<T, P>& operator=(VolatilePtr<T, P>&& val) = delete;
+    // nullptr is assigned via nullptr_t
+    inline VolatilePtr<T, P>& operator=(std::nullptr_t val) {this->Store((P)val); return *this;}


VolatilePtr::operator=(std::nullptr_t) stores (P)val, which won’t compile for pointer-wrapper P types (like DAC DPTR<T>). Store a null P via value-initialization instead so nullptr assignment works uniformly.

Suggested change

inline VolatilePtr<T, P>& operator=(std::nullptr_t val) {this->Store((P)val); return *this;}

inline VolatilePtr<T, P>& operator=(std::nullptr_t) { this->Store(P()); return *this; }

Copilot · 2026-02-10T00:00:18Z

src/coreclr/vm/ceeload.cpp

+    for (;;)
+    {
+        DWORD dwTransientFlags = m_dwTransientFlags;
+        DWORD dwNewTransientFlags = (dwTransientFlags & ~dwMask) | dwFlag;


SetTransientFlagInterlockedWithMask claims to set bits “according to the given mask”, but it ORs in dwFlag without constraining it to dwMask. This can unintentionally set bits outside the mask if a caller passes a wider flag value. Mask dwFlag with dwMask when computing dwNewTransientFlags to match the method contract.

Suggested change

DWORD dwNewTransientFlags = (dwTransientFlags & ~dwMask) | dwFlag;

DWORD dwNewTransientFlags = (dwTransientFlags & ~dwMask) | (dwFlag & dwMask);

Copilot · 2026-02-10T00:00:18Z

src/coreclr/inc/volatile.h

+    inline explicit VolatilePtr(std::nullptr_t val) : Volatile<P>((P)val)
    {


This header now uses std::nullptr_t, but it doesn’t include <cstddef> (and some includers pull in volatile.h before any C++ standard headers). To keep the header self-contained and avoid build breaks, include <cstddef> here or switch the signature to decltype(nullptr) to avoid the dependency.

Copilot · 2026-02-10T00:00:18Z

src/coreclr/gc/env/volatile.h

+    inline explicit VolatilePtr(std::nullptr_t val) : Volatile<P>((P)val)
    {


This header now references std::nullptr_t but doesn’t include <cstddef>. Since some translation units may include this header before any standard C++ headers, this can cause compilation failures. Add the required include or use decltype(nullptr) instead of std::nullptr_t.

Copilot · 2026-02-10T00:00:19Z

src/coreclr/inc/volatile.h

+    inline explicit VolatilePtr(std::nullptr_t val) : Volatile<P>((P)val)
    {
        STATIC_CONTRACT_SUPPORTS_DAC;
    }


VolatilePtr(std::nullptr_t) currently initializes the underlying P via (P)val. This cast won’t work for non-raw pointer P types like DAC DPTR<T> wrappers (which are class types and aren’t constructible from nullptr_t). Prefer value-initializing P (e.g., P{}/P()) to represent null for both raw pointers and wrapper pointer types.

Copilot · 2026-02-10T00:00:19Z

src/coreclr/inc/volatile.h

+    inline VolatilePtr<T, P>& operator=(const VolatilePtr<T, P>& val) {this->Store(val.Load()); return *this;}
+    inline VolatilePtr<T, P>& operator=(VolatilePtr<T, P>&& val) = delete;
+    // nullptr is assigned via nullptr_t
+    inline VolatilePtr<T, P>& operator=(std::nullptr_t val) {this->Store((P)val); return *this;}


VolatilePtr::operator=(std::nullptr_t) uses (P)val, which will fail for wrapper pointer types such as DAC DPTR<T> (class types not convertible from nullptr_t). Use a null P value via value-initialization instead so assignments to null work for both raw and wrapper pointer types.

Suggested change

inline VolatilePtr<T, P>& operator=(std::nullptr_t val) {this->Store((P)val); return *this;}

inline VolatilePtr<T, P>& operator=(std::nullptr_t) {this->Store(P()); return *this;}

Copilot · 2026-02-10T00:00:19Z

src/coreclr/gc/env/volatile.h

+    inline explicit VolatilePtr(std::nullptr_t val) : Volatile<P>((P)val)
    {
    }


VolatilePtr(std::nullptr_t) initializes P via (P)val. If P is a pointer-wrapper (e.g., DAC DPTR<T>), this cast is ill-formed since nullptr_t doesn’t convert to the wrapper type. Prefer value-initializing P (e.g., P{}/P()) to obtain a null representation across both raw pointers and wrapper pointer types.

Copilot AI review requested due to automatic review settings February 6, 2026 20:52

github-actions bot added the area-VM-coreclr label Feb 6, 2026

dotnet-policy-service bot assigned davidwrighton Feb 6, 2026

Copilot started reviewing on behalf of davidwrighton February 6, 2026 20:53 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

davidwrighton added 2 commits February 6, 2026 14:10

Fix Linux X64 build

6b01a8d

Copilot fixes + another build fix

8c8f13a

Copilot AI review requested due to automatic review settings February 6, 2026 23:05

Copilot started reviewing on behalf of davidwrighton February 6, 2026 23:06 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

davidwrighton added 3 commits February 6, 2026 17:05

Merge branch 'AdjustVolatileUsage' of https://github.com/davidwrighto…

808bd08

…n/runtime into AdjustVolatileUsage

Fix missed operator usage

30fcea1

build-analysis bot mentioned this pull request Feb 7, 2026

[android][arm64] System.Net.Sockets.Tests.SendTo_SyncForceNonBlocking.Datagram_UDP_ShouldImplicitlyBindLocalEndpoint fails with NetworkUnreachable #120526

Open

davidwrighton added 3 commits February 9, 2026 15:32

Merge branch 'main' of https://github.com/dotnet/runtime into AdjustV…

68e55d5

…olatileUsage

More merge details

95bd772

Copilot AI review requested due to automatic review settings February 9, 2026 23:50

Copilot started reviewing on behalf of davidwrighton February 9, 2026 23:51 View session

Copilot AI reviewed Feb 10, 2026

View reviewed changes

Remove unneeded operators

bfe7b9c

build-analysis bot mentioned this pull request Feb 10, 2026

Test failure: baseservices/exceptions/stackoverflow/stackoverflowtester/stackoverflowtester.cmd #110173

Open

davidwrighton mentioned this pull request Feb 10, 2026

Reduce custom operator behavior in the Volatile<T> structure #124244

Open

	inline VolatilePtr<T, P>& operator=(std::nullptr_t val) {this->Store((P)val); return *this;}
	inline VolatilePtr<T, P>& operator=(std::nullptr_t) { this->Store(P()); return *this; }

	DWORD dwNewTransientFlags = (dwTransientFlags & ~dwMask) \| dwFlag;
	DWORD dwNewTransientFlags = (dwTransientFlags & ~dwMask) \| (dwFlag & dwMask);

		inline explicit VolatilePtr(std::nullptr_t val) : Volatile<P>((P)val)
		{

Comments

Conversation

davidwrighton commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dotnet-policy-service bot commented Feb 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

davidwrighton commented Feb 6, 2026 •

edited

Loading