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.

Sign up for GitHub

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

Jump to bottom

UnscopedRef can be different in interface implementation, leading to ref safety holes #76100

Open
jjonescz opened this issue Nov 26, 2024 · 3 comments · May be fixed by #76296 or #76261
Open

UnscopedRef can be different in interface implementation, leading to ref safety holes #76100

jjonescz opened this issue Nov 26, 2024 · 3 comments · May be fixed by #76296 or #76261
Assignees
@jjonescz
Labels
Area-Compilers Bug Feature - Ref Fields
Milestone
17.13

Comments

@jjonescz
Copy link
Member

Version Used: 20241126.6 (b23da04)

Steps to Reproduce:

using System;
using System.Diagnostics.CodeAnalysis;

var r = F();
Console.WriteLine(r.F.ToString());

static R F()
{
    var r = new R();
    ((I)new C()).M(ref r);
    return r;
}

interface I
{
    void M(ref R r);
}

class C : I
{
    public void M([UnscopedRef] ref R r)
    {
        r.F = ref r.I;
    }
}

ref struct R
{
    public int I;
    public ref int F;
}

Expected Behavior: An error at ((I)new C()).M(ref r); - an error would be reported there if replaced with new C().M(ref r);.

Actual Behavior: No errors. The program accesses an invalid reference at runtime.
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label Nov 26, 2024
@Sergio0694
Copy link
Contributor

Just so I understand, the problem is just because the implementation is narrowing the escape scope, right? Ie. if you had:

interface I
{
    void M([UnscopedRef] ref R r);
}

class C : I
{
    public void M(ref R r)
    {
    }
}

That should be fine, right? Would that still be allowed?

@jjonescz
Copy link
Member Author

That should be fine, right? Would that still be allowed?

Yes, that should be fine. Per the spec, removing [UnscopedRef] in the interface implementation is allowed.

@jjonescz
Copy link
Member Author

Also this section of the spec needs to be updated - it does not consider the "silly cyclic self-assignment":

The compiler will report a diagnostic for unsafe scoped mismatches across overrides, interface implementations, and delegate conversions when:

  • The method returns a ref struct or returns a ref or ref readonly, or the method has a ref or out parameter of ref struct type, and
  • The method has at least one additional ref, in, or out parameter, or a parameter of ref struct type.

@jaredpar jaredpar added Bug and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Dec 2, 2024
@jaredpar jaredpar added this to the 17.13 milestone Dec 2, 2024
This was referenced Dec 4, 2024
Draft
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jjonescz jjonescz

Labels
Area-Compilers Bug Feature - Ref Fields
Projects
None yet
Milestone
17.13
3 participants
@jaredpar @jjonescz @Sergio0694