Runtime: C# `in` parameter modifier: Should I be able to create a delegate for a method with non-matching signature?

[stakx]

While working on Moq (https://github.com/moq/moq4/pull/625), I've run into something strange: It appears that the runtime does not match signatures properly when creating delegates through e.g. MethodInfo.CreateDelegate. It appears that custom modifiers aren't taken into account at all:

using System;

public delegate void WithIn(in AttributeTargets arg);
//                          ^^
// C# compiler emits a required custom modifier (among other things; see IL below)

public interface IWithoutIn
{
    void Invoke(ref AttributeTargets arg);
    //          ^^^
    // no custom modifier here
}

public class WithoutIn : IWithoutIn
{
    public void Invoke(ref AttributeTargets arg)
    {
        Console.WriteLine($"Called with {arg}.");
    }
}

class Program
{
    static void Main()
    {
        var interfaceMethod = typeof(IWithoutIn).GetMethod("Invoke");
        var interfaceMethodViaDelegate = (WithIn)interfaceMethod.CreateDelegate(typeof(WithIn), new WithoutIn());
        interfaceMethodViaDelegate.Invoke(AttributeTargets.All);
    }
}

I can create a delegate with signature void(in AttributeTargets) for a method with signature void(ref AttributeTargets). This doesn't seem right, since the signatures differ by a required custom modifier. From ECMA-335, section 13.6 Delegates:

_"The Invoke method shall be virtual and have the same signature (return type, parameter types, calling convention, and modifiers [...]) as the target method. When actually called, the arguments passed shall match the types specified in this signature."_ (emphasis added by me)

Is this imprecise signature matching for delegates intentional, or a bug?

(Expand to see the IL for WithIn and IWithoutIn)

.class public auto ansi sealed WithIn extends [System.Runtime]System.MulticastDelegate
{
  // (ctor omitted)

  .method public hidebysig newslot virtual instance void Invoke(
    [in] valuetype [System.Runtime]System.AttributeTargets& modreq([System.Runtime.InteropServices]System.Runtime.InteropServices.InAttribute) arg) runtime managed
  {
    .param [1]
    .custom instance void [System.Runtime]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 ) 
  }

  // (BeginInvoke and EndInvoke omitted)
}

.class interface public abstract auto ansi IWithoutIn
{
  .method public hidebysig newslot abstract virtual instance void Invoke(
    valuetype [System.Runtime]System.AttributeTargets& arg) cil managed
  {
  }
}

/cc @zvirja

[zvirja]

Notice, this issue might lead to the critical language contract violation:
```c#
public class InRewritter
{
public delegate void DelegateWithIn(in MutableStruct value);

public class TypeWithRef
{
    public void Method(ref MutableStruct value)
    {
        value.Value = 42;
    }
}

public struct MutableStruct
{
    public int Value { get; set; }
}

[Fact]
public void Test()
{
    var deleg = (DelegateWithIn)typeof(TypeWithRef)
        .GetMethod(nameof(TypeWithRef.Method))
        .CreateDelegate(typeof(DelegateWithIn), new TypeWithRef());

    var val = new MutableStruct { Value = 24 };
    deleg.Invoke(val);

    Assert.Equal(24, val.Value);
}

}

You might expect that test should pass, but it fails:

Assert.Equal() Failure
Expected: 24
Actual: 42
```

The most upset part is that we are not using some very low-level features of .NET (like IL emit), but rather a simple reflection and it's very bad that now we cannot fully trust when passing the in value.

[stakx]

Small P.S.: This issue isn't exclusive to reflection. You can also recreate the situation in pure IL (i.e. use ldvirtfn to get a function pointer, then pass it to a (incompatible) delegate's ctor with newobj).

[jkotas]

From ECMA-335, section 13.6 Delegates

I do not see the text you have quoted in the current edition of the ECMA spec: http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-335.pdf

I specifically says that the custom modifiers are not considered for assignment compatibility in section I I.14.6.1 Delegate signature compatibility:

The delegate-assignable-to relation is defined in terms of the parameter types, ignoring the this
parameter, if any, the return type and calling convention. (Custom modifiers are not considered
significant and do not impact compatibility.)

[jkotas]

I am sympathetic to your concern and I wish we did not have discrepancies between the C# language and runtime like these. We ended up with several of them recently because of there was a strong desire to make new C# language features work on existing runtimes. We are reevaluating this strategy going forward.

cc @MadsTorgersen @jaredpar

[stakx]

@jkotas - Many thanks for replying. I can appreciate the difficulties of mapping new language features onto the runtime.

It would be good however to get a statement from you folks whether you see this as a bug (which will likely get fixed whenever—I'm not even asking for a timeline here) or whether it should be considered new behavior that's going to stay that way, so that we can plan accordingly in user code.

[jkotas]

This is design bug. Design bugs are hard to fix - I think it is unlikely to be fixed.

[mikedn]

it's very bad that now we cannot fully trust when passing the in value.

IMO it would be useful to clarify what you mean by this. With or without this issue, you really cannot trust that a method that takes an in parameter does not modify that value.

[stakx]

@jkotas - Thanks for letting me know. I'm going to leave this issue open for a few more days, in case anyone else wants to discuss this further. After that I'll close it.

[stakx]

(Actually, I do have one more thing myself, @jkotas:)

I wish we did not have discrepancies between the C# language and runtime like these.

(This doesn't look like a discrepancy between C# and the runtime. You can take C# out of the picture completely. This seems like a runtime-only defect, it doesn't appear to perform strict-enough signature matching.)

[jkotas]

The ECMA spec says that the custom modifiers are not relevant for delegate-assignable-to. This sentence was written in the spec intentionally, not by accident. The runtime works as intended if you take C# out of the picture.

BTW: It was always possible to change the value of readonly fields using reflection (in full trust environments). It is a similar design bug.

[stakx]

@jkotas - The delegate-assignable-to relation in ECMA 335 section II.14.6.1 is in fact what I was looking for. If I had come upon that section in time, I wouldn't have opened this issue at all. Thanks so much for pointing me to it.

[GrabYourPitchforks]

I think we should reconsider this issue - or at least the part where modreqs are ignored as part of signature matching. Here's a follow-up sample to @zvirja's repro. It allows mutating string literals using only "safe" public reflection.

using System;

namespace Demo
{
    class Program
    {
        delegate ref T Indexer<T>(in ReadOnlySpan<T> span, int idx);

        static void Main()
        {
            var mi = typeof(ReadOnlySpan<char>).GetMethod("get_Item");
            var del = mi.CreateDelegate<Indexer<char>>();
            del("Hello!", 0) = 'J';
            Console.WriteLine("Hello!"); // prints "Jello!"
        }
    }
}

Edit: I know this doesn't fix the in / ref / out issue, since those are implemented as standard attributes rather than modreqs. And it'd still be possible to mutate string instances by passing in myString.AsSpan()[0] to a method whose parameter is typed as out char. But that still seems like something we should be able to address.

And if this really is by-design, what line of code above is the one where the type safety violation occurs? The call to _CreateDelegate_? Might have implications for the doc we're working on at https://github.com/dotnet/runtime/issues/41418.

Edit 2: Huh, apparently the below is unverifiable. Reason is that the compiler emits readonly. before the ldelema. And per ECMA-335, § III.1.8.1.2.2, the call PrintIt(string&) instruction which immediately follows is unverifiable because the controlled mutability pointer is not passed as the _this_ parameter. So it seems that since the method is only ever intended to be called via unverifiable constructs, any public reflection over such a method is also doomed to have type safety concerns.

static void Main(string[] args)
{
    string[] strings = new string[] { "Hello!" };
    PrintIt(strings[0]);
}

private static void PrintIt(in string s) { Console.WriteLine(s); }