Skip to content

Commit

Permalink
Fix a StackOverflowException reading windows runtime assemblies. (#879)
Browse files Browse the repository at this point in the history 
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`
  • Loading branch information
@mrvoorhe
mrvoorhe authored Nov 16, 2022
1 parent 341fb14 commit cc48622
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 43 deletions.
2 changes: 1 addition & 1 deletion Mono.Cecil/AssemblyReader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2508,7 +2508,7 @@ public Collection<CustomAttribute> 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;
}
Expand Down
10 changes: 5 additions & 5 deletions Mono.Cecil/WindowsRuntimeProjections.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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<CustomAttribute> owner_attributes, CustomAttribute attribute)
{
if (!IsWindowsAttributeUsageAttribute (owner, attribute))
return;
Expand All @@ -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;
}

Expand Down Expand Up @@ -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<CustomAttribute> 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;
Expand Down
2 changes: 1 addition & 1 deletion Test/Mono.Cecil.Tests/ImageReadTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
29 changes: 20 additions & 9 deletions Test/Mono.Cecil.Tests/WindowsRuntimeAssemblyResolver.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,20 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using Microsoft.Win32;

namespace Mono.Cecil.Tests {
public class WindowsRuntimeAssemblyResolver : DefaultAssemblyResolver {

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

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;
}
Expand All @@ -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);
}
});
Expand Down
87 changes: 60 additions & 27 deletions Test/Mono.Cecil.Tests/WindowsRuntimeProjectionsTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<System.Int32>");
}
}, 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;
Expand All @@ -64,16 +67,41 @@ public void CanProjectInterfaces ()
Assert.IsNotNull (type.Interfaces.SingleOrDefault (i => i.InterfaceType.FullName == "System.Collections.Generic.IList`1<System.Int32>"));
Assert.IsNotNull (type.Interfaces.SingleOrDefault (i => i.InterfaceType.FullName == "System.Collections.Generic.IEnumerable`1<System.Int32>"));
}
}, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (), applyWindowsRuntimeProjections: true);
}, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance (readSdkAssembliesWithApplyWindowsRuntimeProjections), applyWindowsRuntimeProjections: true);
}

/// <summary>
/// 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
/// </summary>
[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 ();
Expand Down Expand Up @@ -107,8 +135,9 @@ public class ManagedWindowsRuntimeProjectionsTests : BaseWindowsRuntimeProjectio

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

[Test]
public void CanProjectClasses ()
[TestCase (true)]
[TestCase (false)]
public void CanProjectClasses (bool readSdkAssembliesWithApplyWindowsRuntimeProjections)
{
if (Platform.OnMono)
return;
Expand All @@ -129,11 +158,12 @@ public void CanProjectClasses ()
var winrtSomeOtherClassType = module.Types.Single (t => t.Name == "<WinRT>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;
Expand All @@ -147,7 +177,7 @@ public void VerifyTypeReferenceToProjectedTypeInAttributeArgumentReferencesUnman
var attributeArgument = (TypeReference)attribute.ConstructorArguments[0].Value;
Assert.AreEqual("ManagedWinmd.ClassWithAsyncMethod/<DoStuffAsync>d__0", attributeArgument.FullName);
}, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance(), applyWindowsRuntimeProjections: true);
}, verify: false, assemblyResolver: WindowsRuntimeAssemblyResolver.CreateInstance(readSdkAssembliesWithApplyWindowsRuntimeProjections), applyWindowsRuntimeProjections: true);
}
}

Expand All @@ -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;
Expand Down Expand Up @@ -213,11 +244,12 @@ public void CanProjectAndRedirectInterfaces ()
Assert.AreEqual (0, customPropertySetClass.Interfaces[6].CustomAttributes.Count);
Assert.AreEqual ("Windows.Foundation.Collections.IIterable`1<System.Collections.Generic.KeyValuePair`2<System.String,System.Object>>", 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;
Expand Down Expand Up @@ -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<System.Int32> 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;
Expand Down Expand Up @@ -299,7 +332,7 @@ public void CanProjectMethodOverrides ()
Assert.AreEqual (customListClass.Methods[26].Overrides[0].FullName, "System.Collections.Generic.IEnumerator`1<T> System.Collections.Generic.IEnumerable`1<System.Int32>::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);
}
}
}
Expand Down

0 comments on commit cc48622

Please sign in to comment.