Skip to content

Commit

Permalink
Fix DiagnosticSource to work with NativeAOT (#76109)
Browse files Browse the repository at this point in the history
* 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

* Enable new NativeAotTests in CI

- Only run them in Release configuration
- Suppress IL2026 warning

* Don't run NativeAot published app tests on OSX since it isn't supported

Set EventSourceSupport only on the projects that need it.
  • Loading branch information
eerhardt authored Sep 28, 2022
1 parent fbb3d57 commit 70b33f8
Show file tree
Hide file tree
Showing 19 changed files with 305 additions and 67 deletions.
6 changes: 6 additions & 0 deletions eng/pipelines/coreclr/nativeaot-post-build-steps.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,9 @@ steps:
- ${{ if ne(parameters.osGroup, 'windows') }}:
- script: $(Build.SourcesDirectory)/src/tests/run.sh --runnativeaottests $(buildConfigUpper) ${{ parameters.archType }}
displayName: Run tests in single file mode

# Publishing tooling doesn't support different configs between runtime and libs, so only run tests in Release config
# PublishAot on OSX doesn't work yet. Need an SDK with https://github.com/dotnet/installer/pull/14443.
- ${{ if and(eq(parameters.buildConfig, 'release'), ne(parameters.osGroup, 'OSX')) }}:
- script: $(Build.SourcesDirectory)$(dir)build$(scriptExt) -ci -arch ${{ parameters.archType }} $(_osParameter) -s libs.tests -c $(_BuildConfig) /p:TestAssemblies=false /p:RunNativeAotTestApps=true $(_officialBuildParameter) $(_crossBuildPropertyArg) /bl:$(Build.SourcesDirectory)/artifacts/log/$(buildConfigUpper)/NativeAotTests.binlog ${{ parameters.extraTestArgs }}
displayName: Run NativeAot Library Tests
1 change: 0 additions & 1 deletion eng/testing/linker/SupportFiles/Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
<PropertyGroup>
<SkipConfigureTrimming>true</SkipConfigureTrimming>
<PublishTrimmed>true</PublishTrimmed>
<SkipImportRepoLinkerTargets>true</SkipImportRepoLinkerTargets>
<TrimMode>full</TrimMode>
<TrimmerRemoveSymbols>false</TrimmerRemoveSymbols>
<SelfContained>true</SelfContained>
Expand Down
4 changes: 4 additions & 0 deletions eng/testing/linker/SupportFiles/Directory.Build.targets
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
DependsOnTargets="BundleTestWasmApp"
Condition="'$(TargetArchitecture)' == 'wasm' And '$(TargetOS)' == 'browser'" />

<PropertyGroup Condition="'$(PublishAot)' == 'true'">
<ILCompilerTargetsPath>$(CoreCLRBuildIntegrationDir)Microsoft.DotNet.ILCompiler.SingleEntry.targets</ILCompilerTargetsPath>
</PropertyGroup>

<!-- Overriding these targets as these projects won't need to binplace -->
<Target Name="PublishTestAsSelfContained" />

Expand Down
10 changes: 10 additions & 0 deletions eng/testing/linker/project.csproj.template
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<NETCoreAppMaximumVersion>{NetCoreAppMaximumVersion}</NETCoreAppMaximumVersion>
<UseMonoRuntime>{UseMonoRuntime}</UseMonoRuntime>
<RuntimeIdentifier>{RuntimeIdentifier}</RuntimeIdentifier>
<PublishAot>{PublishAot}</PublishAot>

<!-- wasm specific -->
<MonoAOTCompilerDir>{MonoAOTCompilerDir}</MonoAOTCompilerDir>
Expand All @@ -25,6 +26,15 @@

<RepositoryEngineeringDir>{RepositoryEngineeringDir}</RepositoryEngineeringDir>
<_ExtraTrimmerArgs>{ExtraTrimmerArgs} $(_ExtraTrimmerArgs)</_ExtraTrimmerArgs>
{AdditionalProperties}

<!-- Needed for PublishAot -->
<IlcToolsPath>{IlcToolsPath}</IlcToolsPath>
<IlcBuildTasksPath>{IlcBuildTasksPath}</IlcBuildTasksPath>
<IlcSdkPath>{IlcSdkPath}</IlcSdkPath>
<IlcFrameworkPath>{IlcFrameworkPath}</IlcFrameworkPath>
<IlcFrameworkNativePath>{IlcFrameworkNativePath}</IlcFrameworkNativePath>
<CoreCLRBuildIntegrationDir>{CoreCLRBuildIntegrationDir}</CoreCLRBuildIntegrationDir>
</PropertyGroup>

<ItemGroup>
Expand Down
12 changes: 12 additions & 0 deletions eng/testing/linker/trimmingTests.targets
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,14 @@
<ItemGroup>
<_switchesAsItems Include="%(TestConsoleApps.DisabledFeatureSwitches)" Value="false" />
<_switchesAsItems Include="%(TestConsoleApps.EnabledFeatureSwitches)" Value="true" />

<_propertiesAsItems Include="%(TestConsoleApps.DisabledProperties)" Value="false" />
<_propertiesAsItems Include="%(TestConsoleApps.EnabledProperties)" Value="true" />
</ItemGroup>

<PropertyGroup>
<_runtimeHostConfigurationOptionsString>@(_switchesAsItems->'&lt;RuntimeHostConfigurationOption Include=&quot;%(Identity)&quot; Value=&quot;%(Value)&quot; Trim=&quot;true&quot; /&gt;', '%0a ')</_runtimeHostConfigurationOptionsString>
<_additionalPropertiesString>@(_propertiesAsItems->'&lt;%(Identity)&gt;%(Value)&lt;/%(Identity)&gt;', '%0a ')</_additionalPropertiesString>
</PropertyGroup>

<MakeDir Directories="$(_projectDir)" />
Expand All @@ -85,8 +89,16 @@
.Replace('{NetCoreAppMaximumVersion}', '$(NetCoreAppMaximumVersion)')
.Replace('{UseMonoRuntime}','$(UseMonoRuntime)')
.Replace('{RuntimeIdentifier}','%(TestConsoleApps.TestRuntimeIdentifier)')
.Replace('{PublishAot}','$(IsNativeAotTestProject)')
.Replace('{MicrosoftNETILLinkTasksVersion}', '$(MicrosoftNETILLinkTasksVersion)')
.Replace('{ExtraTrimmerArgs}', '%(TestConsoleApps.ExtraTrimmerArgs)')
.Replace('{AdditionalProperties}', '$(_additionalPropertiesString)')
.Replace('{IlcToolsPath}', '$(CoreCLRILCompilerDir)')
.Replace('{IlcBuildTasksPath}', '$(CoreCLRILCompilerDir)netstandard/ILCompiler.Build.Tasks.dll')
.Replace('{IlcSdkPath}', '$(CoreCLRAotSdkDir)')
.Replace('{IlcFrameworkPath}', '$(MicrosoftNetCoreAppRuntimePackRidLibTfmDir)')
.Replace('{IlcFrameworkNativePath}', '$(MicrosoftNetCoreAppRuntimePackNativeDir)')
.Replace('{CoreCLRBuildIntegrationDir}', '$(CoreCLRBuildIntegrationDir)')
.Replace('{RuntimeHostConfigurationOptions}', '$(_runtimeHostConfigurationOptionsString)')
.Replace('{AdditionalProjectReferences}', '$(_additionalProjectReferencesString)')
.Replace('{RepositoryEngineeringDir}', '$(RepositoryEngineeringDir)')
Expand Down
10 changes: 6 additions & 4 deletions src/libraries/Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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. -->
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/Directory.Build.targets
Original file line number Diff line number Diff line change
Expand Up @@ -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'" />
Expand Down
2 changes: 0 additions & 2 deletions src/libraries/Microsoft.Extensions.Hosting/src/HostBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,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'">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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
Expand Down
Loading

0 comments on commit 70b33f8

Please sign in to comment.