Skip to content

Fix the interpreted version of Expression<>.Compile() for setting field on value types #116901

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+478 −66
Open

Fix the interpreted version of Expression<>.Compile() for setting field on value types #116901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8866da3
Add tests showing buggy behaviour
manofstick Jun 22, 2025
7a8f4a9
Add FieldData object to represent valuetype field operation; wrap access
manofstick Jun 22, 2025
669ca7d
Utilize GetValueDirect
manofstick Jun 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/libraries/System.Linq.Expressions/src/System.Linq.Expressions.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@
<Compile Include="System\Linq\Expressions\Interpreter\DivInstruction.cs" />
<Compile Include="System\Linq\Expressions\Interpreter\EqualInstruction.cs" />
<Compile Include="System\Linq\Expressions\Interpreter\ExclusiveOrInstruction.cs" />
<Compile Include="System\Linq\Expressions\Interpreter\FieldData.cs" />
<Compile Include="System\Linq\Expressions\Interpreter\FieldOperations.cs" />
<Compile Include="System\Linq\Expressions\Interpreter\GreaterThanInstruction.cs" />
<Compile Include="System\Linq\Expressions\Interpreter\GreaterThanOrEqualInstruction.cs" />
Expand Down
28 changes: 14 additions & 14 deletions ...braries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/AddInstruction.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ private sealed class AddInt16 : AddInstruction
public override int Run(InterpretedFrame frame)
{
int index = frame.StackIndex;
object?[] stack = frame.Data;
InterpretedFrame.DataView stack = frame.Data;
object? left = stack[index - 2];
if (left != null)
{
Expand All @@ -39,7 +39,7 @@ private sealed class AddInt32 : AddInstruction
public override int Run(InterpretedFrame frame)
{
int index = frame.StackIndex;
object?[] stack = frame.Data;
InterpretedFrame.DataView stack = frame.Data;
object? left = stack[index - 2];
if (left != null)
{
Expand All @@ -57,7 +57,7 @@ private sealed class AddInt64 : AddInstruction
public override int Run(InterpretedFrame frame)
{
int index = frame.StackIndex;
object?[] stack = frame.Data;
InterpretedFrame.DataView stack = frame.Data;
object? left = stack[index - 2];
if (left != null)
{
Expand All @@ -75,7 +75,7 @@ private sealed class AddUInt16 : AddInstruction
public override int Run(InterpretedFrame frame)
{
int index = frame.StackIndex;
object?[] stack = frame.Data;
InterpretedFrame.DataView stack = frame.Data;
object? left = stack[index - 2];
if (left != null)
{
Expand All @@ -93,7 +93,7 @@ private sealed class AddUInt32 : AddInstruction
public override int Run(InterpretedFrame frame)
{
int index = frame.StackIndex;
object?[] stack = frame.Data;
InterpretedFrame.DataView stack = frame.Data;
object? left = stack[index - 2];
if (left != null)
{
Expand All @@ -111,7 +111,7 @@ private sealed class AddUInt64 : AddInstruction
public override int Run(InterpretedFrame frame)
{
int index = frame.StackIndex;
object?[] stack = frame.Data;
InterpretedFrame.DataView stack = frame.Data;
object? left = stack[index - 2];
if (left != null)
{
Expand All @@ -129,7 +129,7 @@ private sealed class AddSingle : AddInstruction
public override int Run(InterpretedFrame frame)
{
int index = frame.StackIndex;
object?[] stack = frame.Data;
InterpretedFrame.DataView stack = frame.Data;
object? left = stack[index - 2];
if (left != null)
{
Expand All @@ -147,7 +147,7 @@ private sealed class AddDouble : AddInstruction
public override int Run(InterpretedFrame frame)
{
int index = frame.StackIndex;
object?[] stack = frame.Data;
InterpretedFrame.DataView stack = frame.Data;
object? left = stack[index - 2];
if (left != null)
{
Expand Down Expand Up @@ -193,7 +193,7 @@ private sealed class AddOvfInt16 : AddOvfInstruction
public override int Run(InterpretedFrame frame)
{
int index = frame.StackIndex;
object?[] stack = frame.Data;
InterpretedFrame.DataView stack = frame.Data;
object? left = stack[index - 2];
if (left != null)
{
Expand All @@ -211,7 +211,7 @@ private sealed class AddOvfInt32 : AddOvfInstruction
public override int Run(InterpretedFrame frame)
{
int index = frame.StackIndex;
object?[] stack = frame.Data;
InterpretedFrame.DataView stack = frame.Data;
object? left = stack[index - 2];
if (left != null)
{
Expand All @@ -229,7 +229,7 @@ private sealed class AddOvfInt64 : AddOvfInstruction
public override int Run(InterpretedFrame frame)
{
int index = frame.StackIndex;
object?[] stack = frame.Data;
InterpretedFrame.DataView stack = frame.Data;
object? left = stack[index - 2];
if (left != null)
{
Expand All @@ -247,7 +247,7 @@ private sealed class AddOvfUInt16 : AddOvfInstruction
public override int Run(InterpretedFrame frame)
{
int index = frame.StackIndex;
object?[] stack = frame.Data;
InterpretedFrame.DataView stack = frame.Data;
object? left = stack[index - 2];
if (left != null)
{
Expand All @@ -265,7 +265,7 @@ private sealed class AddOvfUInt32 : AddOvfInstruction
public override int Run(InterpretedFrame frame)
{
int index = frame.StackIndex;
object?[] stack = frame.Data;
InterpretedFrame.DataView stack = frame.Data;
object? left = stack[index - 2];
if (left != null)
{
Expand All @@ -283,7 +283,7 @@ private sealed class AddOvfUInt64 : AddOvfInstruction
public override int Run(InterpretedFrame frame)
{
int index = frame.StackIndex;
object?[] stack = frame.Data;
InterpretedFrame.DataView stack = frame.Data;
object? left = stack[index - 2];
if (left != null)
{
Expand Down
16 changes: 8 additions & 8 deletions ...braries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/DivInstruction.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ private sealed class DivInt16 : DivInstruction
public override int Run(InterpretedFrame frame)
{
int index = frame.StackIndex;
object?[] stack = frame.Data;
InterpretedFrame.DataView stack = frame.Data;
object? left = stack[index - 2];
if (left != null)
{
Expand All @@ -39,7 +39,7 @@ private sealed class DivInt32 : DivInstruction
public override int Run(InterpretedFrame frame)
{
int index = frame.StackIndex;
object?[] stack = frame.Data;
InterpretedFrame.DataView stack = frame.Data;
object? left = stack[index - 2];
if (left != null)
{
Expand All @@ -57,7 +57,7 @@ private sealed class DivInt64 : DivInstruction
public override int Run(InterpretedFrame frame)
{
int index = frame.StackIndex;
object?[] stack = frame.Data;
InterpretedFrame.DataView stack = frame.Data;
object? left = stack[index - 2];
if (left != null)
{
Expand All @@ -75,7 +75,7 @@ private sealed class DivUInt16 : DivInstruction
public override int Run(InterpretedFrame frame)
{
int index = frame.StackIndex;
object?[] stack = frame.Data;
InterpretedFrame.DataView stack = frame.Data;
object? left = stack[index - 2];
if (left != null)
{
Expand All @@ -93,7 +93,7 @@ private sealed class DivUInt32 : DivInstruction
public override int Run(InterpretedFrame frame)
{
int index = frame.StackIndex;
object?[] stack = frame.Data;
InterpretedFrame.DataView stack = frame.Data;
object? left = stack[index - 2];
if (left != null)
{
Expand All @@ -111,7 +111,7 @@ private sealed class DivUInt64 : DivInstruction
public override int Run(InterpretedFrame frame)
{
int index = frame.StackIndex;
object?[] stack = frame.Data;
InterpretedFrame.DataView stack = frame.Data;
object? left = stack[index - 2];
if (left != null)
{
Expand All @@ -129,7 +129,7 @@ private sealed class DivSingle : DivInstruction
public override int Run(InterpretedFrame frame)
{
int index = frame.StackIndex;
object?[] stack = frame.Data;
InterpretedFrame.DataView stack = frame.Data;
object? left = stack[index - 2];
if (left != null)
{
Expand All @@ -147,7 +147,7 @@ private sealed class DivDouble : DivInstruction
public override int Run(InterpretedFrame frame)
{
int index = frame.StackIndex;
object?[] stack = frame.Data;
InterpretedFrame.DataView stack = frame.Data;
object? left = stack[index - 2];
if (left != null)
{
Expand Down
73 changes: 73 additions & 0 deletions src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/FieldData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Reflection;

namespace System.Linq.Expressions.Interpreter
{
internal sealed class FieldData
{
private readonly object _parent;
private readonly FieldInfo _field;

public FieldData(object parent, FieldInfo field)
{
_parent = parent;
_field = field;
}

private (object Root, FieldInfo[] FieldInfos) GetFieldAccessors(int count)
{
if (_parent is FieldData parentField)
{
var accessors = parentField.GetFieldAccessors(count + 1);

accessors.FieldInfos[^count] = _field;

return accessors;
}
else
{
FieldInfo[] fieldInfos = new FieldInfo[count];

fieldInfos[0] = _field;

return (_parent, fieldInfos);
}
}

public object? ToObject()
{
(object root, FieldInfo[] fieldInfos) = GetFieldAccessors(1);

var typedReference = TypedReference.MakeTypedReference(root, fieldInfos);

return TypedReference.ToObject(typedReference);
}

public void SetValueDirect(FieldInfo field, object? value)
{
(object root, FieldInfo[] fieldInfos) = GetFieldAccessors(1);

var typedReference = TypedReference.MakeTypedReference(root, fieldInfos);

field.SetValueDirect(typedReference, value!);
}

public object? GetValueDirect(FieldInfo field)
{
(object root, FieldInfo[] fieldInfos) = GetFieldAccessors(1);

var typedReference = TypedReference.MakeTypedReference(root, fieldInfos);

return field.GetValueDirect(typedReference);
}

public static void SetValueDirect(object obj, FieldInfo field, object? value)
{
var typedReference = __makeref(obj);

field.SetValueDirect(typedReference, value!);
}
}
}
39 changes: 34 additions & 5 deletions ...raries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/FieldOperations.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,20 @@ public LoadFieldInstruction(FieldInfo field)

public override int Run(InterpretedFrame frame)
{
object? self = frame.Pop();
object? self = frame.PopRaw();

NullCheck(self);
frame.Push(_field.GetValue(self));

object? frameData =
(self, _field.FieldType) switch
{
(_, { IsPrimitive: false, IsValueType: true }) => new FieldData(self!, _field),
(FieldData fieldData, _) => fieldData.GetValueDirect(_field),
(_, _) => _field.GetValue(self),
};

frame.Push(frameData);

return 1;
}
}
Expand All @@ -72,10 +82,29 @@ public StoreFieldInstruction(FieldInfo field)
public override int Run(InterpretedFrame frame)
{
object? value = frame.Pop();
object? self = frame.Pop();

NullCheck(self);
_field.SetValue(self, value);
if (_field.DeclaringType is not { IsPrimitive: false, IsValueType: true })
{
object? self = frame.Pop();
NullCheck(self);

_field.SetValue(self, value);
}
else
{
object? self = frame.PopRaw();
NullCheck(self);

if (self is FieldData fieldData)
{
fieldData.SetValueDirect(_field, value);
}
else
{
FieldData.SetValueDirect(self!, _field, value);
}
}

return 1;
}
}
Expand Down
28 changes: 26 additions & 2 deletions ...aries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/InterpretedFrame.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,23 @@ namespace System.Linq.Expressions.Interpreter
{
internal sealed class InterpretedFrame
{
public readonly struct DataView(object?[] objects)
{
public object? this[int index]
{
get =>
objects[index] switch
{
FieldData fieldData => fieldData.ToObject(),
var nonFieldData => nonFieldData
};

set => objects[index] = value;
}

public int Length => objects.Length;
}

[ThreadStatic]
private static InterpretedFrame? s_currentFrame;

Expand All @@ -21,7 +38,8 @@ internal sealed class InterpretedFrame
private int _pendingContinuation;
private object? _pendingValue;

public readonly object?[] Data;
private readonly object?[] _rawData;
public readonly DataView Data;

public readonly IStrongBox[]? Closure;

Expand All @@ -38,7 +56,8 @@ internal InterpretedFrame(Interpreter interpreter, IStrongBox[]? closure)
{
Interpreter = interpreter;
StackIndex = interpreter.LocalCount;
Data = new object[StackIndex + interpreter.Instructions.MaxStackDepth];
_rawData = new object[StackIndex + interpreter.Instructions.MaxStackDepth];
Data = new DataView(_rawData);

int c = interpreter.Instructions.MaxContinuationDepth;
if (c > 0)
Expand Down Expand Up @@ -96,6 +115,11 @@ public void Push(ushort value)
Data[StackIndex++] = value;
}

public object? PopRaw()
{
return _rawData[--StackIndex];
}

public object? Pop()
{
return Data[--StackIndex];
Expand Down
Loading
Loading