Skip to content

Detect multiple conflicting assemblies being passed to crossgen #117786

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

Closed
wants to merge 1 commit into from
Closed

Detect multiple conflicting assemblies being passed to crossgen #117786

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 16 additions & 0 deletions src/coreclr/tools/aot/crossgen2/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,22 @@ public int Run()
}
}

Dictionary<string, AssemblyNameInfo> allAssemblyNames = new ();
Copy link
Member

Choose a reason for hiding this comment

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

I think this is too late to detect duplicates.

crossgen, ilc and ilverify are all on first reference wins plan for references and ignore duplicates.

The duplicates are eliminated very early during command line parsing at

runtime/src/coreclr/tools/Common/CommandLineHelpers.cs

Lines 347 to 358 in 0621e64

if (dictionary.TryGetValue(simpleName, out string otherFullFileName))
{
if (strict)
{
throw new CommandLineException("Multiple input files matching same simple name " +
fullFileName + " " + otherFullFileName);
}
}
else
{
dictionary.Add(simpleName, fullFileName);
}
.

strict flag is true for input assemblies and it is false for references.

Copy link
Member

Choose a reason for hiding this comment

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

Changing the policy for duplicates is likely going to break some of the folks who are invoking crossgen manually. Manual invoked crossgen command line often references using wildcards, like -r somepath\*.dll -r someotherpath\*.dll and the different paths tend to have duplicates that are harmless. These folks will see wall of warnings with this change.

We do not officially support invoking crossgen manually. You won't run into this with PublishReadyToRun that's our officially supported way to invoke crossgen. I am not sure whether this is worth fixing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for investigating this. I'll figure out whether it's worth doing the work to actually detect this scenario robustly and generate warnings for it.

foreach (var rModule in _referenceableModules)
{
// Check for a loose match (two assemblies with the same name, i.e. 'System.Private.CoreLib')
var moduleName = rModule.Assembly.GetName();
if (allAssemblyNames.TryGetValue(moduleName.Name, out var firstModule))
{
// Now check whether the full name is an exact match. If it's not, this indicates that the user accidentally provided two versions of the same
// assembly during compilation, and the results will probably not be what they want, so we should generate a warning message.
if (firstModule.FullName != moduleName.FullName)
Copy link
Member

Choose a reason for hiding this comment

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

Identical full names do not imply that assembly content is identical. It is very common for two assemblies to have identical full name but have different content. For example., assemblies from given nuget package that target different TFMs have identical full name.

If we want to change how we treat duplicate references, we should disallow or warn on any duplicated simple names, similar how the strict flag works for input assemblies.

Console.Error.WriteLine($"warning: Two or more modules named '{moduleName.Name}' were provided for R2R compilation. First encountered: '{firstModule.FullName}'. Additional: '{moduleName.FullName}'");
}
else
allAssemblyNames.Add(moduleName.Name, moduleName);
}

string systemModuleName = Get(_command.SystemModuleName) ?? Helpers.DefaultSystemModule;
_typeSystemContext.SetSystemModule((EcmaModule)_typeSystemContext.GetModuleForSimpleName(systemModuleName));
ReadyToRunCompilerContext typeSystemContext = _typeSystemContext;
Expand Down
Loading