diff --git a/docs/design/tools/illink/task-cache.md b/docs/design/tools/illink/task-cache.md index 21ca8a50e85aa5..55662b0ce75432 100644 --- a/docs/design/tools/illink/task-cache.md +++ b/docs/design/tools/illink/task-cache.md @@ -62,7 +62,7 @@ internal sealed class ILLinkCache - Defer identifying the runtime that executes ILLink; do not include a runtime version or framework description in the key. The task runs in MSBuild, so its CoreLib informational version and framework description are not reliable identities for the child linker's runtime. The runtimeconfig describes requested frameworks and roll-forward policy, not the exact resolved CoreLib. Finding that CoreLib would require resolved-runtime information from the caller, host-resolution plumbing, or an additional process; defer that complexity and startup cost. - This deliberately accepts that runtime installation changes not represented by other keyed inputs may reuse an entry; callers needing invalidation for those changes must clear or isolate the cache. - Include platform, architecture, and culture. Do not enumerate or hash inherited environment variables, or bypass caching based on them. This deliberately keeps v1 simple rather than maintaining a partial list of runtime/loader settings. Environment changes are reflected only when they change other keyed inputs, such as the selected host path or culture. Callers must disable caching when inherited settings introduce otherwise untracked dependencies, affect outputs, or require tool-execution side effects (for example, startup hooks or profilers). -- Bypass caching for non-whitespace `ExtraArgs`, custom steps/data, dependency-dump options, and explicit `ToolTask.EnvironmentVariables` overrides. These are caller-supplied inputs that v1 does not model in its key and can introduce undeclared dependencies, external outputs, or side effects. These invocations still run the linker normally; no extra-argument parsing or runtime-specific exceptions are supported. +- Bypass caching for arbitrary `ExtraArgs`, custom steps/data, dependency-dump options, and explicit `ToolTask.EnvironmentVariables` overrides. These are caller-supplied inputs that v1 does not model in its key and can introduce undeclared dependencies, external outputs, or side effects. The supported `ExtraArgs` shape for runtime's library builds is `--ignore-link-attributes true` followed by zero or more `--link-attributes FILE`, `--substitutions FILE`, or `-d DIRECTORY` pairs. Hash every referenced XML file and every top-level `.dll`, `.exe`, and `.winmd` candidate in each search directory, including assembly sidecars/modules/resources. Preserve directory order in the arguments and leave linker resolution unchanged; adding, changing, or removing candidates invalidates the key, including shadowed candidates. An unreadable directory or uninspectable candidate bypasses caching. All other SDK options passed through `ExtraArgs` remain subject to the bypass. - Log and bypass caching if required files cannot be read or an input assembly cannot be inspected, including PE images without managed metadata. Normal linking determines whether those inputs are acceptable. Input files must remain stable during hashing and linking. ## Entry layout diff --git a/docs/tools/illink/illink-tasks.md b/docs/tools/illink/illink-tasks.md index 209a3f8b7a631b..c6f092e7fca48e 100644 --- a/docs/tools/illink/illink-tasks.md +++ b/docs/tools/illink/illink-tasks.md @@ -67,9 +67,11 @@ clear or isolate the cache when that distinction is required. Cache hits replace the output directory without running ILLink. Directories are created in the cache only when storing a successful result. Hits do not replay warnings or other linker diagnostics. -Caching is bypassed with a diagnostic for non-whitespace `ExtraArgs`, custom steps/data, -dependency-dump options, and explicit task environment overrides. Options supplied through -`ExtraArgs` remain unsupported for caching; these invocations still run the linker normally. +Caching is bypassed with a diagnostic for arbitrary `ExtraArgs`, custom steps/data, +dependency-dump options, and explicit task environment overrides. Runtime's library-build +arguments for link attributes, substitutions, and assembly search directories are supported. +Their XML files and search-directory assembly candidates (including sidecars) are content-hashed, +without changing assembly resolution. Other SDK options supplied through `ExtraArgs` remain unsupported. Inherited environment variables are not tracked; disable caching when they affect outputs or dependencies beyond the keyed inputs, or require tool-execution side effects such as startup hooks or profiling. Unreadable inputs or linker files also diff --git a/eng/illink.targets b/eng/illink.targets index 248e5f6a34e50a..714ac859810554 100644 --- a/eng/illink.targets +++ b/eng/illink.targets @@ -178,10 +178,9 @@ + <_ILLinkHasCustomArgs Condition="'$(ILLinkArgs)' != ''">true $(ILLinkArgs) --ignore-link-attributes true - - $(ILLinkArgs) --skip-unresolved true + $(LinkerNoWarn);IL2008;IL2009;IL2121;IL2025;IL2035 + + + + $(ILLinkArgs) --trim-mode skip @@ -215,18 +226,30 @@ $(ILLinkArgs) -x "@(ILLinkDescriptorsLibraryBuildXml->'%(FullPath)', '" -x "')" $(ILLinkArgs) --substitutions "$(ILLinkSubstitutionsLibraryBuildXml)" $(ILLinkArgs) --link-attributes "@(ILLinkSuppressionsLibraryBuildXml->'%(FullPath)', '" --link-attributes "')" - - $(LinkerNoWarn);IL2008;IL2009;IL2121;IL2025;IL2035 $(ILLinkArgs) --nowarn $(LinkerNoWarn) $(ILLinkArgs) --disable-opt ipconstprop + + <_ILLinkTrimMode>skip + <_ILLinkDefaultAction>skip + <_ILLinkNoWarn>$(LinkerNoWarn) + <_ILLinkRemoveSymbols>true + <_ILLinkRemoveSymbols Condition="'$(ILLinkRewritePDBs)' == 'true' and Exists('$(ILLinkTrimAssemblySymbols)')">false + <_ILLinkPreserveSymbolPaths Condition="'$(_ILLinkRemoveSymbols)' == 'false' and '$(DeterministicSourcePaths)' == 'true'">true + <_ILLinkIPConstProp Condition="'$(ILLinkDisableIPConstProp)' == 'true'">false + $(ILLinkArgs) --substitutions "$(ILLinkSubstitutionsLibraryBuildXml)" + $(ILLinkArgs) --link-attributes "@(ILLinkSuppressionsLibraryBuildXml->'%(FullPath)', '" --link-attributes "')" + + + + <_ILLinkAssemblyToTrim Include="$(ILLinkTrimInputPath)$(TargetName)$(TargetExt)"> + link + + <_ILLinkRootDescriptors Include="@(ILLinkDescriptorsLibraryBuildXml->'%(FullPath)')" + Condition="'$(_ILLinkHasCustomArgs)' != 'true'" /> + + @@ -264,8 +287,15 @@ $(ILLinkArgs) -d @(_DependencyDirectories->'"%(Identity)"', ' -d ') - $(NetCoreAppCurrent)-$(TargetOS) true + + IL2121 @@ -37,8 +39,6 @@ $(ILLinkArgs) - - $(ILLinkArgs) --nowarn IL2121 $(OOBILLinkArgs) --link-attributes "@(OOBLibrarySuppressionsXml->'%(FullPath)', '" --link-attributes "')" @@ -52,6 +52,7 @@ ReferenceAssemblyPaths="@(OOBAssemblyReference)" OutputDirectory="$(OOBAssembliesTrimDir)" ExtraArgs="$(OOBILLinkArgs)" + NoWarn="$(OOBILLinkNoWarn)" ToolExe="$(_DotNetHostFileName)" ToolPath="$(_DotNetHostDirectory)" /> diff --git a/src/libraries/sfx-finish.proj b/src/libraries/sfx-finish.proj index 47e300d3adc289..a0af3730354192 100644 --- a/src/libraries/sfx-finish.proj +++ b/src/libraries/sfx-finish.proj @@ -124,8 +124,6 @@ $(ILLinkArgs) - - $(SharedFrameworkILLinkArgs) -b true $(SharedFrameworkILLinkArgs) --link-attributes "@(SharedFrameworkSuppressionsXml, '" --link-attributes "')" @@ -141,6 +139,7 @@ RootAssemblyNames="@(_SharedFrameworkRootAssemblyName)" OutputDirectory="$(SharedFrameworkAssembliesTrimDir)" ExtraArgs="$(SharedFrameworkILLinkArgs)" + RemoveSymbols="false" ToolExe="$(_DotNetHostFileName)" ToolPath="$(_DotNetHostDirectory)" /> diff --git a/src/tools/illink/src/ILLink.Tasks/LinkTask.cs b/src/tools/illink/src/ILLink.Tasks/LinkTask.cs index 900b945591367b..0f43e75a857db0 100644 --- a/src/tools/illink/src/ILLink.Tasks/LinkTask.cs +++ b/src/tools/illink/src/ILLink.Tasks/LinkTask.cs @@ -310,15 +310,16 @@ private bool TryComputeCacheKey(string pathToTool, string commandLineCommands, s return false; } - if (!string.IsNullOrWhiteSpace(ExtraArgs)) - { - Log.LogMessage(MessageImportance.Low, "ILLink cache bypassed: extra arguments are not supported."); - return false; - } - try { var files = new SortedSet(StringComparer.Ordinal); + var directories = new SortedSet(StringComparer.Ordinal); + if (!TryAddCacheableExtraArgsInputs(files, directories)) + { + Log.LogMessage(MessageImportance.Low, "ILLink cache bypassed: extra arguments are not supported."); + return false; + } + var assemblies = new HashSet(StringComparer.Ordinal); foreach (ITaskItem assembly in AssemblyPaths) AddAssembly(assembly.ItemSpec); @@ -327,6 +328,22 @@ private bool TryComputeCacheKey(string pathToTool, string commandLineCommands, s foreach (ITaskItem descriptor in RootDescriptorFiles ?? Array.Empty()) AddFile(descriptor.ItemSpec); + // Preserve AssemblyResolver's search behavior instead of replacing -d with + // explicit references. Include every candidate so additions and shadowing invalidate. + foreach (string directory in directories) + { + foreach (string file in Directory.EnumerateFiles(directory)) + { + string extension = Path.GetExtension(file); + if (extension.Equals(".dll", StringComparison.OrdinalIgnoreCase) || + extension.Equals(".exe", StringComparison.OrdinalIgnoreCase) || + extension.Equals(".winmd", StringComparison.OrdinalIgnoreCase)) + { + AddAssembly(file); + } + } + } + #pragma warning disable IL3000 // MSBuild tasks are loaded from assemblies on disk. AddFile(typeof(ILLink).Assembly.Location); #pragma warning restore IL3000 @@ -408,6 +425,80 @@ void AddAssembly(string path) } } + private bool TryAddCacheableExtraArgsInputs(SortedSet files, SortedSet directories) + { + ReadOnlySpan arguments = ExtraArgs.AsSpan().Trim(); + if (arguments.IsEmpty) + return true; + + // The runtime library builds pass this policy through ExtraArgs. Keep the + // supported shape narrow so arbitrary linker arguments continue to bypass caching. + if (!TryReadArgument(ref arguments, out string option) || + option != "--ignore-link-attributes" || + !TryReadArgument(ref arguments, out string value) || + value != "true") + { + return false; + } + + while (!arguments.TrimStart().IsEmpty) + { + if (!TryReadArgument(ref arguments, out option) || + option is not ("--link-attributes" or "--substitutions" or "-d") || + !TryReadArgument(ref arguments, out string path) || + string.IsNullOrEmpty(path)) + { + return false; + } + + if (option == "-d") + directories.Add(Path.GetFullPath(path)); + else + files.Add(Path.GetFullPath(path)); + } + + return true; + + static bool TryReadArgument(ref ReadOnlySpan arguments, out string argument) + { + arguments = arguments.TrimStart(); + if (arguments.IsEmpty) + { + argument = string.Empty; + return false; + } + + if (arguments[0] == '"') + { + int closingQuote = arguments.Slice(1).IndexOf('"'); + if (closingQuote < 0) + { + argument = string.Empty; + return false; + } + + argument = arguments.Slice(1, closingQuote).ToString(); + arguments = arguments.Slice(closingQuote + 2); + return arguments.IsEmpty || char.IsWhiteSpace(arguments[0]); + } + + int end = 0; + while (end < arguments.Length && !char.IsWhiteSpace(arguments[end])) + end++; + + ReadOnlySpan token = arguments.Slice(0, end); + if (token.IndexOf('"') >= 0) + { + argument = string.Empty; + return false; + } + + argument = token.ToString(); + arguments = arguments.Slice(end); + return true; + } + } + private string _illinkPath = ""; public string ILLinkPath diff --git a/src/tools/illink/test/ILLink.Tasks.Tests/ILLink.Tasks.Tests.cs b/src/tools/illink/test/ILLink.Tasks.Tests/ILLink.Tasks.Tests.cs index 90cc266ac8eac1..84135d1a807f87 100644 --- a/src/tools/illink/test/ILLink.Tasks.Tests/ILLink.Tasks.Tests.cs +++ b/src/tools/illink/test/ILLink.Tasks.Tests/ILLink.Tasks.Tests.cs @@ -1629,11 +1629,144 @@ public void CacheKeyTracksFileContentsNotTimestamps(string relativePath) } [Theory] - [InlineData("--help")] - [InlineData("--ignore-link-attributes true")] + [InlineData("--link-attributes", 0)] + [InlineData("--link-attributes", 1)] + [InlineData("--substitutions", 0)] + [InlineData("--substitutions", 1)] + public void CacheKeyTracksRuntimeXmlFiles(string option, int changedFile) + { + using var test = new OutputDirectoryTest(); + string host = PrepareCacheKeyTest(test); + string[] linkAttributesFiles = + { + Path.Combine(test.Root, "first link attributes.xml"), + Path.Combine(test.Root, "second link attributes.xml") + }; + foreach (string file in linkAttributesFiles) + File.WriteAllText(file, "contents"); + + test.Task.ExtraArgs = $" --ignore-link-attributes true {option} \"{linkAttributesFiles[0]}\" {option} \"{linkAttributesFiles[1]}\""; + Assert.True(test.Task.TryGetCacheKey(host, out string original)); + + File.AppendAllText(linkAttributesFiles[changedFile], "changed"); + Assert.True(test.Task.TryGetCacheKey(host, out string changed)); + Assert.NotEqual(original, changed); + } + + [Theory] + [InlineData(".dll")] + [InlineData(".exe")] + [InlineData(".winmd")] + public void CacheKeyTracksRuntimeSearchDirectoryCandidates(string extension) + { + using var test = new OutputDirectoryTest(); + string host = PrepareCacheKeyTest(test); + string first = Path.Combine(test.Root, "first search directory"); + string second = Path.Combine(test.Root, "second search directory"); + Directory.CreateDirectory(first); + Directory.CreateDirectory(second); + test.Task.ExtraArgs = $"--ignore-link-attributes true -d \"{first}\" -d \"{second}\""; + Assert.True(test.Task.TryGetCacheKey(host, out string empty)); + + string candidate = Path.Combine(second, "Dependency" + extension); + WriteTestAssembly(candidate); + Assert.True(test.Task.TryGetCacheKey(host, out string added)); + Assert.NotEqual(empty, added); + WriteTestAssembly(candidate, "changed"); + Assert.True(test.Task.TryGetCacheKey(host, out string changed)); + Assert.NotEqual(added, changed); + + string earlierCandidate = Path.Combine(first, Path.GetFileName(candidate)); + File.Copy(candidate, earlierCandidate); + Assert.True(test.Task.TryGetCacheKey(host, out string shadowed)); + Assert.NotEqual(changed, shadowed); + test.Task.ExtraArgs = $"--ignore-link-attributes true -d \"{second}\" -d \"{first}\""; + Assert.True(test.Task.TryGetCacheKey(host, out string reordered)); + Assert.NotEqual(shadowed, reordered); + + File.Delete(candidate); + File.Delete(earlierCandidate); + test.Task.ExtraArgs = $"--ignore-link-attributes true -d \"{first}\" -d \"{second}\""; + Assert.True(test.Task.TryGetCacheKey(host, out string removed)); + Assert.Equal(empty, removed); + } + + [Theory] + [InlineData("Dependency.pdb")] + [InlineData("Dependency.dll.mdb")] + [InlineData("Dependency.dll.config")] + [InlineData("fr/Dependency.resources.dll")] + public void CacheKeyTracksRuntimeSearchDirectorySidecars(string relativePath) + { + using var test = new OutputDirectoryTest(); + string host = PrepareCacheKeyTest(test); + string directory = Path.Combine(test.Root, "search"); + Directory.CreateDirectory(directory); + WriteTestAssembly(Path.Combine(directory, "Dependency.dll")); + test.Task.ExtraArgs = $"--ignore-link-attributes true -d \"{directory}\""; + Assert.True(test.Task.TryGetCacheKey(host, out string original)); + + string sidecar = Path.Combine(directory, relativePath); + Directory.CreateDirectory(Path.GetDirectoryName(sidecar)); + File.WriteAllText(sidecar, "contents"); + Assert.True(test.Task.TryGetCacheKey(host, out string added)); + Assert.NotEqual(original, added); + File.AppendAllText(sidecar, "changed"); + Assert.True(test.Task.TryGetCacheKey(host, out string changed)); + Assert.NotEqual(added, changed); + File.Delete(sidecar); + Assert.True(test.Task.TryGetCacheKey(host, out string removed)); + Assert.Equal(original, removed); + } + + [Theory] + [InlineData("missing-directory")] + [InlineData("invalid-candidate")] + [InlineData("metadata-less-candidate")] + public void CacheKeyBypassesUnmodeledSearchDirectoryInputs(string reason) + { + using var test = new OutputDirectoryTest(); + string host = PrepareCacheKeyTest(test); + string directory = Path.Combine(test.Root, "search"); + if (reason != "missing-directory") + { + Directory.CreateDirectory(directory); + string candidate = Path.Combine(directory, "Dependency.dll"); + if (reason == "invalid-candidate") + File.WriteAllText(candidate, "invalid"); + else + WriteMetadataLessPE(candidate); + } + test.Task.ExtraArgs = $"--ignore-link-attributes true -d \"{directory}\""; + + Assert.False(test.Task.TryGetCacheKey(host, out string key)); + Assert.Empty(key); + Assert.Contains(test.BuildEngine.Messages, message => + message.Message.StartsWith("ILLink cache bypassed: input identity could not be computed:")); + } + + [Fact] + public void CacheKeyAllowsRuntimeIgnoreLinkAttributesArgument() + { + using var test = new OutputDirectoryTest(); + string host = PrepareCacheKeyTest(test); + test.Task.ExtraArgs = "--ignore-link-attributes true"; + + Assert.True(test.Task.TryGetCacheKey(host, out string key)); + Assert.Matches("^[0-9a-f]{64}$", key); + } + + [Theory] + [InlineData("--ignore-link-attributes false")] [InlineData("--link-attributes attributes.xml")] - [InlineData("--ignore-link-attributes true --substitutions substitutions.xml")] - [InlineData("--ignore-link-attributes true -d assemblies")] + [InlineData("--ignore-link-attributes true --help")] + [InlineData("--ignore-link-attributes true --link-attributes")] + [InlineData("--ignore-link-attributes true --link-attributes \"\"")] + [InlineData("--ignore-link-attributes true --link-attributes \"unterminated")] + [InlineData("--ignore-link-attributes true --substitutions")] + [InlineData("--ignore-link-attributes true -d")] + [InlineData("--ignore-link-attributes true -d \"\"")] + [InlineData("--ignore-link-attributes true -d \"unterminated")] public void CacheKeyBypassesUnsupportedExtraArgs(string extraArgs) { using var test = new OutputDirectoryTest(); @@ -1646,6 +1779,22 @@ public void CacheKeyBypassesUnsupportedExtraArgs(string extraArgs) message.Message.StartsWith("ILLink cache bypassed: extra arguments are not supported.")); } + [Theory] + [InlineData("--link-attributes")] + [InlineData("--substitutions")] + public void CacheKeyBypassesMissingRuntimeXmlFile(string option) + { + using var test = new OutputDirectoryTest(); + string host = PrepareCacheKeyTest(test); + string missing = Path.Combine(test.Root, "missing link attributes.xml"); + test.Task.ExtraArgs = $"--ignore-link-attributes true {option} \"{missing}\""; + + Assert.False(test.Task.TryGetCacheKey(host, out string key)); + Assert.Empty(key); + Assert.Contains(test.BuildEngine.Messages, message => + message.Message.StartsWith("ILLink cache bypassed: input identity could not be computed:")); + } + [Theory] [InlineData(null)] [InlineData("")]