Skip to content

Commit cafd697

Browse files
jeromelabanjeromelaban
committed
fix: resolve add member typeguard and nested type reflection issues
1 parent 73a656d commit cafd697

2 files changed

Lines changed: 446 additions & 0 deletions

File tree

‎patches/0003-fix-hot-reload-field-add-member-typedef-guard.patch‎

Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,215 @@
1+
diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNewTypeModifyField/AddNewTypeModifyField.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNewTypeModifyField/AddNewTypeModifyField.cs
2+
new file mode 100644
3+
index 00000000000..0366b29b57e
4+
--- /dev/null
5+
+++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNewTypeModifyField/AddNewTypeModifyField.cs
6+
@@ -0,0 +1,14 @@
7+
+// Licensed to the .NET Foundation under one or more agreements.
8+
+// The .NET Foundation licenses this file to you under the MIT license.
9+
+using System;
10+
+
11+
+namespace System.Reflection.Metadata.ApplyUpdate.Test
12+
+{
13+
+ public class AddNewTypeModifyField
14+
+ {
15+
+ public string TestMethod()
16+
+ {
17+
+ return "baseline";
18+
+ }
19+
+ }
20+
+}
21+
diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNewTypeModifyField/AddNewTypeModifyField_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNewTypeModifyField/AddNewTypeModifyField_v1.cs
22+
new file mode 100644
23+
index 00000000000..ddc5abe7855
24+
--- /dev/null
25+
+++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNewTypeModifyField/AddNewTypeModifyField_v1.cs
26+
@@ -0,0 +1,36 @@
27+
+// Licensed to the .NET Foundation under one or more agreements.
28+
+// The .NET Foundation licenses this file to you under the MIT license.
29+
+using System;
30+
+
31+
+namespace System.Reflection.Metadata.ApplyUpdate.Test
32+
+{
33+
+ public class AddNewTypeModifyField
34+
+ {
35+
+ public string TestMethod()
36+
+ {
37+
+ var s = new Settings();
38+
+ return s.GetDescription();
39+
+ }
40+
+ }
41+
+
42+
+ /// <summary>
43+
+ /// Generation 1: adds a new type with auto-properties (which produce
44+
+ /// backing fields) and an explicit field. These create Field entries
45+
+ /// with a preceding ENC_FUNC_ADD_FIELD in the ENCLOG.
46+
+ /// </summary>
47+
+ public class Settings
48+
+ {
49+
+ public string Theme { get; set; } = "Light";
50+
+ public int FontSize { get; set; } = 14;
51+
+ public bool ShowLineNumbers { get; set; } = true;
52+
+
53+
+ // explicit field
54+
+ internal string _cachedDescription;
55+
+
56+
+ public string GetDescription()
57+
+ {
58+
+ _cachedDescription = Theme + ":" + FontSize;
59+
+ return _cachedDescription;
60+
+ }
61+
+ }
62+
+}
63+
diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNewTypeModifyField/AddNewTypeModifyField_v2.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNewTypeModifyField/AddNewTypeModifyField_v2.cs
64+
new file mode 100644
65+
index 00000000000..f17d98710b4
66+
--- /dev/null
67+
+++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNewTypeModifyField/AddNewTypeModifyField_v2.cs
68+
@@ -0,0 +1,37 @@
69+
+// Licensed to the .NET Foundation under one or more agreements.
70+
+// The .NET Foundation licenses this file to you under the MIT license.
71+
+using System;
72+
+
73+
+namespace System.Reflection.Metadata.ApplyUpdate.Test
74+
+{
75+
+ public class AddNewTypeModifyField
76+
+ {
77+
+ public string TestMethod()
78+
+ {
79+
+ var s = new Settings();
80+
+ return s.GetDescription();
81+
+ }
82+
+ }
83+
+
84+
+ /// <summary>
85+
+ /// Generation 2: modifies the method body and property defaults of the
86+
+ /// same type added in gen 1. The ENCLOG re-emits Field tokens for the
87+
+ /// existing backing fields WITHOUT a preceding ENC_FUNC_ADD_FIELD entry
88+
+ /// — the add_member_typedef context variable is zero.
89+
+ /// Before the fix, g_assert(add_member_typedef) crashed here.
90+
+ /// </summary>
91+
+ public class Settings
92+
+ {
93+
+ public string Theme { get; set; } = "Dark";
94+
+ public int FontSize { get; set; } = 16;
95+
+ public bool ShowLineNumbers { get; set; } = false;
96+
+
97+
+ internal string _cachedDescription;
98+
+
99+
+ public string GetDescription()
100+
+ {
101+
+ _cachedDescription = Theme + ":" + FontSize + ":" + (ShowLineNumbers ? "ln" : "no-ln");
102+
+ return _cachedDescription;
103+
+ }
104+
+ }
105+
+}
106+
diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNewTypeModifyField/System.Reflection.Metadata.ApplyUpdate.Test.AddNewTypeModifyField.csproj b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNewTypeModifyField/System.Reflection.Metadata.ApplyUpdate.Test.AddNewTypeModifyField.csproj
107+
new file mode 100644
108+
index 00000000000..fb455276ee5
109+
--- /dev/null
110+
+++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNewTypeModifyField/System.Reflection.Metadata.ApplyUpdate.Test.AddNewTypeModifyField.csproj
111+
@@ -0,0 +1,11 @@
112+
+<Project Sdk="Microsoft.NET.Sdk">
113+
+ <PropertyGroup>
114+
+ <RootNamespace>System.Runtime.Loader.Tests</RootNamespace>
115+
+ <TargetFramework>$(NetCoreAppCurrent)</TargetFramework>
116+
+ <TestRuntime>true</TestRuntime>
117+
+ <DeltaScript>deltascript.json</DeltaScript>
118+
+ </PropertyGroup>
119+
+ <ItemGroup>
120+
+ <Compile Include="AddNewTypeModifyField.cs" />
121+
+ </ItemGroup>
122+
+</Project>
123+
diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNewTypeModifyField/deltascript.json b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNewTypeModifyField/deltascript.json
124+
new file mode 100644
125+
index 00000000000..905ee84bedb
126+
--- /dev/null
127+
+++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNewTypeModifyField/deltascript.json
128+
@@ -0,0 +1,6 @@
129+
+{
130+
+ "changes": [
131+
+ {"document": "AddNewTypeModifyField.cs", "update": "AddNewTypeModifyField_v1.cs"},
132+
+ {"document": "AddNewTypeModifyField.cs", "update": "AddNewTypeModifyField_v2.cs"}
133+
+ ]
134+
+}
135+
diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs
136+
index 8506c146367..f7f451c54ca 100644
137+
--- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs
138+
+++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs
139+
@@ -979,6 +979,41 @@ public static void TestReflectionAddNewTypeWithProperties()
140+
});
141+
}
142+
143+
+ [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))]
144+
+ public static void TestAddNewTypeModifyField()
145+
+ {
146+
+ // Regression test: adding a new type with auto-properties (backing fields)
147+
+ // in gen 1, then modifying the type in gen 2. Gen 2's ENCLOG re-emits
148+
+ // Field entries without a preceding ENC_FUNC_ADD_FIELD, so add_member_typedef
149+
+ // is zero. Before the fix, g_assert(add_member_typedef) crashed here.
150+
+ ApplyUpdateUtil.TestCase(static () =>
151+
+ {
152+
+ var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.AddNewTypeModifyField).Assembly;
153+
+
154+
+ var x = new System.Reflection.Metadata.ApplyUpdate.Test.AddNewTypeModifyField();
155+
+ Assert.Equal("baseline", x.TestMethod());
156+
+
157+
+ // Gen 1: adds Settings with auto-properties and explicit field
158+
+ ApplyUpdateUtil.ApplyUpdate(assm);
159+
+
160+
+ var result = x.TestMethod();
161+
+ Assert.Equal("Light:14", result);
162+
+
163+
+ // Verify the type was created with properties
164+
+ var settingsTy = assm.GetType("System.Reflection.Metadata.ApplyUpdate.Test.Settings");
165+
+ Assert.NotNull(settingsTy);
166+
+ Assert.NotNull(settingsTy.GetProperty("Theme"));
167+
+ Assert.NotNull(settingsTy.GetProperty("FontSize"));
168+
+
169+
+ // Gen 2: modifies method body + defaults — re-emits Field tokens
170+
+ // without add_member_typedef. This is the crash point before fix 4.
171+
+ ApplyUpdateUtil.ApplyUpdate(assm);
172+
+
173+
+ result = x.TestMethod();
174+
+ Assert.Equal("Dark:16:no-ln", result);
175+
+ });
176+
+ }
177+
+
178+
[ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))]
179+
public static void TestReflectionAddNewMethod()
180+
{
181+
diff --git a/src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj b/src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj
182+
index 891545bf721..5d04d7bc2f5 100644
183+
--- a/src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj
184+
+++ b/src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj
185+
@@ -85,6 +85,7 @@
186+
<ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.GenericAddInstanceField\System.Reflection.Metadata.ApplyUpdate.Test.GenericAddInstanceField.csproj" />
187+
<ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.NewMethodThrows\System.Reflection.Metadata.ApplyUpdate.Test.NewMethodThrows.csproj" />
188+
<ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.IncreaseMetadataRowSize\System.Reflection.Metadata.ApplyUpdate.Test.IncreaseMetadataRowSize.csproj" />
189+
+ <ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.AddNewTypeModifyField\System.Reflection.Metadata.ApplyUpdate.Test.AddNewTypeModifyField.csproj" />
190+
</ItemGroup>
191+
<ItemGroup Condition="'$(TargetOS)' == 'browser'">
192+
<WasmFilesToIncludeFromPublishDir Include="$(AssemblyName).dll" />
193+
diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c
194+
index ff2ea665159..8a24d0ebfee 100644
195+
--- a/src/mono/mono/component/hot_reload.c
196+
+++ b/src/mono/mono/component/hot_reload.c
197+
@@ -2041,11 +2041,13 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base
198+
break;
199+
}
200+
case MONO_TABLE_FIELD: {
201+
- /* is_addition can be false when the field token index falls within
202+
- * prev_gen_rows (e.g. adding properties to an existing type where
203+
- * the backing field reuses a token slot). The real guard is
204+
- * add_member_typedef which is set by the preceding ENC_FUNC_ADD_FIELD entry. */
205+
- g_assert (add_member_typedef);
206+
+ /* add_member_typedef is set by the preceding ENC_FUNC_ADD_FIELD entry.
207+
+ * When zero, the field is being re-processed in a later generation
208+
+ * without a new add-field signal — skip registration. */
209+
+ if (!add_member_typedef) {
210+
+ mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "EnC: field token=0x%08x without add_member_typedef, skipping", log_token);
211+
+ break;
212+
+ }
213+
if (pass2_context_is_skeleton (ctx, add_member_typedef)) {
214+
mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new field 0x%08x to new class 0x%08x", log_token, add_member_typedef);
215+
pass2_context_add_skeleton_member (ctx, add_member_typedef, log_token);

0 commit comments

Comments
 (0)