Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix invalid cast exception with stream APIs and make code fixer emit diagnostics as warnings #1654

Merged
merged 5 commits into from
Jul 2, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions docs/aot.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ As of CsWinRT **2.1.0-preview** and Windows SDK projection 10.0.<sdk_ver>.**35-p

## Consuming projected types and being AOT compatible

If your app or library has non-WinRT classes that implement C#/WinRT projected interfaces or built-in .NET interfaces that are mapped to WinRT types and are passed across the ABI to other WinRT functions, the class needs to be marked `partial` in order to be AOT compatible. Marking it `partial` allows the source generator distributed with C#/WinRT to add an attribute to the class which has the necessary logic to produce the WinRT vtable for it in a way that is both trimming and AOT compatible.
If your app or library has non-WinRT classes that implement C#/WinRT projected interfaces or built-in .NET interfaces that are mapped to WinRT types and are passed across the ABI to other WinRT functions, the class needs to be marked `partial` in order to be AOT compatible. Marking it `partial` allows the source generator distributed with C#/WinRT to add an attribute to the class which has the necessary logic to produce the WinRT vtable for it in a way that is both trimming and AOT compatible. To help with this, there is a `code fixer` that will warn you when such types are not marked `partial`. The deafult `warning level` (`CsWinRTAotWarningLevel`) for this code fixer is `1` which is to `not warn for scenarios involving only built-in types`. But if you pass types implementing only built-in interfaces that are mapped to WinRT across the WinRT ABI, it recommended to set the warning level to `2` and mark such types `partial` or suppress the warning if those types are not passed across the WinRT ABI.
manodasanW marked this conversation as resolved.
Show resolved Hide resolved

The source generator also detects other scenarios such as boxing of arrays and instantiations of generic WinRT types for which it generates code that will allow the WinRT vtable for it to be looked up when passed across the WinRT ABI.

Expand All @@ -18,11 +18,9 @@ There are a couple issues related to our AOT support that are known with the cur

1. If any built-in .NET private types implementing mapped WinRT interfaces are passed across the WinRT ABI, they will not be detected by the source generator and thereby not be AOT compatible. You can workaround this by either avoiding passing them across the ABI or by registering your own WinRT vtable lookup function using `WinRT.ComWrappersSupport.RegisterTypeComInterfaceEntriesLookup` and `WinRT.ComWrappersSupport.RegisterTypeRuntimeClassNameLookup`. An example of this can be seen [here](https://github.com/manodasanW/WinUI-Gallery/blob/16ed717700b929dcb6591d32a4f10cd8b102aa07/WinUIGallery/VtableInitialization.cs#L57-L75) and [here](https://github.com/manodasanW/WinUI-Gallery/blob/16ed717700b929dcb6591d32a4f10cd8b102aa07/WinUIGallery/VtableInitialization.cs#L87-L90).

2. If you have any nested C# types that you use as part of .NET collections that are passed across the WinRT ABI, the source generator generates the incorrect type name to represent it in the generated lookup function. You can workaround this issue until it is addressed by registering your own WinRT vtable lookup function using `WinRT.ComWrappersSupport.RegisterTypeComInterfaceEntriesLookup` and `WinRT.ComWrappersSupport.RegisterTypeRuntimeClassNameLookup`. An example of this can be seen [here](https://github.com/manodasanW/WinUI-Gallery/blob/16ed717700b929dcb6591d32a4f10cd8b102aa07/WinUIGallery/VtableInitialization.cs#L21-L53) and [here](https://github.com/manodasanW/WinUI-Gallery/blob/16ed717700b929dcb6591d32a4f10cd8b102aa07/WinUIGallery/VtableInitialization.cs#L83-L86).
2. In WinUI apps, you might run into a race condition which triggers a hang during .NET GC needing to restart the app. This is the result of an exception being thrown unexpectedly during GC due to an invalid handle.

3. In WinUI apps, you might run into a race condition which triggers a hang during .NET GC needing to restart the app. This is the result of an exception being thrown unexpectedly during GC due to an invalid handle.

4. If you have an app or library that just consumes WinRT types and doesn't generate a projection, it still needs a CsWinRT package reference in order for the source generator to run on it and make it AOT compatible. We will be looking at addressing this to avoid this requirement for the official version.
3. If you have an app or library that just consumes WinRT types and doesn't generate a projection, it still needs a CsWinRT package reference in order for the source generator to run on it and make it AOT compatible. We will be looking at addressing this to avoid this requirement for the official version.

## Known issues when publishing for JIT

Expand Down
19 changes: 18 additions & 1 deletion nuget/Microsoft.Windows.CsWinRT.targets
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ Copyright (C) Microsoft Corporation. All rights reserved.
<CompilerVisibleProperty Include="CsWinRTRcwFactoryFallbackGeneratorForceOptOut" />
<CompilerVisibleProperty Include="CsWinRTCcwLookupTableGeneratorEnabled" />
<CompilerVisibleProperty Include="CsWinRTMergeReferencedActivationFactories" />
<CompilerVisibleProperty Include="CsWinRTAotWarningLevel" />
</ItemGroup>

<Import Project="$(MSBuildThisFileDirectory)Microsoft.Windows.CsWinRT.Embedded.targets" Condition="'$(CsWinRTEmbedded)' == 'true'"/>
Expand Down Expand Up @@ -100,7 +101,23 @@ Copyright (C) Microsoft Corporation. All rights reserved.
'$(OutputType)' == 'Library' and
'$(CsWinRTHasAnyIncludes)' == 'true'">false</CsWinRTCcwLookupTableGeneratorEnabled>
<CsWinRTCcwLookupTableGeneratorEnabled Condition="'$(CsWinRTCcwLookupTableGeneratorEnabled)' == ''">true</CsWinRTCcwLookupTableGeneratorEnabled>
</PropertyGroup>

<!--
CsWinRTAotWarningLevel decides the level of warning from the AOT analyzer.
0: No warnings
*1: Warnings for scenarios involving WinRT types that are not built-in system types mapped to WinRT.
2: Level 1 warnings and warnings for scenarios involving built-in system types mapped to WinRT.
This is not intended to be ran on projections, so we try to detect that and disable it if a projection is being generated.
Level 1 is currently the default to avoid noise from built-in system types that might not be used in WinRT scenarios
but level 2 should be enabled and the warnings from it should be evaluated to ensure AOT and trimming compatibility.
Level 2 might be made the default in the future.
-->
<CsWinRTAotWarningLevel Condition="'$(CsWinRTAotWarningLevel)' == '' and
'$(CsWinRTGenerateProjection)' == 'true' and
'$(OutputType)' == 'Library' and
'$(CsWinRTHasAnyIncludes)' == 'true'">0</CsWinRTAotWarningLevel>
<CsWinRTAotWarningLevel Condition="'$(CsWinRTAotWarningLevel)' == ''">1</CsWinRTAotWarningLevel>
</PropertyGroup>
</Target>

<!-- Remove WinRT.Host.dll and WinRT.Host.Shim.dll references -->
Expand Down
1 change: 1 addition & 0 deletions nuget/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ C#/WinRT behavior can be customized with these project properties:
| CsWinRTRcwFactoryFallbackGeneratorForceOptIn | true \| *false | Forces the RCW factory fallback generator to be enabled (it only runs on .exe projects by default) |
| CsWinRTRcwFactoryFallbackGeneratorForceOptOut | true \| *false | Forces the RCW factory fallback generator to be disabled (overrides "ForceOptIn" as well) |
| CsWinRTMergeReferencedActivationFactories | true \| *false | Makes the native `DllGetActivationFactory` exported function for AOT scenarios also forward the activation call to all referenced WinRT components, allowing them to all be merged into a single executable or shared library |
| CsWinRTAotWarningLevel | 0 \| *1 \| 2 | Specifies the warning level used by the code fixer for trimming and AOT compat (0 - no warnings, 1 - warnings for scenarios involving non built-in types, warnings for all scenarios) |
manodasanW marked this conversation as resolved.
Show resolved Hide resolved
\*Default value

**If CsWinRTFilters is not defined, the following effective value is used:
Expand Down
38 changes: 38 additions & 0 deletions src/Authoring/WinRT.SourceGenerator/Helper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,17 @@ public static bool IsCsWinRTCcwLookupTableGeneratorEnabled(this AnalyzerConfigOp
return false;
}

public static int GetCsWinRTAotWarningLevel(this AnalyzerConfigOptionsProvider provider)
{
if (provider.GlobalOptions.TryGetValue("build_property.CsWinRTAotWarningLevel", out var csWinRTAotWarningLevelStr) &&
int.TryParse(csWinRTAotWarningLevelStr, out var csWinRTAotWarningLevel))
{
return csWinRTAotWarningLevel;
}

return 0;
}

public static bool ShouldGenerateWinMDOnly(this GeneratorExecutionContext context)
{
if (context.AnalyzerConfigOptions.GlobalOptions.TryGetValue("build_property.CsWinRTGenerateWinMDOnly", out var CsWinRTGenerateWinMDOnlyStr))
Expand Down Expand Up @@ -302,6 +313,33 @@ type is INamedTypeSymbol namedType &&
return isProjectedType;
}

// Assuming a type is a WinRT type, this determines whether it is a WinRT type from custom type mappings.
// i.e Whether it is a built-in type that is also a WinRT type.
public static bool IsCustomMappedType(ISymbol type, TypeMapper mapper)
{
if (IsFundamentalType(type))
{
return true;
}

bool isCustomMappedType = false;
if (type.ContainingNamespace != null)
{
isCustomMappedType = mapper.HasMappingForType(string.Join(".", type.ContainingNamespace.ToDisplayString(), type.MetadataName));
}

// Ensure all generic parameters are WinRT types.
if (isCustomMappedType &&
type is INamedTypeSymbol namedType &&
namedType.IsGenericType &&
!namedType.IsDefinition)
{
isCustomMappedType = namedType.TypeArguments.All(t => IsCustomMappedType(t, mapper));
}

return isCustomMappedType;
}

// Checks if the interface references any internal types (either the interface itself or within its generic types).
public static bool IsInternalInterfaceFromReferences(INamedTypeSymbol iface, IAssemblySymbol currentAssembly)
{
Expand Down
13 changes: 9 additions & 4 deletions src/Authoring/WinRT.SourceGenerator/WinRTAotCodeFixer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ namespace WinRT.SourceGenerator
[DiagnosticAnalyzer(LanguageNames.CSharp), Shared]
public sealed class WinRTAotDiagnosticAnalyzer : DiagnosticAnalyzer
{
private static ImmutableArray<DiagnosticDescriptor> _supportedDiagnostics = ImmutableArray.Create(WinRTRules.ClassNotAotCompatible);
private static ImmutableArray<DiagnosticDescriptor> _supportedDiagnostics = ImmutableArray.Create(WinRTRules.ClassNotAotCompatibleWarning, WinRTRules.ClassNotAotCompatibleInfo);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => _supportedDiagnostics;

public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
context.EnableConcurrentExecution();

context.RegisterCompilationStartAction(static context =>
Expand All @@ -43,6 +43,7 @@ public override void Initialize(AnalysisContext context)
}

var typeMapper = new TypeMapper(context.Options.AnalyzerConfigOptionsProvider.GlobalOptions.GetUIXamlProjectionsMode());
var csWinRTAotWarningLevel = context.Options.AnalyzerConfigOptionsProvider.GetCsWinRTAotWarningLevel();

context.RegisterSymbolAction(context =>
{
Expand All @@ -63,7 +64,11 @@ public override void Initialize(AnalysisContext context)
{
if (GeneratorHelper.IsWinRTType(iface, winrtTypeAttribute, typeMapper, isComponentProject, context.Compilation.Assembly))
{
context.ReportDiagnostic(Diagnostic.Create(WinRTRules.ClassNotAotCompatible, namedType.Locations[0], namedType.Name));
// Based on the warning level, emit as a warning or as an info.
var diagnosticDescriptor = (csWinRTAotWarningLevel == 2 ||
(csWinRTAotWarningLevel == 1 && !GeneratorHelper.IsCustomMappedType(iface, typeMapper))) ?
WinRTRules.ClassNotAotCompatibleWarning : WinRTRules.ClassNotAotCompatibleInfo;
context.ReportDiagnostic(Diagnostic.Create(diagnosticDescriptor, namedType.Locations[0], namedType.Name));
return;
}
}
Expand All @@ -79,7 +84,7 @@ public sealed class WinRTAotCodeFixer : CodeFixProvider
{
private const string title = "Make type partial";

private static ImmutableArray<string> _fixableDiagnosticIds = ImmutableArray.Create(WinRTRules.ClassNotAotCompatible.Id);
private static ImmutableArray<string> _fixableDiagnosticIds = ImmutableArray.Create(WinRTRules.ClassNotAotCompatibleWarning.Id);

public override ImmutableArray<string> FixableDiagnosticIds => _fixableDiagnosticIds;

Expand Down
13 changes: 10 additions & 3 deletions src/Authoring/WinRT.SourceGenerator/WinRTRules.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ public class WinRTRules
/// <param name="title">string, a few words generally describing the diagnostic</param>
/// <param name="messageFormat">string, describes the diagnostic -- formatted with {0}, ... --
/// such that data can be passed in for the code the diagnostic is reported for</param>
private static DiagnosticDescriptor MakeRule(string id, string title, string messageFormat, bool isError = true)
private static DiagnosticDescriptor MakeRule(string id, string title, string messageFormat, bool isError = true, bool isWarning = false)
{
return new DiagnosticDescriptor(
id: id,
title: title,
messageFormat: messageFormat,
category: "Usage",
defaultSeverity: isError ? DiagnosticSeverity.Error : DiagnosticSeverity.Info,
defaultSeverity: isError ? DiagnosticSeverity.Error : isWarning? DiagnosticSeverity.Warning : DiagnosticSeverity.Info,
isEnabledByDefault: true,
helpLinkUri: "https://github.com/microsoft/CsWinRT/tree/master/src/Authoring/WinRT.SourceGenerator/AnalyzerReleases.Unshipped.md");
}
Expand Down Expand Up @@ -180,7 +180,14 @@ private static DiagnosticDescriptor MakeRule(string id, string title, string mes
CsWinRTDiagnosticStrings.UnimplementedInterface_Brief,
CsWinRTDiagnosticStrings.UnimplementedInterface_Text);

public static DiagnosticDescriptor ClassNotAotCompatible = MakeRule(
public static DiagnosticDescriptor ClassNotAotCompatibleWarning = MakeRule(
"CsWinRT1028",
CsWinRTDiagnosticStrings.ClassNotMarkedPartial_Brief,
CsWinRTDiagnosticStrings.ClassNotMarkedPartial_Text,
false,
true);

public static DiagnosticDescriptor ClassNotAotCompatibleInfo = MakeRule(
"CsWinRT1028",
CsWinRTDiagnosticStrings.ClassNotMarkedPartial_Brief,
CsWinRTDiagnosticStrings.ClassNotMarkedPartial_Text,
Expand Down
32 changes: 32 additions & 0 deletions src/Tests/FunctionalTests/Async/Program.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
using System;
using System.IO;
using System.Runtime.InteropServices.WindowsRuntime;
using System.Threading.Tasks;
using TestComponentCSharp;
using Windows.Foundation;
using Windows.Storage.Streams;
using WinRT;

var instance = new Class();

Expand Down Expand Up @@ -87,6 +91,34 @@
return 109;
}

using var fileStream = File.OpenRead(folderPath + "\\Async.exe");
var randomAccessStream = fileStream.AsRandomAccessStream();
var ptr = MarshalInterface<IRandomAccessStream>.FromManaged(randomAccessStream);
if (ptr == IntPtr.Zero)
{
return 110;
}
var arr = new byte[100];
var buffer = arr.AsBuffer();
ptr = MarshalInterface<IBuffer>.FromManaged(buffer);
if (ptr == IntPtr.Zero)
{
return 111;
}

var asyncOperation = randomAccessStream.ReadAsync(buffer, 50, InputStreamOptions.Partial);
ptr = MarshalInterface<IAsyncOperationWithProgress<IBuffer, uint>>.FromManaged(asyncOperation);
if (ptr == IntPtr.Zero)
{
return 112;
}

ptr = MarshalInterface<IAsyncInfo>.FromManaged(asyncOperation);
if (ptr == IntPtr.Zero)
{
return 113;
}

return 100;

static async Task<int> InvokeAddAsync(Class instance, int lhs, int rhs)
Expand Down
2 changes: 2 additions & 0 deletions src/Tests/FunctionalTests/CCW/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,7 @@ public static void TestGenericList<T>()
instance.BindableIterableProperty = nestedClassList;
}

#pragma warning disable CsWinRT1028 // Class is not marked partial
sealed class NestedClass : IProperties2
{
private int _value;
Expand Down Expand Up @@ -476,6 +477,7 @@ internal sealed class NestedClass3<S> : IProperties2
public int ReadWriteProperty { get => _value; set => _value = value; }
}
}
#pragma warning restore CsWinRT1028 // Class is not marked partial
}

partial class TestClass2
Expand Down
1 change: 1 addition & 0 deletions src/cswinrt/cswinrt.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
<None Include="strings\additions\Windows.Storage.Streams\Windows.Storage.Streams.SR.cs" />
<None Include="strings\additions\Windows.Storage.Streams\StreamOperationAsyncResult.cs" />
<None Include="strings\additions\Windows.Storage.Streams\StreamOperationsImplementation.cs" />
<None Include="strings\additions\Windows.Storage.Streams\StreamTaskAdaptersImplementation.cs" />
<None Include="strings\additions\Windows.Storage.Streams\WindowsRuntimeBuffer.cs" />
<None Include="strings\additions\Windows.Storage.Streams\WindowsRuntimeBufferExtensions.cs" />
<None Include="strings\additions\Windows.Storage.Streams\WindowsRuntimeMarshal.cs" />
Expand Down
3 changes: 3 additions & 0 deletions src/cswinrt/cswinrt.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,9 @@
<None Include="strings\additions\Windows.Storage.Streams\WindowsRuntimeMarshal.cs">
<Filter>strings\additions\Windows.Storage.Streams</Filter>
</None>
<None Include="strings\additions\Windows.Storage.Streams\StreamTaskAdaptersImplementation.cs">
<Filter>strings\additions\Windows.Storage.Streams</Filter>
</None>
</ItemGroup>
<ItemGroup>
<Midl Include="WinRT.Interop.idl" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,14 @@ namespace Windows.Storage.Streams
[global::System.Runtime.Versioning.SupportedOSPlatform("windows10.0.10240.0")]
#endif
internal static class StreamOperationsImplementation
{
{
#if NET
static StreamOperationsImplementation()
{
_ = StreamTaskAdaptersImplementation.Initialized;
}
#endif

#region ReadAsync implementations

internal static IAsyncOperationWithProgress<IBuffer, uint> ReadAsync_MemoryStream(Stream stream, IBuffer buffer, uint count)
Expand Down
Loading