From 86dd3ae74f8cd7054a7000e107a02f6f342cb519 Mon Sep 17 00:00:00 2001 From: Michael Voorhees Date: Wed, 5 Oct 2022 10:21:55 -0400 Subject: [PATCH] Fix a StackOverflowException reading windows runtime assemblies. 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 https://github.com/jbevain/cecil/pull/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 (); ``` 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` --- Mono.Cecil/AssemblyReader.cs | 2 +- Mono.Cecil/WindowsRuntimeProjections.cs | 10 +-- Test/Mono.Cecil.Tests/ImageReadTests.cs | 2 +- .../WindowsRuntimeAssemblyResolver.cs | 29 +++++-- .../WindowsRuntimeProjectionsTests.cs | 87 +++++++++++++------ 5 files changed, 87 insertions(+), 43 deletions(-) diff --git a/Mono.Cecil/AssemblyReader.cs b/Mono.Cecil/AssemblyReader.cs index 0756fa884..4564071e8 100644 --- a/Mono.Cecil/AssemblyReader.cs +++ b/Mono.Cecil/AssemblyReader.cs @@ -2508,7 +2508,7 @@ public Collection ReadCustomAttributes (ICustomAttributeProvide if (module.IsWindowsMetadata ()) foreach (var custom_attribute in custom_attributes) - WindowsRuntimeProjections.Project (owner, custom_attribute); + WindowsRuntimeProjections.Project (owner, custom_attributes, custom_attribute); return custom_attributes; } diff --git a/Mono.Cecil/WindowsRuntimeProjections.cs b/Mono.Cecil/WindowsRuntimeProjections.cs index 6e83ba66e..b96a891b2 100644 --- a/Mono.Cecil/WindowsRuntimeProjections.cs +++ b/Mono.Cecil/WindowsRuntimeProjections.cs @@ -241,7 +241,7 @@ public static void Project (TypeDefinition type) treatment = TypeDefinitionTreatment.PrefixWindowsRuntimeName; if (treatment == TypeDefinitionTreatment.PrefixWindowsRuntimeName || treatment == TypeDefinitionTreatment.NormalType) - if (!type.IsInterface && HasAttribute (type, "Windows.UI.Xaml", "TreatAsAbstractComposableClassAttribute")) + if (!type.IsInterface && HasAttribute (type.CustomAttributes, "Windows.UI.Xaml", "TreatAsAbstractComposableClassAttribute")) treatment |= TypeDefinitionTreatment.Abstract; } else if (metadata_kind == MetadataKind.ManagedWindowsMetadata && IsClrImplementationType (type)) @@ -860,7 +860,7 @@ AssemblyNameReference GetAssemblyReference (string name) throw new Exception (); } - public static void Project (ICustomAttributeProvider owner, CustomAttribute attribute) + public static void Project (ICustomAttributeProvider owner, Collection owner_attributes, CustomAttribute attribute) { if (!IsWindowsAttributeUsageAttribute (owner, attribute)) return; @@ -876,7 +876,7 @@ public static void Project (ICustomAttributeProvider owner, CustomAttribute attr } if (treatment == CustomAttributeValueTreatment.None) { - var multiple = HasAttribute (type, "Windows.Foundation.Metadata", "AllowMultipleAttribute"); + var multiple = HasAttribute (owner_attributes, "Windows.Foundation.Metadata", "AllowMultipleAttribute"); treatment = multiple ? CustomAttributeValueTreatment.AllowMultiple : CustomAttributeValueTreatment.AllowSingle; } @@ -905,9 +905,9 @@ static bool IsWindowsAttributeUsageAttribute (ICustomAttributeProvider owner, Cu return declaring_type.Name == "AttributeUsageAttribute" && declaring_type.Namespace == /*"Windows.Foundation.Metadata"*/"System"; } - static bool HasAttribute (TypeDefinition type, string @namespace, string name) + static bool HasAttribute (Collection attributes, string @namespace, string name) { - foreach (var attribute in type.CustomAttributes) { + foreach (var attribute in attributes) { var attribute_type = attribute.AttributeType; if (attribute_type.Name == name && attribute_type.Namespace == @namespace) return true; diff --git a/Test/Mono.Cecil.Tests/ImageReadTests.cs b/Test/Mono.Cecil.Tests/ImageReadTests.cs index f663b60ca..fb5c558c6 100644 --- a/Test/Mono.Cecil.Tests/ImageReadTests.cs +++ b/Test/Mono.Cecil.Tests/ImageReadTests.cs @@ -182,7 +182,7 @@ public void MetroAssembly () [Test] public void WindowsRuntimeComponentAssembly () { - var resolver = WindowsRuntimeAssemblyResolver.CreateInstance (); + var resolver = WindowsRuntimeAssemblyResolver.CreateInstance (applyWindowsRuntimeProjections: false); if (resolver == null) return; diff --git a/Test/Mono.Cecil.Tests/WindowsRuntimeAssemblyResolver.cs b/Test/Mono.Cecil.Tests/WindowsRuntimeAssemblyResolver.cs index 65616294f..4f6aaf6ec 100644 --- a/Test/Mono.Cecil.Tests/WindowsRuntimeAssemblyResolver.cs +++ b/Test/Mono.Cecil.Tests/WindowsRuntimeAssemblyResolver.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Linq; using Microsoft.Win32; namespace Mono.Cecil.Tests { @@ -10,12 +11,12 @@ public class WindowsRuntimeAssemblyResolver : DefaultAssemblyResolver { readonly Dictionary assemblies = new Dictionary (); - public static WindowsRuntimeAssemblyResolver CreateInstance () + public static WindowsRuntimeAssemblyResolver CreateInstance (bool applyWindowsRuntimeProjections) { if (Platform.OnMono) return null; try { - return new WindowsRuntimeAssemblyResolver (); + return new WindowsRuntimeAssemblyResolver (applyWindowsRuntimeProjections); } catch { return null; } @@ -30,20 +31,30 @@ public override AssemblyDefinition Resolve (AssemblyNameReference name) return base.Resolve (name); } - private WindowsRuntimeAssemblyResolver () + private WindowsRuntimeAssemblyResolver (bool applyWindowsRuntimeProjections) { - LoadWindowsSdk ("v8.1", "8.1", (installationFolder) => { - var fileName = Path.Combine (installationFolder, @"References\CommonConfiguration\Neutral\Annotated\Windows.winmd"); - var assembly = AssemblyDefinition.ReadAssembly (fileName); - Register (assembly); - }); + var readerParameters = new ReaderParameters { + ApplyWindowsRuntimeProjections = applyWindowsRuntimeProjections + }; LoadWindowsSdk ("v10.0", "10", (installationFolder) => { var referencesFolder = Path.Combine (installationFolder, "References"); var assemblies = Directory.GetFiles (referencesFolder, "*.winmd", SearchOption.AllDirectories); + + var latestVersionDir = Directory.GetDirectories (Path.Combine (installationFolder, "UnionMetadata")) + .Where (d => Path.GetFileName (d) != "Facade") + .OrderBy (d => Path.GetFileName (d)) + .Last (); + + var windowsWinMdPath = Path.Combine (latestVersionDir, "Windows.winmd"); + if (!File.Exists (windowsWinMdPath)) + throw new FileNotFoundException (windowsWinMdPath); + + var windowsWinmdAssembly = AssemblyDefinition.ReadAssembly (windowsWinMdPath, readerParameters); + Register (windowsWinmdAssembly); foreach (var assemblyPath in assemblies) { - var assembly = AssemblyDefinition.ReadAssembly (assemblyPath); + var assembly = AssemblyDefinition.ReadAssembly (assemblyPath, readerParameters); Register (assembly); } }); diff --git a/Test/Mono.Cecil.Tests/WindowsRuntimeProjectionsTests.cs b/Test/Mono.Cecil.Tests/WindowsRuntimeProjectionsTests.cs index 538727329..ae443930b 100644 --- a/Test/Mono.Cecil.Tests/WindowsRuntimeProjectionsTests.cs +++ b/Test/Mono.Cecil.Tests/WindowsRuntimeProjectionsTests.cs @@ -17,19 +17,21 @@ public abstract class BaseWindowsRuntimeProjectionsTests : BaseTestFixture { protected abstract string [] ManagedClassTypeNames { get; } protected abstract string [] CustomListTypeNames { get; } - [Test] - public void CanReadMetadataType () + [TestCase (true)] + [TestCase (false)] + public void CanReadMetadataType (bool readSdkAssembliesWithApplyWindowsRuntimeProjections) { if (Platform.OnMono) return; TestModule (ModuleName, (module) => { Assert.AreEqual (ExpectedMetadataKind, module.MetadataKind); - }, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (), applyWindowsRuntimeProjections: true); + }, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (readSdkAssembliesWithApplyWindowsRuntimeProjections), applyWindowsRuntimeProjections: true); } - [Test] - public void CanProjectParametersAndReturnTypes () + [TestCase (true)] + [TestCase (false)] + public void CanProjectParametersAndReturnTypes (bool readSdkAssembliesWithApplyWindowsRuntimeProjections) { if (Platform.OnMono) return; @@ -48,11 +50,12 @@ public void CanProjectParametersAndReturnTypes () Assert.AreEqual (listSetter.Parameters.Count, 1); Assert.AreEqual (listSetter.Parameters [0].ParameterType.FullName, "System.Collections.Generic.IList`1"); } - }, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (), applyWindowsRuntimeProjections: true); + }, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (readSdkAssembliesWithApplyWindowsRuntimeProjections), applyWindowsRuntimeProjections: true); } - [Test] - public void CanProjectInterfaces () + [TestCase (true)] + [TestCase (false)] + public void CanProjectInterfaces (bool readSdkAssembliesWithApplyWindowsRuntimeProjections) { if (Platform.OnMono) return; @@ -64,16 +67,41 @@ public void CanProjectInterfaces () Assert.IsNotNull (type.Interfaces.SingleOrDefault (i => i.InterfaceType.FullName == "System.Collections.Generic.IList`1")); Assert.IsNotNull (type.Interfaces.SingleOrDefault (i => i.InterfaceType.FullName == "System.Collections.Generic.IEnumerable`1")); } - }, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (), applyWindowsRuntimeProjections: true); + }, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (readSdkAssembliesWithApplyWindowsRuntimeProjections), applyWindowsRuntimeProjections: true); + } + + /// + /// This test exists to verify a StackOverflowException that started happening with https://github.com/jbevain/cecil/pull/843 + /// and was fixed by https://github.com/jbevain/cecil/pull/879 + /// + /// The windows runtime sdk assemblies must be read with ApplyWindowsRuntimeProjections in order for the StackOverflowException to happen + /// + [TestCase (true)] + [TestCase (false)] + public void CanAvoidCircleAttributeReading (bool readSdkAssembliesWithApplyWindowsRuntimeProjections) + { + if (Platform.OnMono) + return; + + TestModule (ModuleName, (module) => { + + var windowsWinMd = module.AssemblyResolver.Resolve (new AssemblyNameReference ("Windows", null)); + + var problematicType = windowsWinMd.MainModule.GetType ("Windows.Foundation.Metadata.ActivatableAttribute"); + + Assert.Greater (problematicType.CustomAttributes.Count, 0, "Expected one or more attributes"); + + }, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (readSdkAssembliesWithApplyWindowsRuntimeProjections), applyWindowsRuntimeProjections: true); } - [Test] - public void CanStripType () + [TestCase (true)] + [TestCase (false)] + public void CanStripType (bool readSdkAssembliesWithApplyWindowsRuntimeProjections) { if (Platform.OnMono) return; - var assemblyResolver = WindowsRuntimeAssemblyResolver.CreateInstance (); + var assemblyResolver = WindowsRuntimeAssemblyResolver.CreateInstance (readSdkAssembliesWithApplyWindowsRuntimeProjections); TestModule (ModuleName, (originalModule) => { var types = CustomListTypeNames.Select (typeName => originalModule.Types.Single (t => t.Name == typeName)).ToArray (); @@ -107,8 +135,9 @@ public class ManagedWindowsRuntimeProjectionsTests : BaseWindowsRuntimeProjectio protected override string [] CustomListTypeNames { get { return new [] { "CustomList", "CustomList" }; } } - [Test] - public void CanProjectClasses () + [TestCase (true)] + [TestCase (false)] + public void CanProjectClasses (bool readSdkAssembliesWithApplyWindowsRuntimeProjections) { if (Platform.OnMono) return; @@ -129,11 +158,12 @@ public void CanProjectClasses () var winrtSomeOtherClassType = module.Types.Single (t => t.Name == "SomeOtherClass"); Assert.AreEqual ("SomeOtherClass", winrtSomeOtherClassType.WindowsRuntimeProjection.Name); Assert.AreEqual (TypeDefinitionTreatment.PrefixWindowsRuntimeName, winrtSomeOtherClassType.WindowsRuntimeProjection.Treatment); - }, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (), applyWindowsRuntimeProjections: true); + }, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (readSdkAssembliesWithApplyWindowsRuntimeProjections), applyWindowsRuntimeProjections: true); } - [Test] - public void VerifyTypeReferenceToProjectedTypeInAttributeArgumentReferencesUnmangledTypeName() + [TestCase (true)] + [TestCase (false)] + public void VerifyTypeReferenceToProjectedTypeInAttributeArgumentReferencesUnmangledTypeName(bool readSdkAssembliesWithApplyWindowsRuntimeProjections) { if (Platform.OnMono) return; @@ -147,7 +177,7 @@ public void VerifyTypeReferenceToProjectedTypeInAttributeArgumentReferencesUnman var attributeArgument = (TypeReference)attribute.ConstructorArguments[0].Value; Assert.AreEqual("ManagedWinmd.ClassWithAsyncMethod/d__0", attributeArgument.FullName); - }, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance(), applyWindowsRuntimeProjections: true); + }, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance(readSdkAssembliesWithApplyWindowsRuntimeProjections), applyWindowsRuntimeProjections: true); } } @@ -162,8 +192,9 @@ public class NativeWindowsRuntimeProjectionsTests : BaseWindowsRuntimeProjection protected override string [] CustomListTypeNames { get { return new [] { "CustomList" }; } } - [Test] - public void CanProjectAndRedirectInterfaces () + [TestCase (true)] + [TestCase (false)] + public void CanProjectAndRedirectInterfaces (bool readSdkAssembliesWithApplyWindowsRuntimeProjections) { if (Platform.OnMono) return; @@ -213,11 +244,12 @@ public void CanProjectAndRedirectInterfaces () Assert.AreEqual (0, customPropertySetClass.Interfaces[6].CustomAttributes.Count); Assert.AreEqual ("Windows.Foundation.Collections.IIterable`1>", customPropertySetClass.Interfaces[6].InterfaceType.FullName); - }, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (), applyWindowsRuntimeProjections: true); + }, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (readSdkAssembliesWithApplyWindowsRuntimeProjections), applyWindowsRuntimeProjections: true); } - [Test] - public void CanProjectInterfaceMethods () + [TestCase (true)] + [TestCase (false)] + public void CanProjectInterfaceMethods (bool readSdkAssembliesWithApplyWindowsRuntimeProjections) { if (Platform.OnMono) return; @@ -256,11 +288,12 @@ public void CanProjectInterfaceMethods () Assert.AreEqual (customListClass.Methods[25].FullName, "System.Boolean NativeWinmd.CustomList::Remove(System.Int32)"); Assert.AreEqual (customListClass.Methods[26].FullName, "System.Collections.Generic.IEnumerator`1 NativeWinmd.CustomList::GetEnumerator()"); Assert.AreEqual (customListClass.Methods[27].FullName, "System.Collections.IEnumerator NativeWinmd.CustomList::GetEnumerator()"); - }, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (), applyWindowsRuntimeProjections: true); + }, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (readSdkAssembliesWithApplyWindowsRuntimeProjections), applyWindowsRuntimeProjections: true); } - [Test] - public void CanProjectMethodOverrides () + [TestCase (true)] + [TestCase (false)] + public void CanProjectMethodOverrides (bool readSdkAssembliesWithApplyWindowsRuntimeProjections) { if (Platform.OnMono) return; @@ -299,7 +332,7 @@ public void CanProjectMethodOverrides () Assert.AreEqual (customListClass.Methods[26].Overrides[0].FullName, "System.Collections.Generic.IEnumerator`1 System.Collections.Generic.IEnumerable`1::GetEnumerator()"); Assert.AreEqual (customListClass.Methods[27].Overrides[0].FullName, "System.Collections.IEnumerator System.Collections.IEnumerable::GetEnumerator()"); - }, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (), applyWindowsRuntimeProjections: true); + }, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (readSdkAssembliesWithApplyWindowsRuntimeProjections), applyWindowsRuntimeProjections: true); } } }