From 23e5d5410ed3e8bc9ab61b3685e1d3a06bdba701 Mon Sep 17 00:00:00 2001
From: Sven Boemer <sbomer@gmail.com>
Date: Tue, 2 Jan 2024 23:49:01 -0800
Subject: [PATCH] Clean up illink suppressions (#96361)

* Clean up illink suppressions

* Suppress IL2008

* Support multiple suppression xmls

CoreLib has a shared ILLink.Substitutions.LibraryBuild.xml, and
one that is specific to coreclr. Use an ItemGroup to support the use
of both.

* Suppress IL2035

* Replace unnecessary &quot;

* PR feedback

- Keep properties in existing order
- Separate PropertyGroup/ItemGroup with blank lines
- Place all NoWarns on one line

* Update eng/illink.targets

Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>

---------

Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
---
 eng/illink.targets                            | 41 ++++++++-----------
 eng/testing/tests.browser.targets             |  2 +-
 eng/testing/tests.wasm.targets                |  4 +-
 .../Microsoft.NET.Sdk.IL.targets.template     |  2 +-
 .../System.Private.CoreLib.Shared.projitems   |  1 +
 src/libraries/oob.proj                        |  2 +-
 src/libraries/sfx.proj                        |  2 +-
 src/mono/browser/browser.proj                 |  4 +-
 src/mono/browser/build/BrowserWasmApp.targets | 10 ++---
 9 files changed, 30 insertions(+), 38 deletions(-)

diff --git a/eng/illink.targets b/eng/illink.targets
index f56f799fcfcc66..0dca1cd421991a 100644
--- a/eng/illink.targets
+++ b/eng/illink.targets
@@ -27,12 +27,11 @@
     <ILLinkSubstitutionsXmlIntermediatePath>$(IntermediateOutputPath)ILLink.Substitutions.xml</ILLinkSubstitutionsXmlIntermediatePath>
     <ILLinkLinkAttributesXmlIntermediatePath>$(IntermediateOutputPath)ILLink.LinkAttributes.xml</ILLinkLinkAttributesXmlIntermediatePath>
 
-    <ILLinkSuppressionsXmlFilePrefix>$(ILLinkDirectory)ILLink.Suppressions</ILLinkSuppressionsXmlFilePrefix>
-    <ILLinkSuppressionsXmlFile>$(ILLinkSuppressionsXmlFilePrefix).xml</ILLinkSuppressionsXmlFile>
-    <ILLinkSuppressionsConfigurationSpecificXmlFile>$(ILLinkSuppressionsXmlFilePrefix).$(Configuration).xml</ILLinkSuppressionsConfigurationSpecificXmlFile>
-    <ILLinkSuppressionsLibraryBuildXmlFile>$(ILLinkSuppressionsXmlFilePrefix).LibraryBuild.xml</ILLinkSuppressionsLibraryBuildXmlFile>
+    <ILLinkSuppressionsXmlPrefix>$(ILLinkDirectory)ILLink.Suppressions</ILLinkSuppressionsXmlPrefix>
+    <ILLinkSuppressionsXml>$(ILLinkSuppressionsXmlPrefix).xml</ILLinkSuppressionsXml>
+    <ILLinkSuppressionsConfigurationSpecificXml>$(ILLinkSuppressionsXmlPrefix).$(Configuration).xml</ILLinkSuppressionsConfigurationSpecificXml>
     <!-- Only run the trim analyzer on libraries which have been annotated. -->
-    <EnableTrimAnalyzer Condition="'$(EnableTrimAnalyzer)' == '' And (Exists('$(ILLinkSuppressionsXmlFile)') Or Exists('$(ILLinkSuppressionsConfigurationSpecificXmlFile)'))">false</EnableTrimAnalyzer>
+    <EnableTrimAnalyzer Condition="'$(EnableTrimAnalyzer)' == '' And (Exists('$(ILLinkSuppressionsXml)') Or Exists('$(ILLinkSuppressionsConfigurationSpecificXml)'))">false</EnableTrimAnalyzer>
 
     <!-- if building a PDB, tell illink to rewrite the symbols file -->
     <ILLinkRewritePDBs Condition="'$(ILLinkRewritePDBs)' == '' and '$(DebugSymbols)' != 'false'">true</ILLinkRewritePDBs>
@@ -41,13 +40,15 @@
     <GenerateResourcesSubstitutions Condition="'$(GenerateResourcesSubstitutions)' == '' and '$(StringResourcesPath)' != ''">true</GenerateResourcesSubstitutions>
   </PropertyGroup>
 
-  <ItemGroup>
-    <ILLinkSuppressionsXmls Include="$(ILLinkSuppressionsXmlFile)"
-                            Condition="Exists('$(ILLinkSuppressionsXmlFile)')" />
-    <ILLinkSuppressionsXmls Include="$(ILLinkSuppressionsConfigurationSpecificXmlFile)"
-                            Condition="Exists('$(ILLinkSuppressionsConfigurationSpecificXmlFile)')" />
-    <ILLinkSuppressionsXmls Include="$(ILLinkSuppressionsLibraryBuildXmlFile)"
-                            Condition="Exists('$(ILLinkSuppressionsLibraryBuildXmlFile)')" />
+   <ItemGroup>
+    <ILLinkSuppressionsLibraryBuildXml Include="$(ILLinkSuppressionsXmlPrefix).LibraryBuild.xml"
+                                       Condition="Exists('$(ILLinkSuppressionsXmlPrefix).LibraryBuild.xml')" />
+
+    <ILLinkSuppressionsXmls Include="$(ILLinkSuppressionsXml)"
+                            Condition="Exists('$(ILLinkSuppressionsXml)')" />
+    <ILLinkSuppressionsXmls Include="$(ILLinkSuppressionsConfigurationSpecificXml)"
+                            Condition="Exists('$(ILLinkSuppressionsConfigurationSpecificXml)')" />
+    <ILLinkSuppressionsXmls Include="@(ILLinkSuppressionsLibraryBuildXml)" />
     <ILLinkSuppressionsXmls Update="@(ILLinkSuppressionsXmls)"
                             TargetPath="%(FileName).$(AssemblyName).xml" />
   </ItemGroup>
@@ -210,25 +211,15 @@
       <!-- pass the non-embedded descriptors xml file on the command line -->
       <ILLinkArgs Condition="'$(ILLinkDescriptorsLibraryBuildXml)' != ''">$(ILLinkArgs) -x "$(ILLinkDescriptorsLibraryBuildXml)"</ILLinkArgs>
       <ILLinkArgs Condition="'$(ILLinkSubstitutionsLibraryBuildXml)' != ''">$(ILLinkArgs) --substitutions "$(ILLinkSubstitutionsLibraryBuildXml)"</ILLinkArgs>
+      <ILLinkArgs Condition="'@(ILLinkSuppressionsLibraryBuildXml)' != ''">$(ILLinkArgs) --link-attributes "@(ILLinkSuppressionsLibraryBuildXml->'%(FullPath)', '" --link-attributes "')"</ILLinkArgs>
       <!-- suppress warnings with the following codes:
            IL2008: Could not find type A specified in resource B
            IL2009: Could not find method A in type B specified in resource C
-           IL2012: Could not find field A in type B specified in resource C
+           IL2121: Unnecessary UnconditionalSuppressMessage attribute
            IL2025: Duplicate preserve of A in B
-           IL2026: Calling A which has B can break functionality when trimming application code. The target method might be removed.
            IL2035: Unresolved assembly A in DynamicDependencyAttribute on B
-           IL2050: P/invoke method A declares a parameter with COM marshalling. Correctness of COM interop
-                   cannot be guaranteed after trimming. Interfaces and interface members might be removed.
       -->
-      <LinkerNoWarn>IL2008;IL2009;IL2012;IL2025;IL2026;IL2035;IL2050</LinkerNoWarn>
-      <!-- IL2032,IL2055,IL2057-IL2061: Reflection intrinsics with unknown arguments -->
-      <LinkerNoWarn>$(LinkerNoWarn);IL2032;IL2055;IL2057;IL2058;IL2059;IL2060;IL2061</LinkerNoWarn>
-      <!-- IL2062-IL2066: Unknown values passed to locations with DynamicallyAccessedMemberTypes -->
-      <LinkerNoWarn>$(LinkerNoWarn);IL2062;IL2063;IL2064;IL2065;IL2066</LinkerNoWarn>
-      <!-- IL2067-IL2091: Unsatisfied DynamicallyAccessedMembers requirements -->
-      <LinkerNoWarn>$(LinkerNoWarn);IL2067;IL2068;IL2069;IL2070;IL2071;IL2072;IL2073;IL2074;IL2075;IL2076;IL2077;IL2078;IL2079;IL2080;IL2081;IL2082;IL2083;IL2084;IL2085;IL2086;IL2087;IL2088;IL2089;IL2090;IL2091</LinkerNoWarn>
-      <!-- IL2121: Unnecessary UnconditionalSuppressMessage attribute -->
-      <LinkerNoWarn>$(LinkerNoWarn);IL2121</LinkerNoWarn>
+      <LinkerNoWarn>$(LinkerNoWarn);IL2008;IL2009;IL2121;IL2025;IL2035</LinkerNoWarn>
       <ILLinkArgs>$(ILLinkArgs) --nowarn $(LinkerNoWarn)</ILLinkArgs>
       <ILLinkArgs Condition="'$(ILLinkDisableIPConstProp)' == 'true'">$(ILLinkArgs) --disable-opt ipconstprop</ILLinkArgs>
     </PropertyGroup>
diff --git a/eng/testing/tests.browser.targets b/eng/testing/tests.browser.targets
index c1eaa6c0817db2..ea561e42ccaff4 100644
--- a/eng/testing/tests.browser.targets
+++ b/eng/testing/tests.browser.targets
@@ -60,7 +60,7 @@
 
       So, set those parameters explicitly here.
       -->
-    <_ExtraTrimmerArgs Condition="'$(WasmEnableLegacyJsInterop)' == 'false'">$(_ExtraTrimmerArgs) --substitutions &quot;$(BrowserProjectRoot)build\ILLink.Substitutions.LegacyJsInterop.xml&quot;</_ExtraTrimmerArgs>
+    <_ExtraTrimmerArgs Condition="'$(WasmEnableLegacyJsInterop)' == 'false'">$(_ExtraTrimmerArgs) --substitutions "$(BrowserProjectRoot)build\ILLink.Substitutions.LegacyJsInterop.xml"</_ExtraTrimmerArgs>
   </PropertyGroup>
 
 
diff --git a/eng/testing/tests.wasm.targets b/eng/testing/tests.wasm.targets
index 2d7ff9622aa6d7..e76f1acb374cde 100644
--- a/eng/testing/tests.wasm.targets
+++ b/eng/testing/tests.wasm.targets
@@ -64,8 +64,8 @@
 
       So, set those parameters explicitly here.
       -->
-    <_ExtraTrimmerArgs Condition="'$(WasmEnableSIMD)' == 'true' and '$(RunAOTCompilation)' == 'true'">$(_ExtraTrimmerArgs) --substitutions &quot;$(BrowserProjectRoot)build\ILLink.Substitutions.WasmIntrinsics.xml&quot;</_ExtraTrimmerArgs>
-    <_ExtraTrimmerArgs Condition="'$(WasmEnableSIMD)' != 'true'">$(_ExtraTrimmerArgs) --substitutions &quot;$(BrowserProjectRoot)build\ILLink.Substitutions.NoWasmIntrinsics.xml&quot;</_ExtraTrimmerArgs>
+    <_ExtraTrimmerArgs Condition="'$(WasmEnableSIMD)' == 'true' and '$(RunAOTCompilation)' == 'true'">$(_ExtraTrimmerArgs) --substitutions "$(BrowserProjectRoot)build\ILLink.Substitutions.WasmIntrinsics.xml"</_ExtraTrimmerArgs>
+    <_ExtraTrimmerArgs Condition="'$(WasmEnableSIMD)' != 'true'">$(_ExtraTrimmerArgs) --substitutions "$(BrowserProjectRoot)build\ILLink.Substitutions.NoWasmIntrinsics.xml"</_ExtraTrimmerArgs>
   </PropertyGroup>
 
   <ItemGroup>
diff --git a/src/coreclr/.nuget/Microsoft.NET.Sdk.IL/Microsoft.NET.Sdk.IL.targets.template b/src/coreclr/.nuget/Microsoft.NET.Sdk.IL/Microsoft.NET.Sdk.IL.targets.template
index ebb865af5582e0..7cd5c90e143741 100644
--- a/src/coreclr/.nuget/Microsoft.NET.Sdk.IL/Microsoft.NET.Sdk.IL.targets.template
+++ b/src/coreclr/.nuget/Microsoft.NET.Sdk.IL/Microsoft.NET.Sdk.IL.targets.template
@@ -77,7 +77,7 @@ Copyright (c) .NET Foundation. All rights reserved.
     <Error Condition="'@(ILResourceReference->Count())' != '1'" Text="Only one ILResourceReference can be specified" />
     <PropertyGroup>
       <_ilResourceReference>%(ILResourceReference.FullPath)</_ilResourceReference>
-      <_IldasmCommand>&quot;$(_IldasmDir)ildasm&quot;</_IldasmCommand>
+      <_IldasmCommand>"$(_IldasmDir)ildasm"</_IldasmCommand>
       <_IldasmCommand>$(_IldasmCommand) "$(_ilResourceReference)"</_IldasmCommand>
       <_IldasmCommand>$(_IldasmCommand) /OUT="$(IntermediateOutputPath)/$(MSBuildProjectName).ref.il"</_IldasmCommand>
 
diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems
index 7a2cbf5b43460b..1c1d6d3530110a 100644
--- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems
+++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems
@@ -67,6 +67,7 @@
     <ILLinkSubstitutionsXmls Include="$(ILLinkSharedDirectory)ILLink.Substitutions.Windows.xml" Condition="'$(TargetsWindows)' == 'true'" />
     <ILLinkSubstitutionsXmls Include="$(ILLinkSharedDirectory)ILLink.Substitutions.Browser.xml" Condition="'$(TargetsBrowser)' == 'true'" />
     <ILLinkLinkAttributesXmls Include="$(ILLinkSharedDirectory)ILLink.LinkAttributes.Shared.xml" />
+    <ILLinkSuppressionsLibraryBuildXml Include="$(ILLinkSharedDirectory)ILLink.Suppressions.LibraryBuild.xml" />
   </ItemGroup>
   <PropertyGroup>
     <ILLinkDescriptorsLibraryBuildXml>$(ILLinkSharedDirectory)ILLink.Descriptors.LibraryBuild.xml</ILLinkDescriptorsLibraryBuildXml>
diff --git a/src/libraries/oob.proj b/src/libraries/oob.proj
index b879127a13a816..34823cd1ba4fe3 100644
--- a/src/libraries/oob.proj
+++ b/src/libraries/oob.proj
@@ -55,7 +55,7 @@
       <OOBILLinkArgs>$(ILLinkArgs)</OOBILLinkArgs>
       <!-- Unnecessary suppressions - disable for now since we didn't clean the runtime yet -->
       <OOBILLinkArgs>$(ILLinkArgs) --nowarn IL2121</OOBILLinkArgs>
-      <OOBILLinkArgs Condition="'@(OOBLibrarySuppressionsXml)' != ''" >$(OOBILLinkArgs) --link-attributes &quot;@(OOBLibrarySuppressionsXml->'%(FullPath)', '&quot; --link-attributes &quot;')&quot;</OOBILLinkArgs>
+      <OOBILLinkArgs Condition="'@(OOBLibrarySuppressionsXml)' != ''" >$(OOBILLinkArgs) --link-attributes "@(OOBLibrarySuppressionsXml->'%(FullPath)', '" --link-attributes "')"</OOBILLinkArgs>
     </PropertyGroup>
 
     <MakeDir Directories="$(OOBAssembliesTrimDir)" />
diff --git a/src/libraries/sfx.proj b/src/libraries/sfx.proj
index 278b269eb219b4..9b174d57c90f0a 100644
--- a/src/libraries/sfx.proj
+++ b/src/libraries/sfx.proj
@@ -98,7 +98,7 @@
       <SharedFrameworkILLinkArgs>$(ILLinkArgs)</SharedFrameworkILLinkArgs>
       <!-- update debug symbols -->
       <SharedFrameworkILLinkArgs>$(SharedFrameworkILLinkArgs) -b true</SharedFrameworkILLinkArgs>
-      <SharedFrameworkILLinkArgs Condition="'@(SharedFrameworkSuppressionsXml)' != ''" >$(SharedFrameworkILLinkArgs) --link-attributes &quot;@(SharedFrameworkSuppressionsXml, '&quot; --link-attributes &quot;')&quot;</SharedFrameworkILLinkArgs>
+      <SharedFrameworkILLinkArgs Condition="'@(SharedFrameworkSuppressionsXml)' != ''" >$(SharedFrameworkILLinkArgs) --link-attributes "@(SharedFrameworkSuppressionsXml, '" --link-attributes "')"</SharedFrameworkILLinkArgs>
     </PropertyGroup>
 
     <ItemGroup>
diff --git a/src/mono/browser/browser.proj b/src/mono/browser/browser.proj
index 1de2f40fdbace1..7e7453a4326494 100644
--- a/src/mono/browser/browser.proj
+++ b/src/mono/browser/browser.proj
@@ -263,8 +263,8 @@
       <EmccExportedFunction Include="_emscripten_main_runtime_thread_id"  />
     </ItemGroup>
     <PropertyGroup>
-      <_EmccExportedLibraryFunction>&quot;[@(EmccExportedLibraryFunction -> '%27%(Identity)%27', ',')]&quot;</_EmccExportedLibraryFunction>
-      <_EmccExportedRuntimeMethods>&quot;[@(EmccExportedRuntimeMethod -> '%27%(Identity)%27', ',')]&quot;</_EmccExportedRuntimeMethods>
+      <_EmccExportedLibraryFunction>"[@(EmccExportedLibraryFunction -> '%27%(Identity)%27', ',')]"</_EmccExportedLibraryFunction>
+      <_EmccExportedRuntimeMethods>"[@(EmccExportedRuntimeMethod -> '%27%(Identity)%27', ',')]"</_EmccExportedRuntimeMethods>
       <_EmccExportedFunctions>@(EmccExportedFunction -> '%(Identity)',',')</_EmccExportedFunctions>
       <EmccInitialHeapSize>16777216</EmccInitialHeapSize>
       <EmccStackSize>5MB</EmccStackSize>
diff --git a/src/mono/browser/build/BrowserWasmApp.targets b/src/mono/browser/build/BrowserWasmApp.targets
index bded4ae2b44993..7ca72154951a9c 100644
--- a/src/mono/browser/build/BrowserWasmApp.targets
+++ b/src/mono/browser/build/BrowserWasmApp.targets
@@ -46,9 +46,9 @@
 
     <_WasmGenerateAppBundleDependsOn>_GetWasmGenerateAppBundleDependencies;$(_WasmGenerateAppBundleDependsOn)</_WasmGenerateAppBundleDependsOn>
 
-    <_ExtraTrimmerArgs Condition="'$(WasmEnableSIMD)' == 'true' and '$(RunAOTCompilation)' == 'true'">$(_ExtraTrimmerArgs) --substitutions &quot;$(MSBuildThisFileDirectory)ILLink.Substitutions.WasmIntrinsics.xml&quot;</_ExtraTrimmerArgs>
-    <_ExtraTrimmerArgs Condition="'$(WasmEnableSIMD)' != 'true'">$(_ExtraTrimmerArgs) --substitutions &quot;$(MSBuildThisFileDirectory)ILLink.Substitutions.NoWasmIntrinsics.xml&quot;</_ExtraTrimmerArgs>
-    <_ExtraTrimmerArgs Condition="'$(WasmEnableLegacyJsInterop)' == 'false'">$(_ExtraTrimmerArgs) --substitutions &quot;$(MSBuildThisFileDirectory)ILLink.Substitutions.LegacyJsInterop.xml&quot;</_ExtraTrimmerArgs>
+    <_ExtraTrimmerArgs Condition="'$(WasmEnableSIMD)' == 'true' and '$(RunAOTCompilation)' == 'true'">$(_ExtraTrimmerArgs) --substitutions "$(MSBuildThisFileDirectory)ILLink.Substitutions.WasmIntrinsics.xml"</_ExtraTrimmerArgs>
+    <_ExtraTrimmerArgs Condition="'$(WasmEnableSIMD)' != 'true'">$(_ExtraTrimmerArgs) --substitutions "$(MSBuildThisFileDirectory)ILLink.Substitutions.NoWasmIntrinsics.xml"</_ExtraTrimmerArgs>
+    <_ExtraTrimmerArgs Condition="'$(WasmEnableLegacyJsInterop)' == 'false'">$(_ExtraTrimmerArgs) --substitutions "$(MSBuildThisFileDirectory)ILLink.Substitutions.LegacyJsInterop.xml"</_ExtraTrimmerArgs>
 
     <WasmUseEMSDK_PATH Condition="'$(WasmUseEMSDK_PATH)' == '' and '$(EMSDK_PATH)' != '' and Exists('$(MSBuildThisFileDirectory)WasmApp.InTree.targets')">true</WasmUseEMSDK_PATH>
     <WasmClang>emcc</WasmClang>
@@ -427,8 +427,8 @@
       <_WasmSIMDLib Condition="'$(WasmEnableSIMD)' != 'true'">libmono-wasm-nosimd.a</_WasmSIMDLib>
       <_WasmSIMDLibToExclude Condition="'$(WasmEnableSIMD)' != 'true'">libmono-wasm-simd.a</_WasmSIMDLibToExclude>
       <_WasmSIMDLibToExclude Condition="'$(WasmEnableSIMD)' == 'true'">libmono-wasm-nosimd.a</_WasmSIMDLibToExclude>
-      <_EmccExportedLibraryFunction>&quot;[@(EmccExportedLibraryFunction -> '%27%(Identity)%27', ',')]&quot;</_EmccExportedLibraryFunction>
-      <_EmccExportedRuntimeMethods>&quot;[@(EmccExportedRuntimeMethod -> '%27%(Identity)%27', ',')]&quot;</_EmccExportedRuntimeMethods>
+      <_EmccExportedLibraryFunction>"[@(EmccExportedLibraryFunction -> '%27%(Identity)%27', ',')]"</_EmccExportedLibraryFunction>
+      <_EmccExportedRuntimeMethods>"[@(EmccExportedRuntimeMethod -> '%27%(Identity)%27', ',')]"</_EmccExportedRuntimeMethods>
       <_EmccExportedFunctions>@(EmccExportedFunction -> '%(Identity)',',')</_EmccExportedFunctions>
     </PropertyGroup>
     <ItemGroup>