[main] Update dependencies from mono/linker #56593

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.

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

src/libraries/System.Data.Common/src/System/Data/Common/DbConnectionStringBuilder.cs

....Private.DataContractSerialization/src/System/Runtime/Serialization/PrimitiveDataContract.cs

- 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

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

@@ -627,6 +627,9 @@ internal sealed class LicenseInteropProxy
        private object? _licContext;
        private Type? _targetRcwType;

+        [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

src/libraries/System.Data.Common/src/System/Data/Common/DbConnectionStringBuilder.cs

-    // 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"); // IL2026 class Foo { [RUC("RUC")] public static void RUC() {} }

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

src/libraries/System.Data.Common/src/System/Data/DataTableReader.cs

+                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)

{

return propInfo.GetValue(null, null)!;

}

FieldInfo fieldInfo = 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

...es/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticSourceEventSource.cs

+    // 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.

src/libraries/System.Diagnostics.Tracing/tests/TrimmingTests/EventSourceManifestTest.cs

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

src/mono/System.Private.CoreLib/src/System/Reflection/Emit/EnumBuilder.Mono.cs

@@ -209,6 +209,10 @@ public override Type GetEnumUnderlyingType()
            return _underlyingType;
        }

+        [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)

sbomer and others added 4 commits July 29, 2021 18:44

Add windows-specific suppressions

890bcb7

Update dependencies from https://github.com/mono/linker build 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 vitek-karas and eerhardt July 30, 2021 00:29

eerhardt reviewed Jul 30, 2021

View reviewed changes

src/coreclr/System.Private.CoreLib/src/Internal/Runtime/InteropServices/ComActivator.cs Outdated Show resolved Hide resolved

eerhardt reviewed Jul 30, 2021

View reviewed changes

src/libraries/System.Data.Common/src/System/Data/Common/DbConnectionStringBuilder.cs Outdated Show resolved Hide resolved

eerhardt reviewed Jul 30, 2021

View reviewed changes

src/libraries/System.Data.Common/src/System/Data/Common/DbConnectionStringBuilder.cs Outdated Show resolved Hide resolved

eerhardt reviewed Jul 30, 2021

View reviewed changes

....Private.DataContractSerialization/src/System/Runtime/Serialization/PrimitiveDataContract.cs Outdated Show resolved Hide resolved

Fix failures and address feedback

06b421b

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

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

Update dependencies from https://github.com/mono/linker build 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 c353468 Compare August 2, 2021 22:32

akoeplinger approved these changes Aug 3, 2021

View reviewed changes

Update dependencies from https://github.com/mono/linker build 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

src/libraries/System.Diagnostics.Tracing/tests/TrimmingTests/EventSourceManifestTest.cs Outdated Show resolved Hide resolved

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 main Aug 3, 2021

dotnet-maestro bot 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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[main] Update dependencies from mono/linker #56593

[main] Update dependencies from mono/linker #56593

dotnet-maestro bot commented Jul 30, 2021 •

edited

Loading

eerhardt commented Jul 30, 2021

akoeplinger commented Aug 2, 2021

eerhardt Aug 3, 2021

eerhardt Aug 3, 2021 •

edited

Loading

sbomer Aug 3, 2021

eerhardt Aug 3, 2021

sbomer Aug 3, 2021

eerhardt Aug 3, 2021

sbomer Aug 3, 2021

eerhardt Aug 3, 2021

vitek-karas Aug 9, 2021

eerhardt Aug 3, 2021 •

edited

Loading

sbomer Aug 3, 2021

eerhardt left a comment

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

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

[main] Update dependencies from mono/linker #56593

[main] Update dependencies from mono/linker #56593

Conversation

dotnet-maestro bot commented Jul 30, 2021 • edited Loading

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

eerhardt commented Jul 30, 2021

akoeplinger commented Aug 2, 2021

Choose a reason for hiding this comment

eerhardt Aug 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eerhardt Aug 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eerhardt left a comment

Choose a reason for hiding this comment

dotnet-maestro bot commented Jul 30, 2021 •

edited

Loading

eerhardt Aug 3, 2021 •

edited

Loading

eerhardt Aug 3, 2021 •

edited

Loading