Allow resolving mscorlib with different assembly version numbers

[sharwell]

Customer scenario

Navigate to Decompiled Sources cannot resolve mscorlib when the target of a navigation operation references netstandard. This causes cascading failures in the decompilation process leading to unusable output.

Bugs this fixes

Fixes #24178

Workarounds, if any

None.

Risk

Low. The original cause of the problem and dependency on assembly unification is not well understood, but this change has been scoped to only impact mscorlib, which ILSpy assumes is always available.

Performance impact

No performance change except in cases where the new behavior is required for correctness.

Is this a regression from a previous update?

No.

Root cause analysis

The behavior was discovered early in this new feature, but remained uncorrected while we attempted to find a stable long-term resolution.

How was the bug found?

New feature testing.

Test documentation updated?

A manual test case was added for this scenario.

[jasonmalinowski]

@sharwell I tend to push back on these types of fixes because they tend to mask root causes. Sloppiness we have around other assembly handling is going to require non-trivial refactoring to fix some bugs we have in find references, for example. Do we understand why this was necessary?

(and can we get a test?)

[sharwell]

@siegfriedpammer Do you know why version 0.0.0.0 is getting requested here?

[siegfriedpammer]

I tried to reproduce this... which version of System.Collections.Immutable did you use? Will look into it...

[sharwell]

@siegfriedpammer I was using 1.4.0

[siegfriedpammer]

@sharwell Follow-up question: what target framework? netcore2.0, netstandard1.0, netstandard2.0 or portable-net45+win8+wp8+wpa81? Also, it seems the package version 1.4.0 contains an assembly with version 1.2.2, which is kinda weird... is that intended? Thanks!

[sharwell]

@siegfriedpammer Here are steps to reproduce

1. Check out sharwell/dotnet-trees@b45e8f3
2. Change the target framework of the test project to net461 here
3. On this line, try to navigate to ImmutableList.Create

📝 If you omit step 2 (i.e. use net452 instead of net461), the bug does not occur.

[siegfriedpammer]

@sharwell Here are the results of my investigation:

• When changing the target framework, the version of System.Collections.Immutable being decompiled first asks for the netstandard.dll (instead of mscorlib.dll).
• The netstandard.dll that is returned by VS is %PROGRAMFILES(x86)%\Microsoft Visual Studio\Preview\Community\MSBuild\Microsoft\Microsoft.NET.Build.Extensions\net461\lib\netstandard.dll.
• In order for us to be able to correctly resolve forwarded types and all types used in signatures, we need to load all transitive references as well. The version of netstandard.dll mentioned above indeed only contains references which have their version set to 0:
  

The reason this works in standalone ILSpy is that we manually look for an assembly with the same name but different (= latest) version in the GAC directories if the version is set to 0. See https://github.com/icsharpcode/ILSpy/blob/master/ICSharpCode.Decompiler/DotNetCore/UniversalAssemblyResolver.cs#L134 (note that this code was adapted from Mono.Cecil).

The IAssemblyResolver implementation used in Roslyn replaces all this special behavior. I think this is another instance of the problem: "Please tell me which assemblies would be loaded when executing the code in the assembly being decompiled.", because netstandard.dll is similar to a reference assembly.

[jasonmalinowski]

Can you point to the code in GitHub with the same hackery?

[sharwell]

It appears the strange behavior is fallout from dotnet/corefx#17363. It is not yet clear why this is expected to work.

[sharwell]

@weshaggard @ericstj Can you help here? This new Visual Studio feature is failing to resolve mscorlib version 0.0.0.0. It appears that you had an idea of how this resolution was supposed to work in practice, but I can't find any definition or specification for this behavior so I'm not sure what to do about it.

[weshaggard]

You can read more about this issue at https://github.com/dotnet/core-setup/issues/3297. Long story short we reuse the netstandard facade in different contexts where the assembly versions are different so in order to avoid trying to get the version correct we always pick the lowest version which is 0.0.0.0 with the expectation that will unify to whatever version is found in the context it is being used in.

[sharwell]

@weshaggard So the rule is, in some cases you are assuming that something downstream from the facade will ignore the 0.0.0.0 version and substitute something else?

1. How can I deterministically detect that an assembly is a facade with fake version numbers in the references?
2. What do I resolve the fake assembly version to?

I need a precise algorithm here because I can't rely on Fusion or assembly binding redirection.

[weshaggard]

I don't think you should be detecting this as a facade with fake version number that is just a hack.

The correct resolution is to use the context you have for the library and resolve to the assembly matching the same name in that context. If you don't have any context then you must use some default context to resolve the reference.

This is true for many cases where an application will have assemblies that reference to different versions of a dependency. To resolve them you need to resolve to the version you have in your context and verify that the one you resolved is of greater than or equal to in version.

[natidea]

Approved to merge via https://devdiv.visualstudio.com/DevDiv/_workitems/edit/558385