Skip to content

Add always trimmed attributes list for browser config #39000

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.

Sign up for GitHub

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

Merged
6 commits merged into from
Jul 13, 2020
Merged

Add always trimmed attributes list for browser config #39000

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c294b47
Add always trimmed attributes list for browser config
marek-safar Jul 9, 2020
08d790a
Review feedback
marek-safar Jul 9, 2020
ffe0b6d
Combine and embed the ILLink.LinkAttributes.xml file into the assembly.
eerhardt Jul 9, 2020
bb840d2
Update linker command line to ignore attributes file.
eerhardt Jul 9, 2020
64f2a45
PR feedback.
eerhardt Jul 9, 2020
fc69953
Update ILLink.LinkAttributes.wasm.xml
marek-safar Jul 10, 2020
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
46 changes: 36 additions & 10 deletions eng/illink.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
<ILLinkDescriptorsXmlIntermediatePath>$(IntermediateOutputPath)ILLink.Descriptors.xml</ILLinkDescriptorsXmlIntermediatePath>

<ILLinkSubstitutionsXmlIntermediatePath>$(IntermediateOutputPath)ILLink.Substitutions.xml</ILLinkSubstitutionsXmlIntermediatePath>
<ILLinkLinkAttributesXmlIntermediatePath>$(IntermediateOutputPath)ILLink.LinkAttributes.xml</ILLinkLinkAttributesXmlIntermediatePath>

<!-- if building a PDB, tell illink to rewrite the symbols file -->
<ILLinkRewritePDBs Condition="'$(ILLinkRewritePDBs)' == '' and '$(DebugSymbols)' != 'false'">true</ILLinkRewritePDBs>
Expand All @@ -51,9 +52,9 @@
<None Include="@(ILLinkSubstitutionsXmls)" />
</ItemGroup>

<!-- Custom binplacing for pre/post-trimming and reports that is useful for analysis
<!-- Custom binplacing for pre/post-trimming and reports that is useful for analysis
Must be enabled by setting BinPlaceILLinkTrimAssembly=true
-->
-->
<ItemGroup Condition="'$(BinPlaceILLinkTrimAssembly)' == 'true'">
<BinPlaceTargetFramework Include="$(BuildSettings)">
<RuntimePath>$(ArtifactsBinDir)ILLinkTrimAssembly/$(BuildSettings)/trimmed</RuntimePath>
Expand All @@ -70,7 +71,7 @@
</ItemGroup>

<Target Name="_EmbedILLinkXmls"
DependsOnTargets="_CombineILLinkDescriptorsXmls;_CombineILLinkSubstitutionsXmls">
DependsOnTargets="_CombineILLinkDescriptorsXmls;_CombineILLinkSubstitutionsXmls;_CombineILLinkLinkAttributesXmls">

<ItemGroup Condition="'$(ILLinkTrimXml)' != ''">
<EmbeddedResource Include="$(ILLinkTrimXml)">
Expand All @@ -84,6 +85,12 @@
</EmbeddedResource>
</ItemGroup>

<ItemGroup Condition="'$(ILLinkLinkAttributesXml)' != ''">
<EmbeddedResource Include="$(ILLinkLinkAttributesXml)">
<LogicalName>ILLink.LinkAttributes.xml</LogicalName>
</EmbeddedResource>
</ItemGroup>

</Target>

<UsingTask TaskName="CombineLinkerXmlFiles" AssemblyFile="$(ILLinkTasksPath)" />
Expand Down Expand Up @@ -111,11 +118,11 @@
<ItemGroup Condition="'$(GenerateResourcesSubstitutions)' == 'true'">
<ILLinkSubstitutionsXmls Include="$(ILLinkResourcesSubstitutionIntermediatePath)" />
</ItemGroup>

<!-- If a library uses string resources, the following target generates a substitution xml that will be embedded on the
library so that if a consumer wants to run the linker they can specify a feature switch to strip out all resources
from the assembly. -->
<Target Name="GenerateResourcesSubstitutionFile"
<Target Name="GenerateResourcesSubstitutionFile"
Condition="'$(GenerateResourcesSubstitutions)' == 'true'"
Inputs="$(MSBuildProjectFullPath)"
Outputs="$(ILLinkResourcesSubstitutionIntermediatePath)">
Expand Down Expand Up @@ -152,7 +159,23 @@
</ItemGroup>
</Target>

<Target Name="_SetILLinkTrimAssembly"
<Target Name="_CombineILLinkLinkAttributesXmls"
Condition="'@(ILLinkLinkAttributesXmls)' != ''"
Inputs="@(ILLinkLinkAttributesXmls)"
Outputs="$(ILLinkLinkAttributesXmlIntermediatePath)">
<PropertyGroup>
<ILLinkLinkAttributesXml>$(ILLinkLinkAttributesXmlIntermediatePath)</ILLinkLinkAttributesXml>
</PropertyGroup>

<CombineLinkerXmlFiles LinkerXmlFiles="@(ILLinkLinkAttributesXmls)"
CombinedLinkerXmlFile="$(ILLinkLinkAttributesXml)" />

<ItemGroup>
<FileWrites Include="$(ILLinkLinkAttributesXml)" />
</ItemGroup>
</Target>

<Target Name="_SetILLinkTrimAssembly"
Condition="'$(ILLinkTrimAssembly)' == ''"
DependsOnTargets="GetBinPlaceTargetFramework">
<PropertyGroup>
Expand Down Expand Up @@ -186,6 +209,9 @@
<ILLinkArgs Condition="'$(ILLinkTrimXmlLibraryBuild)' != ''">$(ILLinkArgs) -x "$(ILLinkTrimXmlLibraryBuild)"</ILLinkArgs>
<!-- don't remove the embedded substitutions xml resource since ILLink may run again on the assembly -->
<ILLinkArgs Condition="'$(ILLinkSubstitutionsXml)' != ''">$(ILLinkArgs) --strip-substitutions false</ILLinkArgs>
<!-- don't remove the embedded link attributes xml resource since ILLink may run again on the assembly -->
<!-- and ignore the link attributes xml file during the library build, since we need the attributes preserved until the final app is linked -->
<ILLinkArgs Condition="'$(ILLinkLinkAttributesXml)' != ''">$(ILLinkArgs) --strip-link-attributes false --ignore-link-attributes true</ILLinkArgs>
<!-- ignore unresolved references -->
<ILLinkArgs>$(ILLinkArgs) --skip-unresolved true</ILLinkArgs>
<!-- keep interface implementations -->
Expand Down Expand Up @@ -235,7 +261,7 @@
<_DotNetHostFileName>dotnet</_DotNetHostFileName>
<_DotNetHostFileName Condition=" '$(OS)' == 'Windows_NT' ">dotnet.exe</_DotNetHostFileName>
</PropertyGroup>

<ILLink AssemblyPaths="$(ILLinkTrimInputAssembly)"
RootAssemblyNames=""
OutputDirectory="$(ILLinkTrimOutputPath)"
Expand Down Expand Up @@ -272,10 +298,10 @@
<Exec Command="$(AsmDiffCmd) $(AsmDiffListArgs)" />
<Message Text="Assembly trimming report: $(AsmDiffList)" />
</Target>
<!-- Similar to _CheckForCompileOutputs and runs in the same places,

<!-- Similar to _CheckForCompileOutputs and runs in the same places,
always set these even if compile didn't run. -->
<Target Name="_CheckForILLinkTrimAssemblyOutputs"
<Target Name="_CheckForILLinkTrimAssemblyOutputs"
BeforeTargets="CopyFilesToOutputDirectory;_CleanGetCurrentAndPriorFileWrites"
Condition="'$(ILLinkTrimAssembly)' == 'true'">
<ItemGroup>
Expand Down
3 changes: 3 additions & 0 deletions src/mono/netcore/System.Private.CoreLib/System.Private.CoreLib.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,9 @@

<ILLinkSubstitutionsXmls Include="$(ILLinkDirectory)ILLink.Substitutions.$(Platform).xml"
Condition="Exists('$(ILLinkDirectory)ILLink.Substitutions.$(Platform).xml')" />

<ILLinkLinkAttributesXmls Include="$(ILLinkDirectory)/ILLink.LinkAttributes.$(Platform).xml"
Condition="Exists('$(ILLinkDirectory)ILLink.LinkAttributes.$(Platform).xml')" />
</ItemGroup>

<!-- Sources -->
Expand Down
149 changes: 149 additions & 0 deletions src/mono/netcore/System.Private.CoreLib/src/ILLink/ILLink.LinkAttributes.wasm.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
<linker>
<assembly fullname="System.Private.CoreLib">
<!-- System -->
<type fullname="System.CLSCompliantAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.ObsoleteAttribute">
<!--
Note that removing this attribute can change runtime behavior. For example,
System.Xml.Serialization will behave differently if a ctor is Obsolete.
This is low enough risk on wasm to justify the removing the attribute for size
savings. The app developer can override this setting to keep all ObsoleteAttributes.
-->
<attribute internal="RemoveAttributeInstances" />
</type>

<!-- System.Diagnostics.CodeAnalysis -->
<type fullname="System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverageAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.SuppressMessageAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.AllowNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.DisallowNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.MaybeNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.NotNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.NotNullWhenAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.NotNullIfNotNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.DoesNotReturnAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.DoesNotReturnIfAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.MemberNotNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.MemberNotNullWhenAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>

<!-- System.Runtime.CompilerServices -->
<type fullname="System.Runtime.CompilerServices.AsyncMethodBuilderAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CallerArgumentExpressionAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CallerMemberNameAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CallerFilePathAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CallerLineNumberAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CallerMemberNameAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CompilerGeneratedAttribute" feature="System.Diagnostics.Debugger.IsSupported" featurevalue="false">
<!--
The attribute is used during pretty stack trace printing
Copy link
Member

@jkotas jkotas Jul 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StackTraceHidden is used for the same purpose. You can add it with the same condition.

Copy link
Contributor Author

@marek-safar marek-safar Jul 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, there is a bunch of others which should be added as conditional (e.g. whole ETW). I'd prefer to add them in follow up PRs

-->
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CompilerGlobalScopeAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
Copy link
Member

@stephentoub stephentoub Jul 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CompilerGenerated is checked by StackTrace.ToString, so this will affect Exception.StackTrace/ToString(), for example.

Copy link
Contributor Author

@marek-safar marek-safar Jul 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, that's ugly. Looking at the code it looks like CompilerGeneratedAttribute usage there is optimization like check.

We could remove the check or if it's really needed we could remove this attribute under Debug.IsEnabled condition only.

Copy link
Member

@eerhardt eerhardt Jul 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we remove this one and add it with the rest of the Debug-only attributes under the Debugger.IsSupported feature switch?

Examples of the Debug-only attributes that can be removed are:

  • DebuggerTypeProxyAttribute
  • DebuggerDisplayAttribute

Copy link
Member

@stephentoub stephentoub Jul 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the code it looks like CompilerGeneratedAttribute usage there is optimization like check.

I think it's more there in case the types in question were implemented by someone manually; it's trying to only remove the relevant frames when they're on compiler generated types for async and iterators. It may be over protective, but I'm hesitant to remove it for the purposes of this PR.

Should we remove this one and add it with the rest of the Debug-only attributes under the Debugger.IsSupported feature switch?

That sounds reasonable.

Copy link
Contributor Author

@marek-safar marek-safar Jul 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed it to be conditional

Copy link
Member

@akoeplinger akoeplinger Jul 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just to make sure I understand: in cases where debugger is disabled the StackTrace would now include the compiler-generated frames?

Copy link
Member

@stephentoub stephentoub Jul 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct: the code looking for the attribute wouldn't find it, and so wouldn't skip the frames.

<type fullname="System.Runtime.CompilerServices.IsReadOnlyAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.EnumeratorCancellationAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.ExtensionAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.IntrinsicAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.SkipLocalsInitAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.TupleElementNamesAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>

<!-- System.Runtime.Versioning -->
<type fullname="System.Runtime.Versioning.NonVersionableAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>

<!-- Microsoft.ComponentModel -->
<type fullname="System.ComponentModel.EditorBrowsableAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
</assembly>

<!-- The following attributes are generated by the compiler, so they could be in any assembly -->
<assembly fullname="*">
<!-- System.Runtime.CompilerServices -->
<type fullname="System.Runtime.CompilerServices.IsUnmanagedAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.NativeIntegerAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.NullableAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.NullableContextAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.NullablePublicOnlyAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>

<!-- Microsoft.CodeAnalysis -->
<type fullname="Microsoft.CodeAnalysis.EmbeddedAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
</assembly>
</linker>