From 5744f1dd8dfe58b3be754464c62eadcc7e4d3eb5 Mon Sep 17 00:00:00 2001
From: Jeremy Kuhne <jeremy.kuhne@microsoft.com>
Date: Fri, 14 Feb 2025 10:57:51 -0800
Subject: [PATCH] Avoid ECMA spec violation in PropertyStore

The manual unboxing we were doing was not an ECMA approved use of the instruction. As such it could potentially fail in the future should the runtime decide to break the way we were using it.

One way to fix this would be to set each of the fields individually on the unboxed ref as the current two types we're optimizing are mutable. I've instead used `StrongBox<T>`. While significantly more complicated, it will not run afoul of the ECMA spec should we get an immutable type we want to handle.

If performance here ever ends up being a problem we could go the other way.

Fixes #12933
---
 .../src/System/Windows/Forms/PropertyStore.cs | 29 ++++++++++++-------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/PropertyStore.cs b/src/System.Windows.Forms/src/System/Windows/Forms/PropertyStore.cs
index 93ae93ff3c1..336207ac233 100644
--- a/src/System.Windows.Forms/src/System/Windows/Forms/PropertyStore.cs
+++ b/src/System.Windows.Forms/src/System/Windows/Forms/PropertyStore.cs
@@ -37,13 +37,14 @@ internal sealed class PropertyStore
 
     /// <summary>
     ///  Gets the current value for the given key, or the <paramref name="defaultValue"/> if the key is not found.
-    ///  Does not allow stored values of <see langword="null"/>.
     /// </summary>
     public T? GetValueOrDefault<T>(int key, T? defaultValue = default)
     {
         if (_values.TryGetValue(key, out Value foundValue))
         {
-            return foundValue.GetValue<T>();
+            return foundValue.Type == typeof(StrongBox<T>)
+                ? foundValue.GetValue<StrongBox<T>>().Value
+                : foundValue.GetValue<T>();
         }
 
         return defaultValue;
@@ -85,7 +86,10 @@ public bool TryGetValue<T>(int key, [NotNullWhen(true)] out T? value)
     {
         if (_values.TryGetValue(key, out Value foundValue))
         {
-            value = foundValue.GetValue<T>();
+            value = foundValue.Type == typeof(StrongBox<T>)
+                ? foundValue.GetValue<StrongBox<T>>().Value
+                : foundValue.GetValue<T>();
+
             return value is not null;
         }
 
@@ -159,9 +163,13 @@ public bool AddOrRemoveString(int key, string? value)
         bool isDefault = (value is null && defaultValue is null)
             || (value is not null && value.Equals(defaultValue));
 
+        bool isStrongBox = foundValue.Type == typeof(StrongBox<T>);
+
         // The previous should be whatever we found or what was specified as the default.
         T? previous = found
-            ? foundValue.Type is null ? default : foundValue.GetValue<T>()
+            ? isStrongBox
+                ? foundValue.GetValue<StrongBox<T>>().Value
+                : foundValue.Type is null ? default : foundValue.GetValue<T>()
             : defaultValue;
 
         if (isDefault)
@@ -174,8 +182,8 @@ public bool AddOrRemoveString(int key, string? value)
         }
         else if (!found || !ReferenceEquals(value, previous))
         {
-            // If it wasn't found or it is the same instance we don't need set it.
-            _values[key] = new(value);
+            // If it wasn't found or it isn't the same instance we need set it.
+            AddValue(key, value);
         }
 
         return previous;
@@ -211,15 +219,14 @@ private unsafe void AddOrUpdate<T>(int key, T value) where T : unmanaged
         // Should only call this from SetValue<T> for value types that are larger than 8 bytes.
         Debug.Assert(sizeof(T) > 8);
 
-        if (_values.TryGetValue(key, out Value foundValue) && foundValue.Type == typeof(T))
+        if (_values.TryGetValue(key, out Value foundValue) && foundValue.Type == typeof(StrongBox<T>))
         {
-            object storedValue = foundValue.GetValue<object>();
-            ref T unboxed = ref Unsafe.Unbox<T>(storedValue);
-            unboxed = value;
+            StrongBox<T> storedValue = foundValue.GetValue<StrongBox<T>>();
+            storedValue.Value = value;
         }
         else
         {
-            _values[key] = Value.Create(value);
+            _values[key] = Value.Create(new StrongBox<T>(value));
         }
     }
 }