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

@vitek-karas

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

Bump to dotnet/installer@c7afae69 7.0.100-preview.4.22174.1 (#6598)

@ViktorHofer

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

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

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?

@jonathanpeppers

@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).

Add reference assembly for System.Private.CoreLib.dll (#72143)

Navigation Menu

Repository navigation

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

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	<!-- 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>

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

Description

Activity

akoeplinger commented on May 9, 2022

ViktorHofer commented on May 10, 2022

jonathanpeppers commented on Jun 28, 2022

vitek-karas commented on Jun 28, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions