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

[loader] Rework get_method_constrained (Fixes #60545) #6037

Merged
merged 2 commits into from Nov 20, 2017

Conversation

@lambdageek
Copy link
Member

commented Nov 17, 2017

Instead of using find_method which relies on method name and exact signature
matching (and thus can't work for variant interfaces), use the vtable and the
interface offset to find the constrained method.

Terminology:

  • base method - a method of the base class to be called
  • base class - some general class or interface that declares a method
  • constraint class - a more specific class that also implements the method.
  • constrained method - the version of the base method in the constraint class.

To find the constrained method given the base method:

  • if the base class is a class:
    the constraint class can't be an interface. it's either the base class or
    some subclass of it. If the base method isn't virtual there's nothing to do
    the constrained method is the same one.
    If the base method is virtual, get its vtable slot and find the corresponding
    method in the constraint class's vtable.
  • if the base class is an interface:
    in this case the constraint class is either another interface or a class.
    if the constraint class is an interface, since interfaces can't have
    implementations, there's nothing to do.
    if the constraint class is a class, then to find the constrained method
    get the interface slot offset of the base method and find the base interface offset
    of the base interface and look in the constrained class vtable.
    (the base interface offset gives the starting vtable slot for the base
    interface in the constraint class vtable; the slot offsets go consecutively
    for each of the methods of the interface).

Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=60545

@lambdageek

This comment has been minimized.

Copy link
Member Author

commented Nov 17, 2017

Broke Mono.CSharp.ExpressionTest.AwaitExpression, investigating...

@kumpera
Copy link
Member

left a comment

Looks good, that's what we discussed, not to figure out the crash :/

@lambdageek

This comment has been minimized.

Copy link
Member Author

commented Nov 17, 2017

Apparently mono_class_setup_interface_offsets(constrained_klass); mono_class_setup_vtable(constrained_klass) causes the failure below, but just mono_class_setup_vtable(constrained_klass) is okay:

System.TypeLoadException : Type <InteractiveExpressionClass>+<Host>c__async0 has invalid vtable method slot 4 with method none

Feels like a bug in mono_class_setup_vtable, but unless I find it right away and it's obvious, i'm going to just fixup this PR to call mono_class_setup_vtable(constrained_class)
Update: it's because mono_class_setup_interface_offsets is basically always the wrong thing to call. It assumes the parent of the current class assigned zero vtable slots and so it will compute incorrect offsets for the interfaces.

@lambdageek

This comment has been minimized.

Copy link
Member Author

commented Nov 20, 2017

@monojenkins build

@lambdageek

This comment has been minimized.

Copy link
Member Author

commented Nov 20, 2017

Hm. FullAOT failure is relevant:

* Assertion at loader.c:1826, condition `mono_class_is_assignable_from (base_class, constrained_class)' not met

Investigating.

lambdageek added 2 commits Nov 16, 2017
[tests] Mono test for contravariant constrained.callvirt
Test that constrained.callvirt IL instruction sequence works for contravariant
interfaces.

Regression test for https://bugzilla.xamarin.com/show_bug.cgi?id=60545
[loader] Rework get_method_constrained (Fixes #60545)
Instead of using find_method which relies on method name and exact signature
matching (and thus can't work for variant interfaces), use the vtable and the
interface offset to find the constrained method.

Terminology:
  base method - a method of the base class to be called
  base class - some general class or interface that declares a method
  constraint class - a more specific class that also implements the method.
  constrained method - the version of the base method in the constraint class.

To find the constrained method given the base method:

- if the base class is a class:
  the constraint class can't be an interface. it's either the base class or
  some subclass of it.  If the base method isn't virtual there's nothing to do
  the constrained method is the same one.
  If the base method is virtual, get its vtable slot and find the corresponding
  method in the constraint class's vtable.
- if the base class is an interface:
  in this case the constraint class is either another interface or a class.
  if the constraint class is an interface, since interfaces can't have
  implementations, there's nothing to do.
  if the constraint class is a class, then to find the constrained method
  get the interface slot offset of the base method and find the base interface offset
  of the base interface and look in the constrained class vtable.
  (the base interface offset gives the starting vtable slot for the base
  interface in the constraint class vtable; the slot offsets go consecutively
  for each of the methods of the interface).

Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=60545

@lambdageek lambdageek force-pushed the lambdageek:bug-60545 branch from 1ecb065 to 8c7d25e Nov 20, 2017

@lambdageek lambdageek merged commit 3259e04 into mono:master Nov 20, 2017

14 of 17 checks passed

Linux AArch64 Interpreter Build finished. 11128 tests run, 94 skipped, 0 failed.
Details
Linux ARMv5 soft float Build finished. 51580 tests run, 1361 skipped, 1 failed.
Details
Linux x64 Interpreter Build finished. 11130 tests run, 94 skipped, 1 failed.
Details
API Diff No public API changes found.
Details
Linux AArch64 Build finished. 63912 tests run, 1433 skipped, 0 failed.
Details
Linux ARMv7 hard float Build finished. 63876 tests run, 1428 skipped, 0 failed.
Details
Linux ARMv7 hard float Interpreter Build finished. 11121 tests run, 94 skipped, 0 failed.
Details
Linux i386 Build finished. 63910 tests run, 1425 skipped, 0 failed.
Details
Linux x64 Build finished. 63910 tests run, 1427 skipped, 0 failed.
Details
Linux x64 FullAOT Build finished. 21704 tests run, 549 skipped, 0 failed.
Details
Linux x64 mcs Build finished.
Details
OS X i386 Build finished. 63072 tests run, 1309 skipped, 0 failed.
Details
OS X x64 Build finished. 63072 tests run, 1311 skipped, 0 failed.
Details
Test Result Viewer Click to view aggregated test results (Xamarin internal).
Details
Windows i386 Build finished. 59067 tests run, 1138 skipped, 0 failed.
Details
Windows x64 Build finished. 59089 tests run, 1140 skipped, 0 failed.
Details
license/cla All CLA requirements met.
Details

@lambdageek lambdageek deleted the lambdageek:bug-60545 branch Feb 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.