Invoking delegate with null argument not working as expected.

[bholmes]

Description

Invoke a delegate pointing to an instance method with a null ref causes a crash with the Mono runtime and raises a question for both CoreCLR and Mono runtimes. What should happen?

Reproduction Steps

Compile and run the following...

using System;
using System.Reflection;

namespace HelloWorld
{
    internal class Program
    {
        private static void Main(string[] args)
        {
            MethodInfo m = typeof(TestClass<string>).GetMethod("Test", BindingFlags.Public | BindingFlags.Instance);
            Action<TestClass<string>> del = (Action<TestClass<string>>)m.CreateDelegate(typeof(Action<TestClass<string>>), null);
            del.Invoke(null);
        }

        public class TestClass<T>
        {
            private int x = 10;
            
            public void Test() {
                Console.WriteLine(Environment.StackTrace);
                Console.WriteLine(x);
            }
        }
    }
}

Expected behavior

The program should throw an argument or null reference exception within the Invoke call? Honestly I am not 100% sure what should happen.

Actual behavior

With CoreClLR the program does run and an exception is thrown. It appears that the instance method is called (with a null this) and works until an member is accessed. (Should the program even enter the method?)

   at System.Environment.get_StackTrace()
   at HelloWorld.Program.TestClass`1.Test() in /home/bill/dev/del/case-1412067/bill/Program.cs:line 21
   at HelloWorld.Program.Main(String[] args) in /home/bill/dev/del/case-1412067/bill/Program.cs:line 13
Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.
   at HelloWorld.Program.TestClass`1.Test() in /home/bill/dev/del/case-1412067/bill/Program.cs:line 22
   at HelloWorld.Program.Main(String[] args) in /home/bill/dev/del/case-1412067/bill/Program.cs:line 13

With Mono there is a hard crash. Again it appears that the instance method is called (with a null this), but the runtime crashes attempting to lookup the generic info.

abbreviated output...

Thread 1 (Thread 0x7f725c522740 (LWP 30783)):
#0  0x00007f725c60cdff in __GI___wait4 (pid=30787, stat_loc=0x7ffffc08a6bc, options=0, usage=0x0) at ../sysdeps/unix/sysv/linux/wait4.c:27
#1  0x00007f725c34a82b in dump_native_stacktrace (signal=0x7f725be70842 "SIGSEGV", mctx=0x7ffffc08b280) at /home/bill/dev/runtime/src/mono/mono/mini/mini-posix.c:843
#2  0x00007f725c34a551 in mono_dump_native_crash_info (signal=0x7f725be70842 "SIGSEGV", mctx=0x7ffffc08b280, info=0x7ffffc08b5b0) at /home/bill/dev/runtime/src/mono/mono/mini/mini-posix.c:870
#3  0x00007f725c25c0a2 in mono_handle_native_crash (signal=0x7f725be70842 "SIGSEGV", mctx=0x7ffffc08b280, info=0x7ffffc08b5b0) at /home/bill/dev/runtime/src/mono/mono/mini/mini-exceptions.c:3004
#4  0x00007f725c154822 in mono_sigsegv_signal_handler_debug (_dummy=11, _info=0x7ffffc08b5b0, context=0x7ffffc08b480, debug_fault_addr=0x0) at /home/bill/dev/runtime/src/mono/mono/mini/mini-runtime.c:3778
#5  0x00007f725c1545d3 in mono_sigsegv_signal_handler (_dummy=11, _info=0x7ffffc08b5b0, context=0x7ffffc08b480) at /home/bill/dev/runtime/src/mono/mono/mini/mini-runtime.c:3811
#6  <signal handler called>
#7  0x00007f725c257558 in mono_get_generic_info_from_stack_frame (ji=0x560983b1f080, ctx=0x7ffffc08bd98) at /home/bill/dev/runtime/src/mono/mono/mini/mini-exceptions.c:863
#8  0x00007f725c259515 in ves_icall_get_frame_info (skip=-1, need_file_info=1 '\001', method=0x7ffffc08c0c0, iloffset=0x7ffffc08c0b8, native_offset=0x7ffffc08c0b0, file=0x7ffffc08c0a8, line=0x7ffffc08c0a0, column=0x7ffffc08c098) at /home/bill/dev/runtime/src/mono/mono/mini/mini-exceptions.c:1501

...   

=================================================================
        Managed Stacktrace:
=================================================================
          at <unknown> <0xffffffff>
          at System.Diagnostics.StackFrame:get_frame_info <0x000b9>
          at System.Diagnostics.StackFrame:BuildStackFrame <0x000af>
          at System.Diagnostics.StackFrame:.ctor <0x00047>
          at System.Diagnostics.StackTrace:InitializeForCurrentThread <0x000bf>
          at System.Diagnostics.StackTrace:.ctor <0x00033>
          at System.Environment:get_StackTrace <0x00043>
          at TestClass`1:Test <0x00027>
          at HelloWorld.Program:Main <0x0007c>
          at <Module>:runtime_invoke_void_object <0x00091>

Regression?

Unknown

Known Workarounds

Don't invoke an instance method via reflection with a null this?

Configuration

I tested this with Linux (WSL) on the Main branch.

Other information

No response

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.

[bholmes]

This is an issue found in Unity that causes the editor to crash. My top priority for this bug is to avoid the Mono crash. As usual, I also request a potential fix be applied in mono/mono as well.

[EgorBo]

Interesting, so it doesn't check delegate's target for null, simplified repro:

private static void Main()
{
    var del = (Action<Program>)typeof(Program)
        .GetMethod("Test")
        .CreateDelegate(typeof(Action<Program>), null);

    del(null);
}

public void Test() { }

doesn't crash with NRE.
Checked .NET Framework 4.5.2 - same behavior

[bholmes]

Removing the generic from the sample avoids the mono runtime crash as the crash occurs in the generic info lookup code. Depending on the perspective of the real bug here, it could remain important.

And yes I have verified that your simplified case does indeed run and both runtimes make the invocation without a null check.

[jkotas]

The program should throw an argument or null reference exception within the Invoke call?

At runtime level, it is valid to call a method with null this.

[MichalStrehovsky]

The expected behavior would be to get a NullReferenceException whenever this needs to be accessed. This includes accessing this to retrieve the generic context (i.e. not just when accessing instance fields). The above program doesn't appear to have a need to access instance fields or the generic context.

[bholmes]

There are 2 parts to this issue. The most important being that the Mono runtime crashes inside mono_get_generic_info_from_stack_frame. Constraining this issue to the Mono crash is acceptable.

The second part may just be a discussion of what is the correct managed exception to throw and when. I am assuming from @jkotas's comment, a managed exception thrown at the point where 'this' is accessed is the expected behavior. IIRC both runtimes behave this way today so that is good news.

That leaves us with the first problem to be addressed. Is that correct?