dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.57k stars 4.55k forks source link

Runtime_90219 fails on Mono #90374

Open jakobbotsch opened 11 months ago

jakobbotsch commented 11 months ago

The test being added in #90318 fails on Mono in full AOT mode:

22:14:29.962 Running test: JIT/Regression/JitBlue/Runtime_90219/Runtime_90219/Runtime_90219.dll
System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation.
 ---> System.NullReferenceException: Object reference not set to an instance of an object
   at Runtime_90219.MainInner(IRuntime rt)
   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
   at System.Reflection.MethodBaseInvoker.InvokeDirectByRefWithFewArgs(Object obj, Span`1 copyOfArgs, BindingFlags invokeAttr)
   --- End of inner exception stack trace ---
   at System.Reflection.MethodBaseInvoker.InvokeDirectByRefWithFewArgs(Object obj, Span`1 copyOfArgs, BindingFlags invokeAttr)
   at System.Reflection.MethodBaseInvoker.InvokeWithOneArg(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at System.Reflection.MethodBase.Invoke(Object obj, Object[] parameters)
   at Runtime_90219.TestEntryPoint()
   at Program.<<Main>$>g__TestExecutor335|0_336(StreamWriter tempLogSw, StreamWriter statsCsvSw, <>c__DisplayClass0_0& )

https://dev.azure.com/dnceng-public/public/_build/results?buildId=370197&view=ms.vss-test-web.build-test-results-tab&runId=7857242&paneView=debug

ivanpovazan commented 10 months ago

By analysing the provided CI build and reproducing locally this seems to only fail in full AOT mode.

The tests passes on: osx-x64 Release AllSubsets_Mono_Minijit_RuntimeTests minijit https://helixre107v0xdeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-90318-merge-cedc27ee86b44aab89/Regression_2/1/Regression_2.testResults.xml.txt?helixlogtype=result

<test name="JIT/Regression/JitBlue/Runtime_90219/Runtime_90219/Runtime_90219.dll" type="Runtime_90219" method="TestEntryPoint" time="0.004416" result="Pass"></test>
ivanpovazan commented 10 months ago

The crash seems to be related to using custom AssemblyLoadContext in fullAOT mode with Mono, which doesn't seem to initialize static members of the reflected type properly. Using the default ALC AssemblyLoadContext.Default works properly.

A smaller repro:

using System;
using System.Reflection;
using System.Runtime.Loader;

namespace HelloWorld
{
    public class CustomAssemblyLoadContext : AssemblyLoadContext
    {
        public CustomAssemblyLoadContext(): base(true)
        {
        }
    }
    internal class Program
    {

        private static void Main(string[] args)
        {
            // This does not crash when we use AssemblyLoadContext.Default
            var customAlc = new CustomAssemblyLoadContext();
            Test(customAlc);
        }

        public static void Test(AssemblyLoadContext alc)
        {
            var asm = alc.LoadFromAssemblyPath(System.Reflection.Assembly.GetExecutingAssembly().Location);
            var mi = asm.GetType(typeof(Program).FullName).GetMethod(nameof(MyMethod));
            var pass = (bool)mi.Invoke(null, null);
            if (pass)
                Console.WriteLine("pass");
            else
                Console.WriteLine("fail");
        }

        public static byte TheByte = 42;
        public static bool MyMethod()
        {
            var aByte = TheByte;
            Console.WriteLine($"aByte: {aByte}");
            return (int) aByte == 42;
        }
    }
}

Running the above in full AOT mode prints:

aByte: 0
fail
ivanpovazan commented 10 months ago

@vargaz do we support loading AOT images for assemblies from non-default ALC?

lambdageek commented 10 months ago

It's a collectible ALC. So it looks like we reall dont' support it.

https://github.com/dotnet/runtime/blob/5ef2a9bf5cb29a5244929a357180f45a20dfbefd/src/mono/mono/mini/aot-runtime.c#L1965-L1970

I think that Invoke should throw something instead of allowing the method to execute somehow

Update but on the other hand, we apparently turn all ALCs into non-collectible ALCs??

https://github.com/dotnet/runtime/blob/5ef2a9bf5cb29a5244929a357180f45a20dfbefd/src/mono/mono/metadata/assembly-load-context.c#L43

lambdageek commented 10 months ago

I think load_image in aot-runtime.c shouldn't use mono_alc_get_ambient() which always just returns mono_alc_get_default(). It should use the ALC of the AOT image's module.

because in decode_klass_ref we do this:

 case MONO_AOT_TYPEREF_TYPEDEF_INDEX:
        idx = decode_value (p, &p);
        image = load_image (module, 0, error);
        if (!image)
            return NULL;
        klass = mono_class_get_checked (image, MONO_TOKEN_TYPE_DEF + idx, error);
        break;
    case MONO_AOT_TYPEREF_TYPEDEF_INDEX_IMAGE:
        idx = decode_value (p, &p);
        image = load_image (module, decode_value (p, &p), error);
        if (!image)
            return NULL;
        klass = mono_class_get_checked (image, MONO_TOKEN_TYPE_DEF + idx, error);
        break;

so we always end up with a reference to the MonoClass* from the default ALC. So I bet when we go to initialize static fields, we share the runtime vtable of the default ALC