Skip to content
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.

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

Fix large version bubble field offset computation #34401

Merged
merged 1 commit into from
Apr 2, 2020

Conversation

janvorli
Copy link
Member

@janvorli janvorli commented Apr 1, 2020

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 of B2 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.

@janvorli janvorli added this to the 5.0 milestone Apr 1, 2020
@janvorli janvorli requested a review from trylek April 1, 2020 15:27
@janvorli janvorli self-assigned this Apr 1, 2020
@janvorli
Copy link
Member Author

janvorli commented Apr 1, 2020

This change allows all the remaining 500-600 previously failing tests in the System.Linq.Parallel.Test.dll to pass.

Copy link
Member

@trylek trylek left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for fixing this!

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.
@janvorli janvorli force-pushed the fix-cg2-field-offset-computation branch from 8b49b60 to 8eac6df Compare April 1, 2020 19:58
@janvorli janvorli merged commit c8b9cf1 into dotnet:master Apr 2, 2020
@janvorli janvorli deleted the fix-cg2-field-offset-computation branch April 2, 2020 13:11
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants