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

Unexpected behavior with nested [InlineArray] #97603

Closed
JeremyKuhne opened this issue Jan 27, 2024 · 4 comments
Closed

Unexpected behavior with nested [InlineArray] #97603

JeremyKuhne opened this issue Jan 27, 2024 · 4 comments

Comments

@JeremyKuhne
Copy link
Member

Description

I'm seeing random data when nesting a ref struct with an [InlineArray] in another ref struct.

Reproduction Steps

Given the following type definitions:

public ref struct FixedPointBuffer
{
    private FixedBuffer16<Point> _buffer;

    public FixedPointBuffer() => _buffer = new();

    public ref Point this[int i] => ref _buffer[i];
}

public ref struct FixedBuffer16<T>
{
    private StackBuffer _stackBuffer;
    private Buffer<T> _buffer;

    [InlineArray(16)]
    private struct StackBuffer
    {
        internal T _element0;
    }

    public unsafe FixedBuffer16() => _buffer = new Buffer<T>(_stackBuffer);
    public ref T this[int i] => ref _buffer[i];
}

public ref struct Buffer<T>
{
    private Span<T> _buffer;
    public Buffer(Span<T> values) => _buffer = values;
    public ref T this[int i] => ref _buffer[i];
}

I get the following behavior:

FixedBuffer16<Point> buffer1 = new();
buffer1[0] = new(1, 2);
// value is (1, 2)
var value = buffer1[0];

FixedPointBuffer buffer2 = new();
buffer2[0] = new(3, 4);
// value is random
value = buffer2[0];

Why would the first case work and the second not? Am I missing some detail on how things work with refs? Is this a bug?

Expected behavior

Would expect that this would just work.

Actual behavior

Garbage from the nesting type.

Regression?

No response

Known Workarounds

No response

Configuration

.NET 8

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jan 27, 2024
@ghost
Copy link

ghost commented Jan 27, 2024

Tagging subscribers to this area: @dotnet/area-system-buffers
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

I'm seeing random data when nesting a ref struct with an [InlineArray] in another ref struct.

Reproduction Steps

Given the following type definitions:

public ref struct FixedPointBuffer
{
    private FixedBuffer16<Point> _buffer;

    public FixedPointBuffer() => _buffer = new();

    public ref Point this[int i] => ref _buffer[i];
}

public ref struct FixedBuffer16<T>
{
    private StackBuffer _stackBuffer;
    private Buffer<T> _buffer;

    [InlineArray(16)]
    private struct StackBuffer
    {
        internal T _element0;
    }

    public unsafe FixedBuffer16() => _buffer = new Buffer<T>(_stackBuffer);
    public ref T this[int i] => ref _buffer[i];
}

public ref struct Buffer<T>
{
    private Span<T> _buffer;
    public Buffer(Span<T> values) => _buffer = values;
    public ref T this[int i] => ref _buffer[i];
}

I get the following behavior:

FixedBuffer16<Point> buffer1 = new();
buffer1[0] = new(1, 2);
// value is (1, 2)
var value = buffer1[0];

FixedPointBuffer buffer2 = new();
buffer2[0] = new(3, 4);
// value is random
value = buffer2[0];

Why would the first case work and the second not? Am I missing some detail on how things work with refs? Is this a bug?

Expected behavior

Would expect that this would just work.

Actual behavior

Garbage from the nesting type.

Regression?

No response

Known Workarounds

No response

Configuration

.NET 8

Other information

No response

Author: JeremyKuhne
Assignees: -
Labels:

area-System.Buffers, untriaged

Milestone: -

@jakobbotsch
Copy link
Member

jakobbotsch commented Jan 27, 2024

FixedPointBuffer's constructor is required to allocate a new temporary to call FixedBuffer16<Point>'s constructor on. Then the initialized temporary is copied to the _buffer field, but it points to the stack frame of FixedPointBuffer's constructor. When you return from the constructor, that is no longer valid.
Roslyn does produce a warning in FixedBuffer16<Point>'s constructor about this, although I'm not sure whether this is expected to be the only diagnostic produced. cc @jaredpar

To reliably do something like this you probably need to make the constructors static methods instead and have them take explicit ref parameters.

Edit: @reflectronic points out that unsafe is what demotes the error to a warning (ref. dotnet/csharplang#6476), so seems expected.

@Windows10CE
Copy link
Contributor

Windows10CE commented Jan 27, 2024

The problem here is much more obvious if you attempt to do this:

static FixedBuffer16<T> CreateBuffer<T>() => new();

You are creating a struct that contains a reference to its own original construction location, in FixedPointBuffer (depending on optimization level) that would be its constructor's stackframe, leading you to reference a location on the stack that is no longer valid, and can therefore be overwritten at any time.

@jkotas
Copy link
Member

jkotas commented Jan 27, 2024

By design according to the linked issues.

@jkotas jkotas closed this as completed Jan 27, 2024
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jan 27, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Feb 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants