dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.06k stars 4.04k forks source link

The compiler should report an error about references to out-of-scope stack memory generated by [UnscopedRef], defensively copied struct and pattern matching #75484

Open TessenR opened 1 month ago

TessenR commented 1 month ago

Version Used:

Microsoft Visual Studio Professional 2022
Version 17.12.0 Preview 2.1
VisualStudio.17.Preview/17.12.0-pre.2.1+35323.107
Microsoft .NET Framework
Version 4.8.09037

Steps to Reproduce:

Compile and run the following code:

using System;
using System.Runtime.CompilerServices;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;

namespace Net7Test;

internal class Program
{
  private static readonly Vec4 ReadOnlyVec = new Vec4(1, 2, 3, 4);

  static void Main(string[] args)
  {
    Console.WriteLine(RuntimeInformation.FrameworkDescription);

    // This refers to stack memory that has already been left out.
    ref Vec4 foo = ref Foo().x;

    foo = new Vec4(0, 0, 0, 0);
    Console.WriteLine(foo.X);
    MakeStackDirty();
    Console.WriteLine(foo.X);
    Console.WriteLine(foo);
  }

  [MethodImpl(MethodImplOptions.NoInlining)]
  private static Wrapper Foo()
  {
    // Defensive copy occurs and it is placed in stack memory implicitly.
    // The method returns a reference to the copy, which happens invalid memory access.
    if (ReadOnlyVec is var (x, _)) return x;

    throw null!;
  }

  [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)]
  private static void MakeStackDirty()
  {
    Span<byte> mem = stackalloc byte[1028];
    mem.Clear();
  }
}

public struct Vec4
{
  public float X, Y, Z, W;
  public Vec4(float x, float y, float z, float w) => (X, Y, Z, W) = (x, y, z, w);

  [UnscopedRef]
  public ref Vec4 Self => ref this;

  [UnscopedRef]
  public void Deconstruct(out Wrapper x, out Vec4 y)
  {
    x = new Wrapper(ref this);
    y = Self;
  }

  public override string ToString() => $"<{X}, {Y}, {Z}, {W}>";
}

public ref struct Wrapper
{
  public ref Vec4 x;

  public Wrapper(ref Vec4 X)
  {
    x = ref X;
  }

  [UnscopedRef]
  public ref Vec4 Self => ref x;

  public override string ToString() => x.ToString();
}

Diagnostic Id:

CS8352: Cannot use variable in this context because it may expose referenced variables outside of their declaration scope

Expected Behavior: There should be an error for return x; in Foo as it returns a ref struct wrapping a reference to stack memory that's going to be left out.

Actual Behavior:

The code compiles and prints

.NET 9.0.0-rc.1.24431.7
0
-5.0481173E+37
<-5.0483445E+37, 3.35E-43, 3.9070498E-24, 4.5911E-41>

Notes: This is a variation of https://github.com/dotnet/roslyn/issues/64776 using pattern matching which Roslyn fails to verify If you replace

    if (ReadOnlyVec is var (x, _)) return x;

with an explicit call to Deconstruct Roslyn will correctly report an error:

    ReadOnlyVec.Deconstruct(out var y, out _);
    return y; // error CS8352
jaredpar commented 4 weeks ago

Guessing that we are not handling the lifetime during the implicit deconstruct call there.