[main] Update dependencies from mono/linker by dotnet-maestro[bot] · Pull Request #56593 · dotnet/runtime

dotnet-maestro · 2021-07-30T00:21:14Z

This pull request updates the following dependencies

From https://github.com/mono/linker

Subscription: 0d6d6ae4-f71f-4395-53e6-08d8e409d87d
Build: 20210802.2
Date Produced: 8/2/2021 1:15 PM
Commit: ae18468b8712503aee67911228dd921601bd423a
Branch: refs/heads/main

Updates:
- Microsoft.NET.ILLink.Tasks: from 6.0.100-preview.6.21378.1 to 6.0.100-preview.6.21402.2

dotnet/linker#2145 warns when accessing members annotated with DynamicallyAccessedMembers using reflection, and dotnet/linker#2162 updates the warning origin of warnings for DynamicallyAccessedMembers on types. This adds suppressions for the new warnings.

Microsoft.NET.ILLink.Tasks From Version 6.0.100-preview.6.21378.1 -> To Version 6.0.100-preview.6.21379.2

…5a93356-f7f7-4eda-8c0e-7abec4014df7

eerhardt · 2021-07-30T02:57:00Z

Looks like this is still broken:

System.Reflection.Emit.EnumBuilder.setup_enum_type(Type): Field 'System.Reflection.Emit.TypeBuilder System.Reflection.Emit.EnumBuilder::_tb' with 'DynamicallyAccessedMembersAttribute' is accessed via reflection. Trimmer can't guarantee availability of the requirements of the field.

- Annotate mono's EnumBuilder and TypeBuilder - Add (non-unique) readable short names to the warning codes

Microsoft.NET.ILLink.Tasks From Version 6.0.100-preview.6.21378.1 -> To Version 6.0.100-preview.6.21380.2

akoeplinger · 2021-08-02T10:07:37Z

A few more trimmer errors in tests:

/_/src/libraries/System.Data.Common/tests/System/Data/Common/DbDataReaderMock.cs(147,13): error IL2111: (NETCORE_ENGINEERING_TELEMETRY=Build) System.Data.Common.Tests.SchemaDbDataReaderMock.GetSchemaTable(): Method 'System.Type.TypeInitializer.get' with parameters or return value with `DynamicallyAccessedMembersAttribute` is accessed via reflection. Trimmer can't guarantee availability of the requirements of the method.
/_/src/libraries/System.Data.Common/tests/System/Xml/XmlDataDocumentTests.cs(138,13): error IL2111: (NETCORE_ENGINEERING_TELEMETRY=Build) System.Xml.Tests.XmlDataDocumentTests.Create(): Method 'System.Type.TypeInitializer.get' with parameters or return value with `DynamicallyAccessedMembersAttribute` is accessed via reflection. Trimmer can't guarantee availability of the requirements of the method.
/_/src/libraries/System.Data.Common/tests/System/Data/XmlDataLoaderTest.cs(69,13): error IL2111: (NETCORE_ENGINEERING_TELEMETRY=Build) System.Data.Tests.XmlDataLoaderTest.Create(): Method 'System.Type.TypeInitializer.get' with parameters or return value with `DynamicallyAccessedMembersAttribute` is accessed via reflection. Trimmer can't guarantee availability of the requirements of the method.

Microsoft.NET.ILLink.Tasks From Version 6.0.100-preview.6.21378.1 -> To Version 6.0.100-preview.6.21402.2

eerhardt · 2021-08-03T15:10:33Z


+ [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2111:RequiresDynamicallyAccessedMembers",
+ Justification = "The type parameter to LicenseManager.CreateWithContext method has PublicConstructors annotation. We only invoke this method" +
+ "from AllocateAndValidateLicense which annotates the value passed in with the same annotation.")]


Yay for extra validation. Caught a bug here...

eerhardt · 2021-08-03T15:13:27Z

- // https://github.com/mono/linker/issues/2136 tracks making it possible to add more granular suppressions at the member level, and with a different warning code.
- [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
- Justification = "The use of GetType preserves members with RequiresUnreferencedCode, but the GetType callsites either "
+ [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2113:RequiresUnreferencedCode",


(nit) - "IL2113:RequiresUnreferencedCode" should be "IL2113:RequiresDynamicallyAccessedMembers" (or whatever short name we decided to give these new warnings).
This is repeated elsewhere for IL2112 in this file, and other files.

My thinking was that RequiresUnreferencedCode here was about the DAM-on-type requiring methods annotated with RequiresUnreferencedCode. I renamed it to ReflectionToRequiresUnreferencedCode. Just note that some instances of IL2026 also represent ReflectionToRequiresUnreferencedCode, for instance:
typeof(Foo).GetMethod("RUC");// IL2026classFoo{[RUC("RUC")]publicstaticvoidRUC(){}}

eerhardt · 2021-08-03T15:17:22Z

+ Justification = "The problem is Type.TypeInitializer which requires constructors on the Type instance." +
+ "In this case the Type instance is about System.Type itself, so adding an explicit dependency on the same" +
+ "annotation here.")]
+ [DynamicDependency(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors, typeof(Type))]


Does the trimmer treat PublicConstructors | NonPublicConstructors as referencing static cctors?
I'm wondering if this DynamicDependency is truly necessary.

Yup, it does. Good question about DynamicDependency. I only found one place where the annotation was ultimately used:
runtime/src/libraries/System.Data.Common/src/System/Data/Common/SqlUDTStorage.cs
Lines 45 to 51 in f9cbd72
PropertyInfo?propInfo=type.GetProperty("Null",BindingFlags.Public|BindingFlags.Static);
if(propInfo!=null)
{
returnpropInfo.GetValue(null,null)!;
}
FieldInfofieldInfo=type.GetField("Null",BindingFlags.Public|BindingFlags.Static)!;
If I didn't miss anything I think it's ok to remove.

eerhardt · 2021-08-03T15:21:55Z

+ // These suppressions can go away with https://github.com/mono/linker/issues/2175
+ [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2113:RequiresUnreferencedCode",
+ Justification = "In EventSource, EnsureDescriptorsInitialized's use of GetType preserves methods on Delegate and MulticastDelegate " +
+ "because the nested type OverrideEventProvider's base type EventProvider defines a delegate. " +


I'm not sure I understand why DiagnosticSourceEventSource is getting warnings for OverrideEventProvider. If it is - why isn't all EventSource's getting this warning?

The warning is due to DAMT.All on EventSource, which ensures that GetType keeps everything on EventSource - in this case the nested type OverrideEventProvider, plus its base type EventProvider, which has a delegate, which has some RUC methods: https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/Delegate.cs#L45.
Ideally we wouldn't produce this warning for derived types, and it would show up only on EventSource, but we haven't taught the linker about this (see dotnet/linker#2175).
In theory this should currently show up on every derived EventSource... but I believe due to dotnet/linker#2159 we only produce a warning for the first one that we see. :( /cc @LakshanF

we only produce a warning for the first one that we see. :(
Oh no, that doesn't sound like a great situation.

This sounds bad - could it lead to us getting warnings from CoreLib when trimming apps? For some reason linker will see this on another EventSource and will warn there - and it's not suppressed there. We need to fix this if that's the case.

eerhardt · 2021-08-03T15:27:01Z


+ [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2110:RequiresDynamicallyAccessedMembers",
+ Justification = "For instance member internal calls, the linker preserves all fields of the declaring type. " +
+ "The _tb field has DynamicallyAccessedMembersAttribute requirements, but the field access is safe because " +


This is a super weird place for this suppression IMO. You don't even see the _tb field around here.
Maybe it might help to use MethodImplOptions.InternalCall (or just InternalCall) in the suppression message instead of "internal calls".

I filed dotnet/linker#2188 about the warning origin.

- Suppress IL2111 in trimming tests - Remove unnecessary DynamicDependency - Fix indentation

eerhardt

Thanks @sbomer !

…ger_proxy_attribute * origin/main: disable token info in traces. (dotnet#56780) [debugger] Fix debugger.break behavior (dotnet#56788) [mono][wasm] Allow setting env variables with '=' characters in the test runner. (dotnet#56802) Ecma edit for `conv.ovf.<to type>.un`. (dotnet#56450) Mark HandleProcessCorruptedStateExceptionsAttribute as obsolete (dotnet#56664) Enable SxS install of previews on Mac OS (dotnet#56797) CoreCLR runtime tests + Mono on the x64 iOS simulator (dotnet#43954) [main] Update dependencies from mono/linker (dotnet#56593) STJ: Fix deserialization of UInt16 properties (dotnet#56793)

sbomerand others added 4 commits July 29, 2021 18:44

Add windows-specific suppressions
890bcb7

Update dependencies fromhttps://github.com/mono/linkerbuild 20210729.2
43f29d7
Microsoft.NET.ILLink.Tasks From Version 6.0.100-preview.6.21378.1 -> To Version 6.0.100-preview.6.21379.2

Merge branch 'updateSuppressionsForReflectionAccess' into darc-main-c…
4afe081
…5a93356-f7f7-4eda-8c0e-7abec4014df7

sbomer requested review from eerhardt and vitek-karas July 30, 2021 00:29

eerhardt reviewed Jul 30, 2021
View reviewed changes

Comment threadsrc/coreclr/System.Private.CoreLib/src/Internal/Runtime/InteropServices/ComActivator.cs Outdated

eerhardt reviewed Jul 30, 2021
View reviewed changes

Comment threadsrc/libraries/System.Data.Common/src/System/Data/Common/DbConnectionStringBuilder.cs Outdated

eerhardt reviewed Jul 30, 2021
View reviewed changes

Comment threadsrc/libraries/System.Data.Common/src/System/Data/Common/DbConnectionStringBuilder.cs Outdated

eerhardt reviewed Jul 30, 2021
View reviewed changes

Comment thread....Private.DataContractSerialization/src/System/Runtime/Serialization/PrimitiveDataContract.cs Outdated

Fix failures and address feedback
06b421b
- Annotate mono's EnumBuilder and TypeBuilder - Add (non-unique) readable short names to the warning codes

dotnet-maestroBot requested a review from marek-safar as a code owner July 30, 2021 20:17

Update dependencies fromhttps://github.com/mono/linkerbuild 20210730.2
a53d3d6
Microsoft.NET.ILLink.Tasks From Version 6.0.100-preview.6.21378.1 -> To Version 6.0.100-preview.6.21380.2

sbomer closed this Aug 2, 2021

sbomer reopened this Aug 2, 2021

Suppress IL2111
c353468

sbomer force-pushed the darc-main-c5a93356-f7f7-4eda-8c0e-7abec4014df7 branch from e1257f8 to c353468Compare August 2, 2021 22:32

akoeplinger approved these changes Aug 3, 2021
View reviewed changes

Update dependencies fromhttps://github.com/mono/linkerbuild 20210802.2
1a111df
Microsoft.NET.ILLink.Tasks From Version 6.0.100-preview.6.21378.1 -> To Version 6.0.100-preview.6.21402.2

eerhardt reviewed Aug 3, 2021
View reviewed changes

Comment threadsrc/libraries/System.Diagnostics.Tracing/tests/TrimmingTests/EventSourceManifestTest.cs Outdated

eerhardt reviewed Aug 3, 2021
View reviewed changes

sbomer mentioned this pull request Aug 3, 2021
Reflection access from Pinvokes and InternalCalls should warn at callsite dotnet/linker#2188
Closed

sbomer added 2 commits August 3, 2021 17:36

Feedback
03dc002
- Suppress IL2111 in trimming tests - Remove unnecessary DynamicDependency - Fix indentation

Update readable warning names
b96ed81

danmoseley added the area-codeflow for labeling automated codeflow label Aug 3, 2021

eerhardt mentioned this pull request Aug 3, 2021
[main] Update dependencies from mono/linker dotnet/sdk#19412
Closed

eerhardt approved these changes Aug 3, 2021
View reviewed changes

sbomer merged commit a35aff7 into mainAug 3, 2021

dotnet-maestroBot deleted the darc-main-c5a93356-f7f7-4eda-8c0e-7abec4014df7 branch August 3, 2021 22:33

This was referenced Aug 17, 2021
Testing out the new system jaredpar/runfo#71
Open
RunContinueWithStressTestsNoState timing out in CI #2271
Closed

ghost locked as resolved and limited conversation to collaborators Sep 8, 2021

	PropertyInfo?propInfo=type.GetProperty("Null",BindingFlags.Public\|BindingFlags.Static);
	if(propInfo!=null)
	{
	returnpropInfo.GetValue(null,null)!;
	}

	FieldInfofieldInfo=type.GetField("Null",BindingFlags.Public\|BindingFlags.Static)!;

Uh oh!

Conversation

dotnet-maestroBot commented Jul 30, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

From https://github.com/mono/linker

Uh oh!

eerhardt commented Jul 30, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

akoeplinger commented Aug 2, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eerhardtAug 3, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

eerhardtAug 3, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eerhardt left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dotnet-maestroBot commented Jul 30, 2021•
edited
Loading

eerhardtAug 3, 2021•
edited
Loading

eerhardtAug 3, 2021•
edited
Loading