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

[mono] crash in class-setup-vtable with re-abstracted covariant returns #76312

Closed
lambdageek opened this issue Sep 28, 2022 · 2 comments
Closed
Assignees
Milestone

Comments

@lambdageek
Copy link
Member

Repro:

namespace ReproMAUI6811;

public static class Program
{
    public static void Main()
    {
        new Leaf();
    }
}

public abstract class Base {
    public abstract I getI();
}

public class PseudoBase : Base {
    public override I getI() => null;
}

public abstract class Itermediate : PseudoBase {
    public override abstract I getI();
}

public class Leaf : Itermediate {
    public Leaf() {}
    public override C getI() { return null; }
}

public interface I {}

public class C : I {}

Build with

$ dotntet run -p:UseMonoRuntime=true --self-contained -r <YOUR_RID>

Expected result:

(no output)

Actual result:

Crash

	0x100d657c0 - /Users/alklig/work/bugs/android-6811/q/bin/Debug/net7.0/osx-arm64/libcoreclr.dylib : mono_class_setup_vtable_general
	0x100d657c0 - /Users/alklig/work/bugs/android-6811/q/bin/Debug/net7.0/osx-arm64/libcoreclr.dylib : mono_class_setup_vtable_general
	0x100d64588 - /Users/alklig/work/bugs/android-6811/q/bin/Debug/net7.0/osx-arm64/libcoreclr.dylib : mono_class_setup_vtable_full
	0x100db29c0 - /Users/alklig/work/bugs/android-6811/q/bin/Debug/net7.0/osx-arm64/libcoreclr.dylib : mono_class_vtable_checked
	0x100ea6948 - /Users/alklig/work/bugs/android-6811/q/bin/Debug/net7.0/osx-arm64/libcoreclr.dylib : mono_method_to_ir
	0x100e743e8 - /Users/alklig/work/bugs/android-6811/q/bin/Debug/net7.0/osx-arm64/libcoreclr.dylib : mini_method_compile
	0x100e76d28 - /Users/alklig/work/bugs/android-6811/q/bin/Debug/net7.0/osx-arm64/libcoreclr.dylib : mono_jit_compile_method_inner
	0x100e7b2e8 - /Users/alklig/work/bugs/android-6811/q/bin/Debug/net7.0/osx-arm64/libcoreclr.dylib : jit_compile_method_with_opt
	0x100e7edc8 - /Users/alklig/work/bugs/android-6811/q/bin/Debug/net7.0/osx-arm64/libcoreclr.dylib : mono_jit_runtime_invoke
	0x100db1cac - /Users/alklig/work/bugs/android-6811/q/bin/Debug/net7.0/osx-arm64/libcoreclr.dylib : mono_runtime_invoke_checked
	0x100db994c - /Users/alklig/work/bugs/android-6811/q/bin/Debug/net7.0/osx-arm64/libcoreclr.dylib : do_exec_main_checked
	0x100ed2238 - /Users/alklig/work/bugs/android-6811/q/bin/Debug/net7.0/osx-arm64/libcoreclr.dylib : mono_jit_exec
	0x100ed4cd4 - /Users/alklig/work/bugs/android-6811/q/bin/Debug/net7.0/osx-arm64/libcoreclr.dylib : mono_main

Foudn in dotnet/maui#6811

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Sep 28, 2022
@lambdageek
Copy link
Member Author

A better stack trace

#0  mono_method_signature_internal (m=0x0) at /__w/1/s/src/mono/mono/mini/../../mono/metadata/class-internals.h:794
#1  signature_is_subsumed (impl_method=impl_method@entry=0x2306b48, decl_method=decl_method@entry=0x0, error=error@entry=0x7ffe852539c0) at /__w/1/s/src/mono/mono/metadata/class-setup-vtable.c:1508
#2  0x00000000004c9a6d in check_signature_covariant (decl=0x0, impl=0x2306b48, klass=0x23069f8) at /__w/1/s/src/mono/mono/metadata/class-setup-vtable.c:1592
#3  check_vtable_covariant_override_impls (vtable_size=86, vtable=0x2cf7d50, klass=0x23069f8) at /__w/1/s/src/mono/mono/metadata/class-setup-vtable.c:1672
#4  mono_class_setup_vtable_general (klass=klass@entry=0x23069f8, overrides=<optimized out>, onum=<optimized out>, in_setup=in_setup@entry=0x2cc67e0) at /__w/1/s/src/mono/mono/metadata/class-setup-vtable.c:2121
#5  0x00000000004c9c6c in mono_class_setup_vtable_full (klass=klass@entry=0x23069f8, in_setup=0x2cc67e0, in_setup@entry=0x0) at /__w/1/s/src/mono/mono/metadata/class-setup-vtable.c:960
#6  0x00000000004c9e47 in mono_class_setup_vtable (klass=klass@entry=0x23069f8) at /__w/1/s/src/mono/mono/metadata/class-setup-vtable.c:888
#7  0x000000000043dcb7 in emit_klass_info (token=33555340, acfg=0x23069f8) at /__w/1/s/src/mono/mono/mini/aot-compiler.c:7484
#8  emit_class_info (acfg=0x23069f8) at /__w/1/s/src/mono/mono/mini/aot-compiler.c:11154
#9  emit_aot_image (acfg=acfg@entry=0x1db9010) at /__w/1/s/src/mono/mono/mini/aot-compiler.c:14871
#10 0x0000000000443cd9 in mono_compile_assembly (ass=ass@entry=0x1db88e0, opts=opts@entry=374417919, 
    aot_options=aot_options@entry=0x1d4ab60 "asmwriter,temp-path=obj/Release/net7.0-android/android-x86/aot/x86/SaxonCS,profile-only,profile=~/xamarin-android/bin/Release/dotnet/packs/Microsoft.Maui.Sdk/7.0.0-rc.2.6736/Sdk"..., 
    global_aot_state=global_aot_state@entry=0x7ffe85253f50) at /__w/1/s/src/mono/mono/mini/aot-compiler.c:14642
#11 0x00000000004290c3 in main_thread_handler (user_data=<synthetic pointer>) at /__w/1/s/src/mono/mono/mini/driver.c:1415
#12 mono_main (argc=5, argv=0x7ffe85254108) at /__w/1/s/src/mono/mono/mini/driver.c:2647
#13 0x000000000041777a in mono_main_with_options (argv=0x7ffe85254108, argc=5) at /__w/1/s/src/mono/mono/mini/main.c:36
#14 main (argc=<optimized out>, argv=<optimized out>) at /__w/1/s/src/mono/mono/mini/main.c:88

@lambdageek lambdageek added area-VM-meta-mono and removed untriaged New issue has not been triaged by the area owner labels Sep 28, 2022
@lambdageek lambdageek self-assigned this Sep 28, 2022
@lambdageek lambdageek added this to the 7.0.0 milestone Sep 28, 2022
lambdageek added a commit to lambdageek/runtime that referenced this issue Sep 28, 2022
When there are covariant return overrides, we check the vtable slot in every
parent class for signature compatability with the proposed override.

However if one of the ancestor classes is abstract, it could have an
abstract override for hte method:

    class Base {
      public virtual Base Method() => this;
    }
    public abstract Intermediate : Base {
      public override abstract Base Method();
    }
    public Leaf : Intermediate {
      public override Leaf Method() => this;
    }

In this case when we're checking that Leaf.Method is compatible with
the vtable slot in Intermediate, we will see a null method (since
Intermediate is abstract).  In such cases we can skip over the class
and continue with its parent.

Fixes
dotnet#76312
github-actions bot pushed a commit that referenced this issue Sep 28, 2022
When there are covariant return overrides, we check the vtable slot in every
parent class for signature compatability with the proposed override.

However if one of the ancestor classes is abstract, it could have an
abstract override for hte method:

    class Base {
      public virtual Base Method() => this;
    }
    public abstract Intermediate : Base {
      public override abstract Base Method();
    }
    public Leaf : Intermediate {
      public override Leaf Method() => this;
    }

In this case when we're checking that Leaf.Method is compatible with
the vtable slot in Intermediate, we will see a null method (since
Intermediate is abstract).  In such cases we can skip over the class
and continue with its parent.

Fixes
#76312
github-actions bot pushed a commit that referenced this issue Sep 28, 2022
When there are covariant return overrides, we check the vtable slot in every
parent class for signature compatability with the proposed override.

However if one of the ancestor classes is abstract, it could have an
abstract override for hte method:

    class Base {
      public virtual Base Method() => this;
    }
    public abstract Intermediate : Base {
      public override abstract Base Method();
    }
    public Leaf : Intermediate {
      public override Leaf Method() => this;
    }

In this case when we're checking that Leaf.Method is compatible with
the vtable slot in Intermediate, we will see a null method (since
Intermediate is abstract).  In such cases we can skip over the class
and continue with its parent.

Fixes
#76312
carlossanlop pushed a commit that referenced this issue Sep 29, 2022
…iant return overrides (#76324)

* Add test for covariant reabstraction

* [metadata] Skip null vtable entires when checking covariant overrides

When there are covariant return overrides, we check the vtable slot in every
parent class for signature compatability with the proposed override.

However if one of the ancestor classes is abstract, it could have an
abstract override for hte method:

    class Base {
      public virtual Base Method() => this;
    }
    public abstract Intermediate : Base {
      public override abstract Base Method();
    }
    public Leaf : Intermediate {
      public override Leaf Method() => this;
    }

In this case when we're checking that Leaf.Method is compatible with
the vtable slot in Intermediate, we will see a null method (since
Intermediate is abstract).  In such cases we can skip over the class
and continue with its parent.

Fixes
#76312

* Fix copypaste

* fix whitespace

Co-authored-by: Aleksey Kliger <alklig@microsoft.com>
carlossanlop pushed a commit that referenced this issue Oct 5, 2022
…iant return overrides (#76327)

* Add test for covariant reabstraction

* [metadata] Skip null vtable entires when checking covariant overrides

When there are covariant return overrides, we check the vtable slot in every
parent class for signature compatability with the proposed override.

However if one of the ancestor classes is abstract, it could have an
abstract override for hte method:

    class Base {
      public virtual Base Method() => this;
    }
    public abstract Intermediate : Base {
      public override abstract Base Method();
    }
    public Leaf : Intermediate {
      public override Leaf Method() => this;
    }

In this case when we're checking that Leaf.Method is compatible with
the vtable slot in Intermediate, we will see a null method (since
Intermediate is abstract).  In such cases we can skip over the class
and continue with its parent.

Fixes
#76312

* Fix copypaste

* fix whitespace

Co-authored-by: Aleksey Kliger <alklig@microsoft.com>
@ghost ghost locked as resolved and limited conversation to collaborators Oct 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant