Fix large version bubble field offset computation (#34401)

This change fixes two bugs in field offset computation where the results
that crossgen2 was getting was different from what runtime computes. In
both cases, the problem was caused by alignment of a derived class being
done differently.
The first issue was happening for the case when the base and derived
classes are in different assemblies. Runtime detect if two assemblies
are in the same version bubble using the native manifest metadata table
containing a list of assemblies that was supposed to contain all
assemblies that the assembly being compiled was found to reference.
However, it contained only assemblies that were not in the original
assembly reference list, e.g. ones pulled in by inlining. So runtime
wasn't getting the same view on what's in the bubble.
The second issue happened for the case when both the base and derived
class were from the same assembly, but one of the ancestor classes had a
field of a value class type that was from another assembly and could be
transitively decomposed to fields of types from the same assembly or types
like primitive types, object, pointer or enums. The alignment of a derived
class members is determined based on that and runtime decision is to
align if there is any type from another assembly in the type hierarchy
of a class or in fields of any ancestors.
For example, the decision would be different for the following scenario:
Assembly A:
struct AA
{
    int a;
}
Assembly B:
class B1
{
    AA aa;
}
class B2 : B1
{
    int x;
}
Here crossgen2 would not align the first member but runtime would. So the
layout of B2 produced by crossgen2 would be:
```
Offset  Field
0       MethodTable
8       a
12      x
```
Layout produced by the runtime would be
```
Offset  Field
0       MethodTable
8       a
16      x
```

The fix for the first issue is to put all referenced assemblies into the
native manifest metadata.
The fix for the second issue is to stop decomposing members of value
classes once we hit a value class that's from another module.

diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/CompilationModuleGroup.ReadyToRun.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/CompilationModuleGroup.ReadyToRun.cs
index 83fc679..acd8aa3 100644 (file)
--- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/CompilationModuleGroup.ReadyToRun.cs
+++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/CompilationModuleGroup.ReadyToRun.cs
@@ -78,6 +78,11 @@ namespace ILCompiler
         public abstract bool IsCompositeBuildMode { get; }
 
         /// <summary>
+        /// Returns true when the compiler is running in large version bubble mode
+        /// </summary>
+        public abstract bool IsInputBubble { get; }
+
+        /// <summary>
         /// List of input modules to use for the compilation.
         /// </summary>
         public abstract IEnumerable<EcmaModule> CompilationModuleSet { get; }

diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ManifestMetadataTableNode.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ManifestMetadataTableNode.cs
index d013150..b54979b 100644 (file)
--- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ManifestMetadataTableNode.cs
+++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ManifestMetadataTableNode.cs
@@ -80,12 +80,16 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
             {
                 MetadataReader mdReader = _nodeFactory.CompilationModuleGroup.CompilationModuleSet.Single().MetadataReader;
                 _assemblyRefCount = mdReader.GetTableRowCount(TableIndex.AssemblyRef) + 1;
-                for (int assemblyRefIndex = 1; assemblyRefIndex < _assemblyRefCount; assemblyRefIndex++)
+
+                if (!_nodeFactory.CompilationModuleGroup.IsInputBubble)
                 {
-                    AssemblyReferenceHandle assemblyRefHandle = MetadataTokens.AssemblyReferenceHandle(assemblyRefIndex);
-                    AssemblyReference assemblyRef = mdReader.GetAssemblyReference(assemblyRefHandle);
-                    string assemblyName = mdReader.GetString(assemblyRef.Name);
-                    _assemblyRefToModuleIdMap[assemblyName] = assemblyRefIndex;
+                    for (int assemblyRefIndex = 1; assemblyRefIndex < _assemblyRefCount; assemblyRefIndex++)
+                    {
+                        AssemblyReferenceHandle assemblyRefHandle = MetadataTokens.AssemblyReferenceHandle(assemblyRefIndex);
+                        AssemblyReference assemblyRef = mdReader.GetAssemblyReference(assemblyRefHandle);
+                        string assemblyName = mdReader.GetString(assemblyRef.Name);
+                        _assemblyRefToModuleIdMap[assemblyName] = assemblyRefIndex;
+                    }
                 }
 
                 // AssemblyRefCount + 1 corresponds to ROWID 0 in the manifest metadata

diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs
index d313f1f..031764a 100644 (file)
--- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs
+++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs
@@ -20,6 +20,7 @@ namespace ILCompiler
         private Dictionary<TypeDesc, ModuleToken> _typeRefsInCompilationModuleSet;
         private readonly bool _compileGenericDependenciesFromVersionBubbleModuleSet;
         private readonly bool _isCompositeBuildMode;
+        private readonly bool _isInputBubble;
         private readonly ConcurrentDictionary<TypeDesc, bool> _containsTypeLayoutCache = new ConcurrentDictionary<TypeDesc, bool>();
         private readonly ConcurrentDictionary<TypeDesc, bool> _versionsWithTypeCache = new ConcurrentDictionary<TypeDesc, bool>();
         private readonly ConcurrentDictionary<MethodDesc, bool> _versionsWithMethodCache = new ConcurrentDictionary<MethodDesc, bool>();
@@ -27,12 +28,14 @@ namespace ILCompiler
         public ReadyToRunCompilationModuleGroupBase(
             TypeSystemContext context,
             bool isCompositeBuildMode,
+            bool isInputBubble,
             IEnumerable<EcmaModule> compilationModuleSet,
             IEnumerable<ModuleDesc> versionBubbleModuleSet,
             bool compileGenericDependenciesFromVersionBubbleModuleSet)
         {
             _compilationModuleSet = new HashSet<EcmaModule>(compilationModuleSet);
             _isCompositeBuildMode = isCompositeBuildMode;
+            _isInputBubble = isInputBubble;
 
             Debug.Assert(_isCompositeBuildMode || _compilationModuleSet.Count == 1);
 
@@ -88,18 +91,7 @@ namespace ILCompiler
             var defType = (MetadataType)type;
             if (!ContainsType(defType))
             {
-                if (!defType.IsValueType)
-                {
-                    // Eventually, we may respect the non-versionable attribute for reference types too. For now, we are going
-                    // to play is safe and ignore it.
-                    return false;
-                }
-
-                // Valuetypes with non-versionable attribute are candidates for fixed layout. Reject the rest.
-                if (!defType.HasCustomAttribute("System.Runtime.Versioning", "NonVersionableAttribute"))
-                {
-                    return false;
-                }
+                return false;
             }
             if (!defType.IsValueType && !ContainsTypeLayout(defType.BaseType))
             {
@@ -220,6 +212,8 @@ namespace ILCompiler
 
         public sealed override bool IsCompositeBuildMode => _isCompositeBuildMode;
 
+        public sealed override bool IsInputBubble => _isInputBubble;
+
         public sealed override IEnumerable<EcmaModule> CompilationModuleSet => _compilationModuleSet;
 
         private bool ComputeTypeVersionsWithCode(TypeDesc type)

diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunSingleAssemblyCompilationModuleGroup.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunSingleAssemblyCompilationModuleGroup.cs
index f4e85d1..01a6c1a 100644 (file)
--- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunSingleAssemblyCompilationModuleGroup.cs
+++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunSingleAssemblyCompilationModuleGroup.cs
@@ -20,11 +20,13 @@ namespace ILCompiler
         public ReadyToRunSingleAssemblyCompilationModuleGroup(
             TypeSystemContext context,
             bool isCompositeBuildMode,
+            bool isInputBubble,
             IEnumerable<EcmaModule> compilationModuleSet,
             IEnumerable<ModuleDesc> versionBubbleModuleSet,
             bool compileGenericDependenciesFromVersionBubbleModuleSet) :
                 base(context,
                      isCompositeBuildMode,
+                     isInputBubble,
                      compilationModuleSet,
                      versionBubbleModuleSet,
                      compileGenericDependenciesFromVersionBubbleModuleSet)

diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/SingleMethodCompilationModuleGroup.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/SingleMethodCompilationModuleGroup.cs
index 11c0b2e..f32c229 100644 (file)
--- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/SingleMethodCompilationModuleGroup.cs
+++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/SingleMethodCompilationModuleGroup.cs
@@ -20,12 +20,14 @@ namespace ILCompiler
         public SingleMethodCompilationModuleGroup(
             TypeSystemContext context,
             bool isCompositeBuildMode,
+            bool isInputBubble,
             IEnumerable<EcmaModule> compilationModuleSet,
             IEnumerable<ModuleDesc> versionBubbleModuleSet,
             bool compileGenericDependenciesFromVersionBubbleModuleSet,
             MethodDesc method) :
                 base(context,
                      isCompositeBuildMode,
+                     isInputBubble,
                      compilationModuleSet,
                      versionBubbleModuleSet,
                      compileGenericDependenciesFromVersionBubbleModuleSet)

diff --git a/src/coreclr/src/tools/crossgen2/crossgen2/Program.cs b/src/coreclr/src/tools/crossgen2/crossgen2/Program.cs
index 6f98ee6..ecb8954 100644 (file)
--- a/src/coreclr/src/tools/crossgen2/crossgen2/Program.cs
+++ b/src/coreclr/src/tools/crossgen2/crossgen2/Program.cs
@@ -295,6 +295,7 @@ namespace ILCompiler
                         compilationGroup = new SingleMethodCompilationModuleGroup(
                             typeSystemContext,
                             _commandLineOptions.Composite,
+                            _commandLineOptions.InputBubble,
                             inputModules,
                             versionBubbleModules,
                             _commandLineOptions.CompileBubbleGenerics,
@@ -307,6 +308,7 @@ namespace ILCompiler
                         compilationGroup = new ReadyToRunSingleAssemblyCompilationModuleGroup(
                             typeSystemContext,
                             _commandLineOptions.Composite,
+                            _commandLineOptions.InputBubble,
                             inputModules,
                             versionBubbleModules,
                             _commandLineOptions.CompileBubbleGenerics);