Skip to content

Commit cc48622

Browse files
mrvoorhemrvoorhe
authored
Fix a StackOverflowException reading windows runtime assemblies. (#879)
During `AssemblyReader.ReadCustomAttributes` there is a call to `WindowsRuntimeProjections.Project` ``` if (module.IsWindowsMetadata ()) foreach (var custom_attribute in custom_attributes) WindowsRuntimeProjections.Project (owner, custom_attributes, custom_attribute); ``` `WindowsRuntimeProjections.Project` would call `WindowsRuntimeProjections.HasAttribute`, which would then call `type.CustomAttributes`, which would end up back in `AssemblyReader.ReadCustomAttributes`. This would lead to a StackOverflowException. This wasn't an issue previously. My PR #843 caused this sequence of calls to start resulting in a StackOverflowException. Prior to my PR, there was a call to `metadata.RemoveCustomAttributeRange (owner);` before the call to `WindowsRuntimeProjections.Project`. This meant that when `WindowsRuntimeProjections.HasAttribute` would call `type.CustomAttributes`, we'd still end up in `AssemblyReader.ReadCustomAttributes`, however, no attributes would be found because the following if would be true and lead to returning an empty collection. ``` if (!metadata.TryGetCustomAttributeRanges (owner, out ranges)) return new Collection<CustomAttribute> (); ``` The old behavior was probably the wrong. Although I'm not certain what the tangible impact was. The fix was pretty easy. `AssemblyReader.ReadCustomAttributes` will now pass in the custom attributes to `WindowsRuntimeProjections.Project` avoiding the need to call `type.CustomAttributes`
1 parent 341fb14 commit cc48622

File tree

5 files changed

+87
-43
lines changed

5 files changed

+87
-43
lines changed

‎Mono.Cecil/AssemblyReader.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2508,7 +2508,7 @@ public Collection<CustomAttribute> ReadCustomAttributes (ICustomAttributeProvide
25082508

25092509
if (module.IsWindowsMetadata ())
25102510
foreach (var custom_attribute in custom_attributes)
2511-
WindowsRuntimeProjections.Project (owner, custom_attribute);
2511+
WindowsRuntimeProjections.Project (owner, custom_attributes, custom_attribute);
25122512

25132513
return custom_attributes;
25142514
}

‎Mono.Cecil/WindowsRuntimeProjections.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ public static void Project (TypeDefinition type)
241241
treatment = TypeDefinitionTreatment.PrefixWindowsRuntimeName;
242242

243243
if (treatment == TypeDefinitionTreatment.PrefixWindowsRuntimeName || treatment == TypeDefinitionTreatment.NormalType)
244-
if (!type.IsInterface && HasAttribute (type, "Windows.UI.Xaml", "TreatAsAbstractComposableClassAttribute"))
244+
if (!type.IsInterface && HasAttribute (type.CustomAttributes, "Windows.UI.Xaml", "TreatAsAbstractComposableClassAttribute"))
245245
treatment |= TypeDefinitionTreatment.Abstract;
246246
}
247247
else if (metadata_kind == MetadataKind.ManagedWindowsMetadata && IsClrImplementationType (type))
@@ -860,7 +860,7 @@ AssemblyNameReference GetAssemblyReference (string name)
860860
throw new Exception ();
861861
}
862862

863-
public static void Project (ICustomAttributeProvider owner, CustomAttribute attribute)
863+
public static void Project (ICustomAttributeProvider owner, Collection<CustomAttribute> owner_attributes, CustomAttribute attribute)
864864
{
865865
if (!IsWindowsAttributeUsageAttribute (owner, attribute))
866866
return;
@@ -876,7 +876,7 @@ public static void Project (ICustomAttributeProvider owner, CustomAttribute attr
876876
}
877877

878878
if (treatment == CustomAttributeValueTreatment.None) {
879-
var multiple = HasAttribute (type, "Windows.Foundation.Metadata", "AllowMultipleAttribute");
879+
var multiple = HasAttribute (owner_attributes, "Windows.Foundation.Metadata", "AllowMultipleAttribute");
880880
treatment = multiple ? CustomAttributeValueTreatment.AllowMultiple : CustomAttributeValueTreatment.AllowSingle;
881881
}
882882

@@ -905,9 +905,9 @@ static bool IsWindowsAttributeUsageAttribute (ICustomAttributeProvider owner, Cu
905905
return declaring_type.Name == "AttributeUsageAttribute" && declaring_type.Namespace == /*"Windows.Foundation.Metadata"*/"System";
906906
}
907907

908-
static bool HasAttribute (TypeDefinition type, string @namespace, string name)
908+
static bool HasAttribute (Collection<CustomAttribute> attributes, string @namespace, string name)
909909
{
910-
foreach (var attribute in type.CustomAttributes) {
910+
foreach (var attribute in attributes) {
911911
var attribute_type = attribute.AttributeType;
912912
if (attribute_type.Name == name && attribute_type.Namespace == @namespace)
913913
return true;

‎Test/Mono.Cecil.Tests/ImageReadTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ public void MetroAssembly ()
182182
[Test]
183183
public void WindowsRuntimeComponentAssembly ()
184184
{
185-
var resolver = WindowsRuntimeAssemblyResolver.CreateInstance ();
185+
var resolver = WindowsRuntimeAssemblyResolver.CreateInstance (applyWindowsRuntimeProjections: false);
186186
if (resolver == null)
187187
return;
188188

‎Test/Mono.Cecil.Tests/WindowsRuntimeAssemblyResolver.cs‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,20 @@
33
using System;
44
using System.Collections.Generic;
55
using System.IO;
6+
using System.Linq;
67
using Microsoft.Win32;
78

89
namespace Mono.Cecil.Tests {
910
public class WindowsRuntimeAssemblyResolver : DefaultAssemblyResolver {
1011

1112
readonly Dictionary<string, AssemblyDefinition> assemblies = new Dictionary<string, AssemblyDefinition> ();
1213

13-
public static WindowsRuntimeAssemblyResolver CreateInstance ()
14+
public static WindowsRuntimeAssemblyResolver CreateInstance (bool applyWindowsRuntimeProjections)
1415
{
1516
if (Platform.OnMono)
1617
return null;
1718
try {
18-
return new WindowsRuntimeAssemblyResolver ();
19+
return new WindowsRuntimeAssemblyResolver (applyWindowsRuntimeProjections);
1920
} catch {
2021
return null;
2122
}
@@ -30,20 +31,30 @@ public override AssemblyDefinition Resolve (AssemblyNameReference name)
3031
return base.Resolve (name);
3132
}
3233

33-
private WindowsRuntimeAssemblyResolver ()
34+
private WindowsRuntimeAssemblyResolver (bool applyWindowsRuntimeProjections)
3435
{
35-
LoadWindowsSdk ("v8.1", "8.1", (installationFolder) => {
36-
var fileName = Path.Combine (installationFolder, @"References\CommonConfiguration\Neutral\Annotated\Windows.winmd");
37-
var assembly = AssemblyDefinition.ReadAssembly (fileName);
38-
Register (assembly);
39-
});
36+
var readerParameters = new ReaderParameters {
37+
ApplyWindowsRuntimeProjections = applyWindowsRuntimeProjections
38+
};
4039

4140
LoadWindowsSdk ("v10.0", "10", (installationFolder) => {
4241
var referencesFolder = Path.Combine (installationFolder, "References");
4342
var assemblies = Directory.GetFiles (referencesFolder, "*.winmd", SearchOption.AllDirectories);
43+
44+
var latestVersionDir = Directory.GetDirectories (Path.Combine (installationFolder, "UnionMetadata"))
45+
.Where (d => Path.GetFileName (d) != "Facade")
46+
.OrderBy (d => Path.GetFileName (d))
47+
.Last ();
48+
49+
var windowsWinMdPath = Path.Combine (latestVersionDir, "Windows.winmd");
50+
if (!File.Exists (windowsWinMdPath))
51+
throw new FileNotFoundException (windowsWinMdPath);
52+
53+
var windowsWinmdAssembly = AssemblyDefinition.ReadAssembly (windowsWinMdPath, readerParameters);
54+
Register (windowsWinmdAssembly);
4455

4556
foreach (var assemblyPath in assemblies) {
46-
var assembly = AssemblyDefinition.ReadAssembly (assemblyPath);
57+
var assembly = AssemblyDefinition.ReadAssembly (assemblyPath, readerParameters);
4758
Register (assembly);
4859
}
4960
});

‎Test/Mono.Cecil.Tests/WindowsRuntimeProjectionsTests.cs‎

Lines changed: 60 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,21 @@ public abstract class BaseWindowsRuntimeProjectionsTests : BaseTestFixture {
1717
protected abstract string [] ManagedClassTypeNames { get; }
1818
protected abstract string [] CustomListTypeNames { get; }
1919

20-
[Test]
21-
public void CanReadMetadataType ()
20+
[TestCase (true)]
21+
[TestCase (false)]
22+
public void CanReadMetadataType (bool readSdkAssembliesWithApplyWindowsRuntimeProjections)
2223
{
2324
if (Platform.OnMono)
2425
return;
2526

2627
TestModule (ModuleName, (module) => {
2728
Assert.AreEqual (ExpectedMetadataKind, module.MetadataKind);
28-
}, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (), applyWindowsRuntimeProjections: true);
29+
}, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (readSdkAssembliesWithApplyWindowsRuntimeProjections), applyWindowsRuntimeProjections: true);
2930
}
3031

31-
[Test]
32-
public void CanProjectParametersAndReturnTypes ()
32+
[TestCase (true)]
33+
[TestCase (false)]
34+
public void CanProjectParametersAndReturnTypes (bool readSdkAssembliesWithApplyWindowsRuntimeProjections)
3335
{
3436
if (Platform.OnMono)
3537
return;
@@ -48,11 +50,12 @@ public void CanProjectParametersAndReturnTypes ()
4850
Assert.AreEqual (listSetter.Parameters.Count, 1);
4951
Assert.AreEqual (listSetter.Parameters [0].ParameterType.FullName, "System.Collections.Generic.IList`1<System.Int32>");
5052
}
51-
}, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (), applyWindowsRuntimeProjections: true);
53+
}, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (readSdkAssembliesWithApplyWindowsRuntimeProjections), applyWindowsRuntimeProjections: true);
5254
}
5355

54-
[Test]
55-
public void CanProjectInterfaces ()
56+
[TestCase (true)]
57+
[TestCase (false)]
58+
public void CanProjectInterfaces (bool readSdkAssembliesWithApplyWindowsRuntimeProjections)
5659
{
5760
if (Platform.OnMono)
5861
return;
@@ -64,16 +67,41 @@ public void CanProjectInterfaces ()
6467
Assert.IsNotNull (type.Interfaces.SingleOrDefault (i => i.InterfaceType.FullName == "System.Collections.Generic.IList`1<System.Int32>"));
6568
Assert.IsNotNull (type.Interfaces.SingleOrDefault (i => i.InterfaceType.FullName == "System.Collections.Generic.IEnumerable`1<System.Int32>"));
6669
}
67-
}, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (), applyWindowsRuntimeProjections: true);
70+
}, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (readSdkAssembliesWithApplyWindowsRuntimeProjections), applyWindowsRuntimeProjections: true);
71+
}
72+
73+
/// <summary>
74+
/// This test exists to verify a StackOverflowException that started happening with https://github.com/jbevain/cecil/pull/843
75+
/// and was fixed by https://github.com/jbevain/cecil/pull/879
76+
///
77+
/// The windows runtime sdk assemblies must be read with ApplyWindowsRuntimeProjections in order for the StackOverflowException to happen
78+
/// </summary>
79+
[TestCase (true)]
80+
[TestCase (false)]
81+
public void CanAvoidCircleAttributeReading (bool readSdkAssembliesWithApplyWindowsRuntimeProjections)
82+
{
83+
if (Platform.OnMono)
84+
return;
85+
86+
TestModule (ModuleName, (module) => {
87+
88+
var windowsWinMd = module.AssemblyResolver.Resolve (new AssemblyNameReference ("Windows", null));
89+
90+
var problematicType = windowsWinMd.MainModule.GetType ("Windows.Foundation.Metadata.ActivatableAttribute");
91+
92+
Assert.Greater (problematicType.CustomAttributes.Count, 0, "Expected one or more attributes");
93+
94+
}, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (readSdkAssembliesWithApplyWindowsRuntimeProjections), applyWindowsRuntimeProjections: true);
6895
}
6996

70-
[Test]
71-
public void CanStripType ()
97+
[TestCase (true)]
98+
[TestCase (false)]
99+
public void CanStripType (bool readSdkAssembliesWithApplyWindowsRuntimeProjections)
72100
{
73101
if (Platform.OnMono)
74102
return;
75103

76-
var assemblyResolver = WindowsRuntimeAssemblyResolver.CreateInstance ();
104+
var assemblyResolver = WindowsRuntimeAssemblyResolver.CreateInstance (readSdkAssembliesWithApplyWindowsRuntimeProjections);
77105

78106
TestModule (ModuleName, (originalModule) => {
79107
var types = CustomListTypeNames.Select (typeName => originalModule.Types.Single (t => t.Name == typeName)).ToArray ();
@@ -107,8 +135,9 @@ public class ManagedWindowsRuntimeProjectionsTests : BaseWindowsRuntimeProjectio
107135

108136
protected override string [] CustomListTypeNames { get { return new [] { "CustomList", "<WinRT>CustomList" }; } }
109137

110-
[Test]
111-
public void CanProjectClasses ()
138+
[TestCase (true)]
139+
[TestCase (false)]
140+
public void CanProjectClasses (bool readSdkAssembliesWithApplyWindowsRuntimeProjections)
112141
{
113142
if (Platform.OnMono)
114143
return;
@@ -129,11 +158,12 @@ public void CanProjectClasses ()
129158
var winrtSomeOtherClassType = module.Types.Single (t => t.Name == "<WinRT>SomeOtherClass");
130159
Assert.AreEqual ("SomeOtherClass", winrtSomeOtherClassType.WindowsRuntimeProjection.Name);
131160
Assert.AreEqual (TypeDefinitionTreatment.PrefixWindowsRuntimeName, winrtSomeOtherClassType.WindowsRuntimeProjection.Treatment);
132-
}, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (), applyWindowsRuntimeProjections: true);
161+
}, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (readSdkAssembliesWithApplyWindowsRuntimeProjections), applyWindowsRuntimeProjections: true);
133162
}
134163

135-
[Test]
136-
public void VerifyTypeReferenceToProjectedTypeInAttributeArgumentReferencesUnmangledTypeName()
164+
[TestCase (true)]
165+
[TestCase (false)]
166+
public void VerifyTypeReferenceToProjectedTypeInAttributeArgumentReferencesUnmangledTypeName(bool readSdkAssembliesWithApplyWindowsRuntimeProjections)
137167
{
138168
if (Platform.OnMono)
139169
return;
@@ -147,7 +177,7 @@ public void VerifyTypeReferenceToProjectedTypeInAttributeArgumentReferencesUnman
147177
var attributeArgument = (TypeReference)attribute.ConstructorArguments[0].Value;
148178

149179
Assert.AreEqual("ManagedWinmd.ClassWithAsyncMethod/<DoStuffAsync>d__0", attributeArgument.FullName);
150-
}, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance(), applyWindowsRuntimeProjections: true);
180+
}, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance(readSdkAssembliesWithApplyWindowsRuntimeProjections), applyWindowsRuntimeProjections: true);
151181
}
152182
}
153183

@@ -162,8 +192,9 @@ public class NativeWindowsRuntimeProjectionsTests : BaseWindowsRuntimeProjection
162192

163193
protected override string [] CustomListTypeNames { get { return new [] { "CustomList" }; } }
164194

165-
[Test]
166-
public void CanProjectAndRedirectInterfaces ()
195+
[TestCase (true)]
196+
[TestCase (false)]
197+
public void CanProjectAndRedirectInterfaces (bool readSdkAssembliesWithApplyWindowsRuntimeProjections)
167198
{
168199
if (Platform.OnMono)
169200
return;
@@ -213,11 +244,12 @@ public void CanProjectAndRedirectInterfaces ()
213244
Assert.AreEqual (0, customPropertySetClass.Interfaces[6].CustomAttributes.Count);
214245
Assert.AreEqual ("Windows.Foundation.Collections.IIterable`1<System.Collections.Generic.KeyValuePair`2<System.String,System.Object>>", customPropertySetClass.Interfaces[6].InterfaceType.FullName);
215246

216-
}, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (), applyWindowsRuntimeProjections: true);
247+
}, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (readSdkAssembliesWithApplyWindowsRuntimeProjections), applyWindowsRuntimeProjections: true);
217248
}
218249

219-
[Test]
220-
public void CanProjectInterfaceMethods ()
250+
[TestCase (true)]
251+
[TestCase (false)]
252+
public void CanProjectInterfaceMethods (bool readSdkAssembliesWithApplyWindowsRuntimeProjections)
221253
{
222254
if (Platform.OnMono)
223255
return;
@@ -256,11 +288,12 @@ public void CanProjectInterfaceMethods ()
256288
Assert.AreEqual (customListClass.Methods[25].FullName, "System.Boolean NativeWinmd.CustomList::Remove(System.Int32)");
257289
Assert.AreEqual (customListClass.Methods[26].FullName, "System.Collections.Generic.IEnumerator`1<System.Int32> NativeWinmd.CustomList::GetEnumerator()");
258290
Assert.AreEqual (customListClass.Methods[27].FullName, "System.Collections.IEnumerator NativeWinmd.CustomList::GetEnumerator()");
259-
}, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (), applyWindowsRuntimeProjections: true);
291+
}, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (readSdkAssembliesWithApplyWindowsRuntimeProjections), applyWindowsRuntimeProjections: true);
260292
}
261293

262-
[Test]
263-
public void CanProjectMethodOverrides ()
294+
[TestCase (true)]
295+
[TestCase (false)]
296+
public void CanProjectMethodOverrides (bool readSdkAssembliesWithApplyWindowsRuntimeProjections)
264297
{
265298
if (Platform.OnMono)
266299
return;
@@ -299,7 +332,7 @@ public void CanProjectMethodOverrides ()
299332
Assert.AreEqual (customListClass.Methods[26].Overrides[0].FullName, "System.Collections.Generic.IEnumerator`1<T> System.Collections.Generic.IEnumerable`1<System.Int32>::GetEnumerator()");
300333
Assert.AreEqual (customListClass.Methods[27].Overrides[0].FullName, "System.Collections.IEnumerator System.Collections.IEnumerable::GetEnumerator()");
301334

302-
}, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (), applyWindowsRuntimeProjections: true);
335+
}, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (readSdkAssembliesWithApplyWindowsRuntimeProjections), applyWindowsRuntimeProjections: true);
303336
}
304337
}
305338
}

0 commit comments

Comments
 (0)