Deterministic MVIDs is needed for assemblies referencing System.Private.CoreLib across different architectures

[akoeplinger]

We're hitting an issue where the MVID of assemblies is different depending on the architecture of the runtime pack, e.g. x64 vs. arm64, for assemblies that are 100% managed code like System.Collections.dll or System.Private.Uri.dll but compile against System.Private.CoreLib.

This causes an issue for Xamarin Android and other internal teams that use the MVID to deduplicate assemblies that are the same across architectures (right now only System.Private.CoreLib.dll is actually architecture-specific).

The reason this is showing up now is because of a linker/Cecil fix (dotnet/linker#2203, dotnet/cecil#31) which fixes the PDB ID calculation which in turn affects the MVID calculation (we actually just got lucky in net6 since we run the linker against all assemblies in the libraries build which "unified" the MVID).

Since the PDB is storing the MVID of all assembly references in "Compilation Metadata References" (described in https://github.com/dotnet/runtime/blob/424a09cb81c678fb1ba1c27211b80aba2de070ad/docs/design/specs/PortablePdb-Metadata.md#CompilationMetadataReferences) we now get a different MVID on the assemblies because these projects compile directly against System.Private.CoreLib.dll whose MVID gets embedded in the PDB:

There is interest to backport the linker/Cecil fix to net6 so I discussed with @vitek-karas and we agreed the least invasive change is to simply hardcode System.Private.CoreLib.dll as the only architecture-specific assembly in Xamarin Android for net6. EDIT it turns out this plan won't work since the Mono AOT compiler verifies the MVIDs of assembly references and will error out if they don't match with the actual assembly.

For net7/net8 we'd like to come up with a "proper" fix though, some ideas:

• Create a reference assembly for System.Private.CoreLib and compile against that one, instead of the real assembly
  • I tried using Roslyn's /refout feature but the reference assembly created by it still differs between architectures since Roslyn includes private fields like Padding.cs#L9-L16, so this'd likely need to be a real reference assembly
• Make System.Private.CoreLib architecture-agnostic by removing all of the TARGET_64BIT and TARGET_32BIT ifdefs.
• Tweak the (official) build so we only ever build the non-architecture-specific assemblies once and copy them into the respective runtime packs, ensuring that we only have a single version of the assemblies
  • As an extension we could split out the assemblies into another package, so they're not duplicated in the runtime packs. This would also help reducing download size.

/cc @ViktorHofer @vitek-karas @agocke @marek-safar @steveisok

[ghost]

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details

---

We're hitting an issue where the MVID of assemblies is different depending on the architecture of the runtime pack, e.g. x64 vs. arm64, for assemblies that are 100% managed code like System.Collections.dll or System.Private.Uri.dll but compile against System.Private.CoreLib.

This causes an issue for Xamarin Android and other internal teams that use the MVID to deduplicate assemblies that are the same across architectures (right now only System.Private.CoreLib.dll is actually architecture-specific).

The reason this is showing up now is because of a linker/Cecil fix (dotnet/linker#2203, dotnet/cecil#31) which fixes the PDB ID calculation which in turn affects the MVID calculation (we actually just got lucky in net6 since we run the linker against all assemblies in the libraries build which "unified" the MVID).

Since the PDB is storing the MVID of all assembly references in "Compilation Metadata References" (described in https://github.com/dotnet/runtime/blob/424a09cb81c678fb1ba1c27211b80aba2de070ad/docs/design/specs/PortablePdb-Metadata.md#CompilationMetadataReferences) we now get a different MVID on the assemblies because these projects compile directly against System.Private.CoreLib.dll whose MVID gets embedded in the PDB:

There is interest to backport the linker/Cecil fix to net6 so I discussed with @vitek-karas and we agreed the least invasive change is to simply hardcode System.Private.CoreLib.dll as the only architecture-specific assembly in Xamarin Android for net6.

For net7/net8 we'd like to come up with a "proper" fix though, some ideas:

• Create a reference assembly for System.Private.CoreLib and compile against that one, instead of the real assembly
  • I tried using Roslyn's /refout feature but the reference assembly created by it still differs between architectures since Roslyn includes private fields like Padding.cs#L9-L16, so this'd likely need to be a real reference assembly
• Tweak the (official) build so we only ever build the non-architecture-specific assemblies once and copy them into the respective runtime packs, ensuring that we only have a single version of the assemblies
  • As an extension we could split out the assemblies into another package, so they're not duplicated in the runtime packs. This would also help reducing download size.

/cc @ViktorHofer @vitek-karas @agocke @marek-safar @steveisok

Author:	akoeplinger
Assignees:	-
Labels:	discussion, area-Infrastructure
Milestone:	7.0.0

[akoeplinger]

We had a meeting on Friday and discussed this a bit more, here are the results:

• Creating a reference assembly for System.Private.CoreLib.dll looks like the best/easiest option for unblocking the Android team
• We discussed making CoreLib architecture-agnostic as it would help save a lot of size for Android apps (CoreLib needs to be duplicated across four architectures right now), but it's not fully clear yet how much work this entails and it's dependent on the runtime team supporting this approach. In addition to that we'll likely run into issues with the "library trimming" mode that we use in the build, we'd need to make sure it doesn't optimize code for one architecture. It can be done independently and/or in addition to the reference assembly work.

@ViktorHofer also explained that having a CoreLib reference assembly has other benefits too:

• Makes it possible to keep public APIs between the different flavours of CoreLib in sync and using existing tooling (ApiCompat) to validate that, see System.Private.CoreLib should expose consistent surface area across all OSes #7553.
• Improves developer innerloop when libraries build against CoreLib since a change no longer triggers rebuilding libs
• Removes the current hack in our build system that allows an AnyCPU libs to reference an arch specific lib.
• Keeps the MVID not just between different architectures in sync but also between diferent OS specific implementations.

A downside is that we'll need to manually build and maintain the reference assembly since the /refout switch doesn't work here. We also don't know yet whether we have mismatches in the public API surface area between OS/arch that we need to reconcile.

/cc @jkotas @ericstj

[ViktorHofer]

runtime/eng/references.targets

Lines 32 to 39 in 6762f54

	<!-- Disable TargetArchitectureMismatch warning when we reference CoreLib as it is platform specific. -->
	<Target Name="DisableProjectReferenceArchitectureMismatchWarningForCoreLib"
	Condition="'@(_coreLibProjectReference)' != ''"
	BeforeTargets="ResolveAssemblyReferences">
	<PropertyGroup>
	<ResolveAssemblyWarnOrErrorOnTargetArchitectureMismatch>None</ResolveAssemblyWarnOrErrorOnTargetArchitectureMismatch>
	</PropertyGroup>
	</Target>

We explicitly had to disable an msbuild check to be able to reference an arch specific assembly from an AnyCPU assembly. When doing the proposed set of work (either introducing a reference assembly or making CoreLib arch agnostic), we should also eliminate this hack

[jonathanpeppers]

One issue that came up, is when using a .NET 7 SDK to build a net6.0-android app:

• You have the .NET 7 linker, and it's newer Mono.Cecil behavior
• You have the old .NET 6 System.Private.CoreLib.dll

So if there was a ref assembly for System.Private.CoreLib.dll, it wouldn't fix building net6.0 apps in .NET 7?

[vitek-karas]

@jonathanpeppers I think the answer is yes - the linker would produce different MVIDs for different archs of framework assemblies since they would be .NET 6 framework which would have internally different assemblies per arch (if nothing else, due to the CoreLib difference).

[ghost]

Tagging subscribers to this area: @directhex
See info in area-owners.md if you want to be subscribed.

Issue Details

---

We're hitting an issue where the MVID of assemblies is different depending on the architecture of the runtime pack, e.g. x64 vs. arm64, for assemblies that are 100% managed code like System.Collections.dll or System.Private.Uri.dll but compile against System.Private.CoreLib.

This causes an issue for Xamarin Android and other internal teams that use the MVID to deduplicate assemblies that are the same across architectures (right now only System.Private.CoreLib.dll is actually architecture-specific).

The reason this is showing up now is because of a linker/Cecil fix (dotnet/linker#2203, dotnet/cecil#31) which fixes the PDB ID calculation which in turn affects the MVID calculation (we actually just got lucky in net6 since we run the linker against all assemblies in the libraries build which "unified" the MVID).

Since the PDB is storing the MVID of all assembly references in "Compilation Metadata References" (described in https://github.com/dotnet/runtime/blob/424a09cb81c678fb1ba1c27211b80aba2de070ad/docs/design/specs/PortablePdb-Metadata.md#CompilationMetadataReferences) we now get a different MVID on the assemblies because these projects compile directly against System.Private.CoreLib.dll whose MVID gets embedded in the PDB:

There is interest to backport the linker/Cecil fix to net6 so I discussed with @vitek-karas and we agreed the least invasive change is to simply hardcode System.Private.CoreLib.dll as the only architecture-specific assembly in Xamarin Android for net6. EDIT it turns out this plan won't work since the Mono AOT compiler verifies the MVIDs of assembly references and will error out if they don't match with the actual assembly.

For net7/net8 we'd like to come up with a "proper" fix though, some ideas:

• Create a reference assembly for System.Private.CoreLib and compile against that one, instead of the real assembly
  • I tried using Roslyn's /refout feature but the reference assembly created by it still differs between architectures since Roslyn includes private fields like Padding.cs#L9-L16, so this'd likely need to be a real reference assembly
• Make System.Private.CoreLib architecture-agnostic by removing all of the TARGET_64BIT and TARGET_32BIT ifdefs.
• Tweak the (official) build so we only ever build the non-architecture-specific assemblies once and copy them into the respective runtime packs, ensuring that we only have a single version of the assemblies
  • As an extension we could split out the assemblies into another package, so they're not duplicated in the runtime packs. This would also help reducing download size.

/cc @ViktorHofer @vitek-karas @agocke @marek-safar @steveisok

Author:	akoeplinger
Assignees:	-
Labels:	area-Infrastructure-mono
Milestone:	7.0.0