-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
aspnetcore tests having Dynamic.Proxy fails on ppc64le architecture #93770
Comments
Given the stack trace, I think you should follow-up with the owner of (https://www.nuget.org/packages/Castle.DynamicProxy). Given the context, this is possibly an Endianness issue. |
closing, please follow-up with Castle to take a look |
I don't think this is an endianness issue. pple64 is little-endian, like x64. I ran the aspnetcore tests on x64 with a mono runtime, and they show the same issue. |
@tmds @jeffschwMSFT these tests were passing till .NET8 Preview6. From Preview7 we are observing these failures. So something changed in runtime between preview6 and preview7 which is causing this issue. |
@lambdageek @akoeplinger may be you have some thoughts on what changes went in since preview 6 that might cause cause this to start failing? |
@tmds Maybe #87406 /cc @ivanpovazan (I'm looking through v8.0.0-preview.6.23329.7...v8.0.0-preview.7.23375.6 ) @tmds you said you can repro the failure on x64? Could you share the repro steps |
I can take a look. Repro steps would be much appreciated. |
We had verified 87406 patch on prview6 but at that time did not observe these test failures. So i think this is not issue. Still again to reconfirm we reverted this commit on rc2 and tried but still observed failures. |
We have built aspnetcore rc2 tag code using .NET8 rc2 SDK with mono and ran aspnetcore tests then observed these failures.
|
If we update CastleCore version from 4.2.1 to 5.1.1 In aspnetcore/eng/Versions.props file then test failures observed in Microsoft.AspNetCore.Server.Kestrel.Core.Tests gets fixed. |
@lambdageek @ivanpovazan
cc: @tmds |
Here's a standalone repro https://github.com/lambdageek/gh-93770-efcorerepro Observations:
|
Yea, rc2 mono with preview6 efcore - works. rc2 mono with preview7 efcore - exception. I'm not sure precisely what changed over there, but there were some interesting changes between previews 6 and 7, like dotnet/efcore#31003 for example. So this is some behavior difference between Mono and CoreCLR that efcore stubmled on. Unfortunately debugging isn't super helpful yet. |
This is indeed the change that introduces the regression:
|
Many of the test failures look like:
The IEnumerable that is => property.GetContainingForeignKeys().All(fk => fk.IsBaseLinking()) It should never be public virtual IEnumerable<ForeignKey> GetContainingForeignKeys()
=> ForeignKeys ?? Enumerable.Empty<ForeignKey>(); So, what method is getting called here ... 🤔 To call the method, there is some casting from one interface to another (as can be seen in the stacktrace above). public class Property : PropertyBase, IMutableProperty, IConventionProperty, IProperty
{
...
public virtual IEnumerable<ForeignKey> GetContainingForeignKeys()
=> ForeignKeys ?? Enumerable.Empty<ForeignKey>();
IEnumerable<IReadOnlyForeignKey> IReadOnlyProperty.GetContainingForeignKeys()
=> GetContainingForeignKeys();
IEnumerable<IMutableForeignKey> IMutableProperty.GetContainingForeignKeys()
=> GetContainingForeignKeys();
IEnumerable<IConventionForeignKey> IConventionProperty.GetContainingForeignKeys()
=> GetContainingForeignKeys();
IEnumerable<IForeignKey> IProperty.GetContainingForeignKeys()
=> GetContainingForeignKeys(); |
Thanks @tmds. I'm going to try two things:
|
Hi I have build dotnet\efcore on power and run their tests. Below is the stacktrace for one of the test run.
|
vtable for |
So on a whim I tried the repro from #93770 (comment) with the interpreter, and it worked % MONO_ENV_OPTIONS=--interp dotnet run
starting
created the database from the model So it's some kind of JIT bug :-( Also, if I'm in the debugger (using the JIT) and I have |
This is kind of suspicious:
The cctor for So this is either soem kind of a cctor ddeadlock (but I don't think there are other threads running). or it's another instance of #77513 or #88066 |
I did some more debugging. Something weird goes on when this cast is made*: protected virtual ValueGenerated? GetValueGenerated(IConventionProperty property)
=> GetValueGenerated((IReadOnlyProperty)property); When I expand it to: protected virtual ValueGenerated? GetValueGenerated(IConventionProperty property)
{
_ = property.GetContainingForeignKeys() ?? throw new Exception("ex1");
IReadOnlyProperty roProp = (IReadOnlyProperty)property;
_ = roProp.GetContainingForeignKeys() ?? throw new Exception("ex2");
return GetValueGenerated(roProp);
} The second exception gets thrown. (*: it may be not the cast, but something else that messes up the vtable for the interface.) |
There is something strange going on with the vtable for I also tried adding a call to So there is an off by one somewhere. I guess I'll look at the vtable setup trace in more detail again. |
I got a little bit further with this yesterday, but had to context switch to something else. The problem is somewhere between mono's IMT (interface method table) builder and the generated IMT trampoline: The virtcall to What I've observed (via some printfs and some lldb debugging) is that the dispatcher ends up with the offset of I ran out of time yesterday to see if:
Update incidentally this is why trying to recreate this scenario with a smaller sample mimicing the EFCore class hierarchy is challenging - the IMT slots are populated based on hashing of namespaces/class names/method names/return and arguent type names. Creating the same exact hash collisions is hard. |
It's a problem in the IMT builder. When runtime/src/mono/mono/metadata/object.c Lines 1599 to 1600 in 69702c3
these two lines need to be under an PR shortly. I want to think a little bit harder whether we can make a test case - I'm not sure if we really depend on an IMT slot collision to exhibit the problem or if it will show up if I can make it show up even with a single method per IMT slot |
…94437) * [mono] [imt] Don't increment vt_slot for non-virtual generic methods Interfaces can have static generic methods, for example. They don't have a vt_slot. When building an IMT slot, we need to collect all the interface methods that map to a particular IMT slot along with their implementing vtable entries. To do that, vt_slot starts at the interface offset of a particular interface and keeps incrementing as we iterate over the methods of the interface. It is crtitical that vt_slot is accurate - otherwise we may dispatch the interface call to the wrong virtual method. Fixes #93770 * [mono][imt] remove dead appdomain code the extra_interfaces argument was used to implement additional interfaces on cross-domain transparent proxy objects. * [mono][imt] fixup ifdefed debug code * Add test case
…rtual generic methods (#94469) * [mono] [imt] Don't increment vt_slot for non-virtual generic methods Interfaces can have static generic methods, for example. They don't have a vt_slot. When building an IMT slot, we need to collect all the interface methods that map to a particular IMT slot along with their implementing vtable entries. To do that, vt_slot starts at the interface offset of a particular interface and keeps incrementing as we iterate over the methods of the interface. It is crtitical that vt_slot is accurate - otherwise we may dispatch the interface call to the wrong virtual method. Fixes #93770 * [mono][imt] remove dead appdomain code the extra_interfaces argument was used to implement additional interfaces on cross-domain transparent proxy objects. * [mono][imt] fixup ifdefed debug code * Add test case --------- Co-authored-by: Aleksey Kliger <[email protected]>
…rtual generic methods (#94478) Backport of #94437 to release/6.0-staging Fixes #93770 * [mono] [imt] Don't increment vt_slot for non-virtual generic methods Interfaces can have static generic methods, for example. They don't have a vt_slot. When building an IMT slot, we need to collect all the interface methods that map to a particular IMT slot along with their implementing vtable entries. To do that, vt_slot starts at the interface offset of a particular interface and keeps incrementing as we iterate over the methods of the interface. It is crtitical that vt_slot is accurate - otherwise we may dispatch the interface call to the wrong virtual method. * [mono][imt] remove dead appdomain code the extra_interfaces argument was used to implement additional interfaces on cross-domain transparent proxy objects. * [mono][imt] fixup ifdefed debug code * Add test case
…rtual generic methods (#94468) Backport of #94437 to release/7.0-staging Fixes #93770 * [mono] [imt] Don't increment vt_slot for non-virtual generic methods Interfaces can have static generic methods, for example. They don't have a vt_slot. When building an IMT slot, we need to collect all the interface methods that map to a particular IMT slot along with their implementing vtable entries. To do that, vt_slot starts at the interface offset of a particular interface and keeps incrementing as we iterate over the methods of the interface. It is crtitical that vt_slot is accurate - otherwise we may dispatch the interface call to the wrong virtual method. * [mono][imt] remove dead appdomain code the extra_interfaces argument was used to implement additional interfaces on cross-domain transparent proxy objects. * [mono][imt] fixup ifdefed debug code * Add test case --------- Co-authored-by: Aleksey Kliger <[email protected]>
The backports for .NET 6, 7, 8 have been merged. |
Description
Around 6000+ aspnetcore tests which are using Dynamic.Proxy are failing on ppc64le with mono complier.
Reproduction Steps
build aspnetcore on ppc64le using cross built net8 rc2 and run tests.
Expected behavior
All tests should pass
Actual behavior
We get below error for 6000+ tests
Regression?
No response
Known Workarounds
No response
Configuration
.NET 8 RC1 cross built for the linux-ppc64 target (using the Mono runtime by default).
Other information
No response
cc: @janani66 @tmds @uweigand
The text was updated successfully, but these errors were encountered: