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.

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

Possibly unintended reuse of ref local to in/ref readonly parameters causes unexpected mutation #73438

Closed
Emik03 opened this issue May 12, 2024 · 2 comments
Labels
Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead

Comments

@Emik03
Copy link

Emik03 commented May 12, 2024

Version Used: 8.0.204 and 9.0.100-preview.3.24204.13.

Steps to Reproduce:

  1. Copy paste the following code into a REPL, IDE, or SharpLab.
using System;

ReadOnlySpan<bool> a = new(true);
ReadOnlySpan<bool> b = new(false);

Console.WriteLine(a[0]);
Console.WriteLine(b[0]);
  1. Run the program or decompile the output. In both Debug and Release, observe that both spans resolve to false, which causes the program to output False twice.
private static void <Main>$(string[] args)
{
    bool reference = true;
    ReadOnlySpan<bool> readOnlySpan = new ReadOnlySpan<bool>(ref reference);
    reference = false;
    ReadOnlySpan<bool> readOnlySpan2 = new ReadOnlySpan<bool>(ref reference);
    Console.WriteLine(readOnlySpan[0]);
    Console.WriteLine(readOnlySpan2[0]);
}

This was discovered while I was writing a library, and finally narrowed it down to these two problematic lines.

// 'entries' splits by new lines lazily.
// The reference gets assigned to '\n' at the start.
var entries = mountTable.AsSpan().SplitOn((byte)'\n');

foreach (var entry in entries)
{
    if (entry is [(byte)'#', ..])
        continue;

    var escaped = entry.SplitOn(s_whitespace)[1];

    // With the execution of this line, subsequent enumerations
    // split by '\' instead due to the shared reference.
    var (first, rest) = escaped.SplitOn((byte)'\\');
// ...

Changing this would technically be a breaking change, and I assume that perhaps this is in some way intended for optimization reasons. If altering behavior is not desired, I would strongly suggest there to be a warning when two in parameters are used within the same scope and of the same type.

Diagnostic Id:

N/A

Expected Behavior:

A reference is generated for every value separately, unless it can be proven that sharing the same reference won't affect the program, such as if both variables live in separate scopes and do not have their lifetimes overlap.

Actual Behavior:

A shared reference is used between multiple seemingly unrelated values, causing unexpected changes when a different in/ref readonly method is invoked.

@jjonescz jjonescz added the untriaged Issues and PRs which have not yet been triaged by a lead label May 15, 2024
@jaredpar
Copy link
Member

Going to let #67435 be the primary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

No branches or pull requests

3 participants