Jtschuster/crossgen2 duplicate assembly warning#129599
Open
jtschuster wants to merge 3 commits into
Open
Conversation
Fixes dotnet#114513. crossgen2 keeps only the first --reference seen for a given assembly simple name; a later reference with the same simple name is silently dropped and the type system binds against the first one. If the inputs were actually built against the dropped assembly, crossgen2 can emit R2R code that throws MissingMethodException or otherwise fails at runtime, with no diagnostic. Detect the conflict at the dictionary-insertion site in the shared command-line helper: BuildPathDictionary now takes an optional Action<string,string,string> (simpleName, keptPath, droppedPath) that is invoked when a non-strict simple-name collision drops a file. The callback is notification-only, so the kept file always wins binding and a caller cannot change reference precedence. crossgen2's --reference option wires up ReferenceConflictWarning, which reads the kept and dropped files' module MVID and assembly version directly from metadata and warns when they differ. Identical copies of one assembly (same MVID and version) stay silent, avoiding false positives on overlapping reference globs. Only --reference opts in; input files, --inputbubbleref, ILCompiler, and ILVerify are unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add the pieces needed to express a test that drives crossgen2 with two references sharing an assembly simple name and asserts on crossgen2's console output: - CompiledAssembly.OutputSubdirectory lets assemblies that share an AssemblyName be emitted to distinct paths (IL/v1, IL/v2). It defaults to none, so existing assemblies keep their current IL/<name>.dll path. - CrossgenCompilation.ValidateResult validates crossgen2's exit code and stdout/stderr (for diagnostics like warnings), complementing the existing image-based Validate. When set, the runner lets the validator own success/failure, and only inspects the emitted image when the compilation actually produced one. - R2RTestCaseCompiler now emits deterministically, so identical sources compile to byte-identical assemblies (same module MVID). RunCrossgenCompilation returns the R2RCompilationResult and checks each input's existence by path rather than a name-keyed dictionary, which collided for same-named assemblies. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add ReferenceVersionConflictTests covering both directions of the crossgen2 reference-conflict diagnostic, using the declarative R2R test framework: - WarnsWhenReferencesHaveDifferentVersions passes two builds of ConflictingLib (AssemblyVersion 1.0.0.0 and 2.0.0.0) as references and asserts crossgen2 succeeds while emitting a "warning:" naming the assembly and both versions. - DoesNotWarnWhenReferencesAreIdentical passes two byte-identical builds of the same source (same MVID) as references and asserts no warning. The "warning:" assertion is case-insensitive, matching what the SDK's RunReadyToRunCompiler task scans stdout for to surface R2R warnings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jtschuster
added
area-ReadyToRun
and removed
area-crossgen2-coreclr
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a targeted diagnostic to crossgen2 so that when multiple --reference inputs share the same simple assembly name but differ by identity (module MVID and/or assembly version), crossgen2 emits a warning instead of silently binding to the first match.
Changes:
- Extend the shared command-line helper to optionally report when a non-strict simple-name collision causes a reference to be dropped.
- Add crossgen2-specific conflict detection that compares the kept vs. dropped reference by MVID/version and prints a warning via
SR. - Add
ILCompiler.ReadyToRun.Testscoverage for (1) conflicting versions => warning, and (2) identical copies => no warning.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/tools/Common/CommandLineHelpers.cs | Adds an opt-in callback for dropped duplicates when building the simple-name → path map. |
| src/coreclr/tools/aot/crossgen2/Crossgen2RootCommand.cs | Enables the new duplicate-drop callback for crossgen2’s --reference option only. |
| src/coreclr/tools/aot/crossgen2/ReferenceConflictWarning.cs | Implements “kept vs dropped” reference identity comparison and prints a warning for real conflicts. |
| src/coreclr/tools/aot/crossgen2/Properties/Resources.resx | Adds a localized warning message for conflicting reference versions. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RTestRunner.cs | Enhances test runner to allow validating crossgen2 stdout/stderr and to support disambiguated IL output paths. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RTestCaseCompiler.cs | Enables deterministic Roslyn emit so duplicate builds of identical source produce identical assembly identity. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/ReferenceVersionConflictTests.cs | Adds regression tests asserting warning/no-warning behavior. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/ReferenceConflict/ConflictingLibV1.cs | Test input: v1 reference assembly. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/ReferenceConflict/ConflictingLibV2.cs | Test input: v2 reference assembly. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/ReferenceConflict/ConflictingApp.cs | Test input: app compiled against v1. |
Comment on lines
219
to
221
| // Step 1: Compile all assemblies with Roslyn in order | ||
| var assemblyPaths = CompileAllAssemblies(assembliesToCompile); | ||
| CompileAllAssemblies(assembliesToCompile); | ||
|
|
Member
|
Is this possible to reach in user scenarios? Roslyn is invoking crossgen2 directly, but that's an unsupported scenarios and can have rough edges. If they weren't invoking crossgen, they would just copy the files over each other. Because warnings that cannot be NoWarn-ed, that don't fail with WarningsAsErrors are also not great, but at that point this is too much of a hassle to maintain (yeah, I know, copilot makes it easy to generate testcases for it and fix this and create docs for the new warning code, etc., but we should should still have some limits on what we want to support). |
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #114513. When two --reference files share a simple name but resolve to different assemblies (differing module MVID or assembly version), crossgen2 silently selects the first one with no diagnostic.
Detect the conflict at the dictionary-insertion site in the shared command-line helper: when a non-strict simple-name collision drops a reference, compare the dropped file against the binding winner by MVID and version and emit a warning. The check is opt-in via a new parameter so only crossgen2's --reference participates; input files, inputbubbleref, ILCompiler, and ILVerify are unchanged. Identical copies of one assembly (same MVID and version) stay silent to avoid false positives on overlapping reference globs.
Add ILCompiler.ReadyToRun.Tests regression coverage for both the conflicting-version (warns) and identical-copy (no warning) cases.