Fix DiagnosticSource to work with NativeAOT

There were 2 problems: 1. The use of MakeGenericType doesn't work when a property is a ValueType. An app will crash when a listener is enabled and DiagnosticSourceEventSource tries writing values. 2. The properties on KeyValuePair were not being preserved correctly, so the Arguments of the DiagnosticSourceEventSource methods were not being serialized correctly. Add test (and infrastructure) to ensure DiagnosticSource works in a NativeAOT app Fix #75945
dotnet · Nov 18, 2022 · 8e0867c · 8e0867c
1 parent 548c2a1
commit 8e0867c
Show file tree

Hide file tree

Showing 16 changed files with 270 additions and 60 deletions.
diff --git a/eng/testing/linker/SupportFiles/Directory.Build.props b/eng/testing/linker/SupportFiles/Directory.Build.props
@@ -2,7 +2,6 @@
   <PropertyGroup>
     <SkipConfigureTrimming>true</SkipConfigureTrimming>
     <PublishTrimmed>true</PublishTrimmed>
-    <SkipImportRepoLinkerTargets>true</SkipImportRepoLinkerTargets>
     <TrimMode>full</TrimMode>
     <TrimmerRemoveSymbols>false</TrimmerRemoveSymbols>
     <SelfContained>true</SelfContained>

diff --git a/eng/testing/linker/project.csproj.template b/eng/testing/linker/project.csproj.template
@@ -6,6 +6,7 @@
     <NETCoreAppMaximumVersion>{NetCoreAppMaximumVersion}</NETCoreAppMaximumVersion>
     <UseMonoRuntime>{UseMonoRuntime}</UseMonoRuntime>
     <RuntimeIdentifier>{RuntimeIdentifier}</RuntimeIdentifier>
+    <PublishAot>{PublishAot}</PublishAot>
 
     <!-- wasm specific -->
     <MonoAOTCompilerDir>{MonoAOTCompilerDir}</MonoAOTCompilerDir>
@@ -25,6 +26,12 @@
 
     <RepositoryEngineeringDir>{RepositoryEngineeringDir}</RepositoryEngineeringDir>
     <_ExtraTrimmerArgs>{ExtraTrimmerArgs} $(_ExtraTrimmerArgs)</_ExtraTrimmerArgs>
+
+    <!-- Needed for PublishAot -->
+    <IlcToolsPath>{IlcToolsPath}</IlcToolsPath>
+    <IlcBuildTasksPath>{IlcBuildTasksPath}</IlcBuildTasksPath>
+    <IlcSdkPath>{IlcSdkPath}</IlcSdkPath>
+    <IlcFrameworkPath>{IlcFrameworkPath}</IlcFrameworkPath>
   </PropertyGroup>
 
   <ItemGroup>

diff --git a/eng/testing/linker/trimmingTests.targets b/eng/testing/linker/trimmingTests.targets
@@ -85,8 +85,13 @@
                                                  .Replace('{NetCoreAppMaximumVersion}', '$(NetCoreAppMaximumVersion)')
                                                  .Replace('{UseMonoRuntime}','$(UseMonoRuntime)')
                                                  .Replace('{RuntimeIdentifier}','%(TestConsoleApps.TestRuntimeIdentifier)')
+                                                 .Replace('{PublishAot}','$(IsNativeAotTestProject)')
                                                  .Replace('{MicrosoftNETILLinkTasksVersion}', '$(MicrosoftNETILLinkTasksVersion)')
                                                  .Replace('{ExtraTrimmerArgs}', '%(TestConsoleApps.ExtraTrimmerArgs)')
+                                                 .Replace('{IlcToolsPath}', '$(CoreCLRILCompilerDir)')
+                                                 .Replace('{IlcBuildTasksPath}', '$(CoreCLRILCompilerDir)netstandard/ILCompiler.Build.Tasks.dll')
+                                                 .Replace('{IlcSdkPath}', '$(CoreCLRAotSdkDir)')
+                                                 .Replace('{IlcFrameworkPath}', '$(MicrosoftNetCoreAppRuntimePackRidLibTfmDir)')
                                                  .Replace('{RuntimeHostConfigurationOptions}', '$(_runtimeHostConfigurationOptionsString)')
                                                  .Replace('{AdditionalProjectReferences}', '$(_additionalProjectReferencesString)')
                                                  .Replace('{RepositoryEngineeringDir}', '$(RepositoryEngineeringDir)')

diff --git a/src/libraries/Directory.Build.props b/src/libraries/Directory.Build.props
@@ -22,10 +22,12 @@
   <PropertyGroup Condition="$(MSBuildProjectFullPath.Contains('$([System.IO.Path]::DirectorySeparatorChar)tests$([System.IO.Path]::DirectorySeparatorChar)'))">
     <IsTestProject Condition="$(MSBuildProjectName.EndsWith('.UnitTests')) or $(MSBuildProjectName.EndsWith('.Tests'))">true</IsTestProject>
     <IsTrimmingTestProject Condition="$(MSBuildProjectName.EndsWith('.TrimmingTests'))">true</IsTrimmingTestProject>
-    <IsTestSupportProject Condition="'$(IsTestProject)' != 'true' and '$(IsTrimmingTestProject)' != 'true'">true</IsTestSupportProject>
+    <IsNativeAotTestProject Condition="$(MSBuildProjectName.EndsWith('.NativeAotTests'))">true</IsNativeAotTestProject>
+    <IsPublishedAppTestProject Condition="'$(IsTrimmingTestProject)' == 'true' or '$(IsNativeAotTestProject)' == 'true'">true</IsPublishedAppTestProject>
+    <IsTestSupportProject Condition="'$(IsTestProject)' != 'true' and '$(IsPublishedAppTestProject)' != 'true'">true</IsTestSupportProject>
 
     <!-- Treat test assemblies as non-shipping (do not publish or sign them). -->
-    <IsShipping Condition="'$(IsTestProject)' == 'true' or '$(IsTestSupportProject)' == 'true' or '$(IsTrimmingTestProject)' == 'true'">false</IsShipping>
+    <IsShipping Condition="'$(IsTestProject)' == 'true' or '$(IsTestSupportProject)' == 'true' or '$(IsPublishedAppTestProject)' == 'true'">false</IsShipping>
   </PropertyGroup>
 
   <PropertyGroup>
@@ -36,14 +38,14 @@
                                 '$(IsReferenceAssemblyProject)' != 'true' and
                                 '$(IsGeneratorProject)' != 'true' and
                                 '$(IsTestProject)' != 'true' and
-                                '$(IsTrimmingTestProject)' != 'true' and
+                                '$(IsPublishedAppTestProject)' != 'true' and
                                 '$(IsTestSupportProject)' != 'true' and
                                 '$(UsingMicrosoftNoTargetsSdk)' != 'true' and
                                 '$(UsingMicrosoftTraversalSdk)' != 'true'">true</IsSourceProject>
   </PropertyGroup>
 
   <!-- Warnings that should be disabled in our test projects. -->
-  <PropertyGroup Condition="'$(IsTestProject)' == 'true' or '$(IsTestSupportProject)' == 'true' or '$(IsTrimmingTestProject)' == 'true'">
+  <PropertyGroup Condition="'$(IsTestProject)' == 'true' or '$(IsTestSupportProject)' == 'true' or '$(IsPublishedAppTestProject)' == 'true'">
     <!-- don't warn on usage of BinaryFormatter from test projects -->
     <NoWarn>$(NoWarn);SYSLIB0011</NoWarn>
     <!-- don't warn about unnecessary trim warning suppressions. can be removed with preview 6. -->

diff --git a/src/libraries/Directory.Build.targets b/src/libraries/Directory.Build.targets
@@ -127,7 +127,7 @@
   <Import Project="$(RepositoryEngineeringDir)references.targets" />
   <Import Project="$(RepositoryEngineeringDir)resolveContract.targets" />
   <Import Project="$(RepositoryEngineeringDir)testing\tests.targets" Condition="'$(EnableTestSupport)' == 'true'" />
-  <Import Project="$(RepositoryEngineeringDir)testing\linker\trimmingTests.targets" Condition="'$(IsTrimmingTestProject)' == 'true'" />
+  <Import Project="$(RepositoryEngineeringDir)testing\linker\trimmingTests.targets" Condition="'$(IsPublishedAppTestProject)' == 'true'" />
   <Import Project="$(RepositoryEngineeringDir)testing\runtimeConfiguration.targets" />
   <Import Project="$(RepositoryEngineeringDir)testing\runsettings.targets" Condition="'$(EnableRunSettingsSupport)' == 'true'" />
   <Import Project="$(RepositoryEngineeringDir)testing\coverage.targets" Condition="'$(EnableRunSettingsSupport)' == 'true' or '$(EnableCoverageSupport)' == 'true'" />

diff --git a/src/libraries/Microsoft.Extensions.Hosting/src/HostBuilder.cs b/src/libraries/Microsoft.Extensions.Hosting/src/HostBuilder.cs
@@ -188,8 +188,6 @@ internal static DiagnosticListener LogHostBuilding(HostApplicationBuilder hostAp
             return diagnosticListener;
         }
 
-        [UnconditionalSuppressMessage("AOT", "IL3050:RequiresDynamicCode",
-            Justification = "DiagnosticSource is used here to pass objects in-memory to code using HostFactoryResolver. This won't require creating new generic types.")]
         [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:UnrecognizedReflectionPattern",
             Justification = "The values being passed into Write are being consumed by the application already.")]
         private static void Write<T>(

diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSource.cs b/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSource.cs
@@ -20,7 +20,6 @@ public virtual void Dispose() { }
         public virtual System.IDisposable Subscribe(System.IObserver<System.Collections.Generic.KeyValuePair<string, object?>> observer, System.Predicate<string>? isEnabled) { throw null; }
         public override string ToString() { throw null; }
         [System.Diagnostics.CodeAnalysis.RequiresUnreferencedCode("The type of object being written to DiagnosticSource cannot be discovered statically.")]
-        [System.Diagnostics.CodeAnalysis.RequiresDynamicCode("DiagnosticSource may require creating new generic types or methods, which requires creating code at runtime. This may not work when AOT compiling.")]
         public override void Write(string name, object? value) { }
     }
     public abstract partial class DiagnosticSource
@@ -29,7 +28,6 @@ protected DiagnosticSource() { }
         public abstract bool IsEnabled(string name);
         public virtual bool IsEnabled(string name, object? arg1, object? arg2 = null) { throw null; }
         [System.Diagnostics.CodeAnalysis.RequiresUnreferencedCode("The type of object being written to DiagnosticSource cannot be discovered statically.")]
-        [System.Diagnostics.CodeAnalysis.RequiresDynamicCode("DiagnosticSource may require creating new generic types or methods, which requires creating code at runtime. This may not work when AOT compiling.")]
         public abstract void Write(string name, object? value);
     }
 }
diff --git a/...es/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs b/...es/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs
@@ -197,10 +197,8 @@ public abstract partial class DiagnosticSource
         public virtual void OnActivityExport(System.Diagnostics.Activity activity, object? payload) { }
         public virtual void OnActivityImport(System.Diagnostics.Activity activity, object? payload) { }
         [System.Diagnostics.CodeAnalysis.RequiresUnreferencedCode("The type of object being written to DiagnosticSource cannot be discovered statically.")]
-        [System.Diagnostics.CodeAnalysis.RequiresDynamicCode("DiagnosticSource may require creating new generic types or methods, which requires creating code at runtime. This may not work when AOT compiling.")]
         public System.Diagnostics.Activity StartActivity(System.Diagnostics.Activity activity, object? args) { throw null; }
         [System.Diagnostics.CodeAnalysis.RequiresUnreferencedCode("The type of object being written to DiagnosticSource cannot be discovered statically.")]
-        [System.Diagnostics.CodeAnalysis.RequiresDynamicCode("DiagnosticSource may require creating new generic types or methods, which requires creating code at runtime. This may not work when AOT compiling.")]
         public void StopActivity(System.Diagnostics.Activity activity, object? args) { }
     }
     public enum ActivitySamplingResult

diff --git a/...raries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj b/...raries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj
@@ -85,6 +85,8 @@ System.Diagnostics.DiagnosticSource</PackageDescription>
     <Compile Include="System\Diagnostics\System.Diagnostics.DiagnosticSource.Typeforwards.netcoreapp.cs" />
     <Compile Include="$(CommonPath)System\LocalAppContextSwitches.Common.cs"
              Link="Common\System\LocalAppContextSwitches.Common.cs" />
+    <Compile Include="$(CommonPath)System\Text\ValueStringBuilder.cs"
+             Link="Common\System\Text\ValueStringBuilder.cs" />
   </ItemGroup>
 
   <ItemGroup Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'">

diff --git a/...ibraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticListener.cs b/...ibraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticListener.cs
@@ -34,8 +34,6 @@ public partial class DiagnosticListener : DiagnosticSource, IObservable<KeyValue
         /// </summary>
         public static IObservable<DiagnosticListener> AllListeners
         {
-            [UnconditionalSuppressMessage("AotAnalysis", "IL3050:RequiresDynamicCode",
-               Justification = "ENABLE_HTTP_HANDLER is not enabled in the .NET current version")]
             get
             {
 #if ENABLE_HTTP_HANDLER
@@ -255,7 +253,6 @@ public override bool IsEnabled(string name, object? arg1, object? arg2 = null)
         /// Override abstract method
         /// </summary>
         [RequiresUnreferencedCode(WriteRequiresUnreferencedCode)]
-        [RequiresDynamicCode(WriteRequiresDynamicCode)]
         public override void Write(string name, object? value)
         {
             for (DiagnosticSubscription? curSubscription = _subscriptions; curSubscription != null; curSubscription = curSubscription.Next)

diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticSource.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticSource.cs
@@ -17,7 +17,6 @@ namespace System.Diagnostics
     public abstract partial class DiagnosticSource
     {
         internal const string WriteRequiresUnreferencedCode = "The type of object being written to DiagnosticSource cannot be discovered statically.";
-        internal const string WriteRequiresDynamicCode = "DiagnosticSource may require creating new generic types or methods, which requires creating code at runtime. This may not work when AOT compiling.";
 
         /// <summary>
         /// Write is a generic way of logging complex payloads.  Each notification
@@ -36,7 +35,6 @@ public abstract partial class DiagnosticSource
         /// <param name="value">An object that represent the value being passed as a payload for the event.
         /// This is often an anonymous type which contains several sub-values.</param>
         [RequiresUnreferencedCode(WriteRequiresUnreferencedCode)]
-        [RequiresDynamicCode(WriteRequiresDynamicCode)]
         public abstract void Write(string name, object? value);
 
         /// <summary>

diff --git a/...es/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticSourceActivity.cs b/...es/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticSourceActivity.cs
@@ -26,7 +26,6 @@ public abstract partial class DiagnosticSource
         /// <returns>Started Activity for convenient chaining</returns>
         /// <seealso cref="Activity"/>
         [RequiresUnreferencedCode(WriteRequiresUnreferencedCode)]
-        [RequiresDynamicCode(WriteRequiresDynamicCode)]
         public Activity StartActivity(Activity activity, object? args)
         {
             activity.Start();
@@ -45,7 +44,6 @@ public Activity StartActivity(Activity activity, object? args)
         /// <param name="args">An object that represent the value being passed as a payload for the event.</param>
         /// <seealso cref="Activity"/>
         [RequiresUnreferencedCode(WriteRequiresUnreferencedCode)]
-        [RequiresDynamicCode(WriteRequiresDynamicCode)]
         public void StopActivity(Activity activity, object? args)
         {
             // Stop sets the end time if it was unset, but we want it set before we issue the write