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

Avoid throwing when attempting to set IInternalModelingOnlyValue #94404

Merged
merged 1 commit into from
Nov 6, 2023
Merged
Changes from all commits
Commits
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
35 changes: 19 additions & 16 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/TypePreinit.cs
Original file line number Diff line number Diff line change
Expand Up @@ -712,13 +712,11 @@ private Status TryScanMethod(MethodIL methodIL, Value[] parameters, Stack<Method
return Status.Fail(methodIL.OwningMethod, opcode, "Byref field");
}

var settableInstance = instance.Value as IHasInstanceFields;
if (settableInstance == null)
if (instance.Value is not IHasInstanceFields settableInstance
|| !settableInstance.TrySetField(field, value))
{
return Status.Fail(methodIL.OwningMethod, opcode);
return Status.Fail(methodIL.OwningMethod, opcode, "Not settable");
}

settableInstance.SetField(field, value);
}
break;

Expand Down Expand Up @@ -2157,7 +2155,7 @@ public interface ISerializableReference : ISerializableValue
/// </summary>
private interface IHasInstanceFields
{
void SetField(FieldDesc field, Value value);
bool TrySetField(FieldDesc field, Value value);
Value GetField(FieldDesc field);
ByRefValue GetFieldAddress(FieldDesc field);
}
Expand Down Expand Up @@ -2505,7 +2503,7 @@ public bool TryAccessElement(int index, out Value value)
return true;
}

public void SetField(FieldDesc field, Value value) => ThrowHelper.ThrowInvalidProgramException();
public bool TrySetField(FieldDesc field, Value value) => false;

public Value GetField(FieldDesc field)
{
Expand Down Expand Up @@ -2587,7 +2585,7 @@ public override bool Equals(Value value)
}

Value IHasInstanceFields.GetField(FieldDesc field) => new FieldAccessor(PointedToBytes, PointedToOffset).GetField(field);
void IHasInstanceFields.SetField(FieldDesc field, Value value) => new FieldAccessor(PointedToBytes, PointedToOffset).SetField(field, value);
bool IHasInstanceFields.TrySetField(FieldDesc field, Value value) => new FieldAccessor(PointedToBytes, PointedToOffset).TrySetField(field, value);
ByRefValue IHasInstanceFields.GetFieldAddress(FieldDesc field) => new FieldAccessor(PointedToBytes, PointedToOffset).GetFieldAddress(field);

public void Initialize(int size)
Expand Down Expand Up @@ -2925,7 +2923,8 @@ private static byte[] ConstructStringInstance(TypeDesc stringType, ReadOnlySpan<
FieldDesc lengthField = stringType.GetField("_stringLength");
Debug.Assert(lengthField.FieldType.IsWellKnownType(WellKnownType.Int32)
&& lengthField.Offset.AsInt == pointerSize);
new FieldAccessor(bytes).SetField(lengthField, ValueTypeValue.FromInt32(value.Length));
bool success = new FieldAccessor(bytes).TrySetField(lengthField, ValueTypeValue.FromInt32(value.Length));
Debug.Assert(success);

FieldDesc firstCharField = stringType.GetField("_firstChar");
Debug.Assert(firstCharField.FieldType.IsWellKnownType(WellKnownType.Char)
Expand All @@ -2949,7 +2948,7 @@ public override bool GetRawData(NodeFactory factory, out object data)

public override ReferenceTypeValue ToForeignInstance(int baseInstructionCounter) => this;
Value IHasInstanceFields.GetField(FieldDesc field) => new FieldAccessor(_value).GetField(field);
void IHasInstanceFields.SetField(FieldDesc field, Value value) => ThrowHelper.ThrowInvalidProgramException();
bool IHasInstanceFields.TrySetField(FieldDesc field, Value value) => false;
ByRefValue IHasInstanceFields.GetFieldAddress(FieldDesc field) => new FieldAccessor(_value).GetFieldAddress(field);
}

Expand Down Expand Up @@ -2999,7 +2998,7 @@ public bool TryUnboxAny(TypeDesc type, out Value value)
}

Value IHasInstanceFields.GetField(FieldDesc field) => new FieldAccessor(_data).GetField(field);
void IHasInstanceFields.SetField(FieldDesc field, Value value) => new FieldAccessor(_data).SetField(field, value);
bool IHasInstanceFields.TrySetField(FieldDesc field, Value value) => new FieldAccessor(_data).TrySetField(field, value);
ByRefValue IHasInstanceFields.GetFieldAddress(FieldDesc field) => new FieldAccessor(_data).GetFieldAddress(field);

public override void WriteFieldData(ref ObjectDataBuilder builder, NodeFactory factory)
Expand Down Expand Up @@ -3050,7 +3049,7 @@ public ValueTypeValue GetField(FieldDesc field)
return result;
}

public void SetField(FieldDesc field, Value value)
public bool TrySetField(FieldDesc field, Value value)
{
Debug.Assert(!field.IsStatic);

Expand All @@ -3059,23 +3058,27 @@ public void SetField(FieldDesc field, Value value)
// Allow setting reference type fields to null. Since this is the only value we can
// write, this is a no-op since reference type fields are always null
Debug.Assert(value == null);
return;
return true;
}

int fieldOffset = field.Offset.AsInt;
int fieldSize = field.FieldType.GetElementSize().AsInt;
if (fieldOffset + fieldSize > _instanceBytes.Length - _offset)
ThrowHelper.ThrowInvalidProgramException();

if (value is IInternalModelingOnlyValue)
{
return false;
}

if (value is not ValueTypeValue vtValue)
{
// This is either invalid IL, or value is one of our modeling-only values
// that don't have a bit representation (e.g. function pointer).
ThrowHelper.ThrowInvalidProgramException();
return; // unreached
return false; // unreached
}

Array.Copy(vtValue.InstanceBytes, 0, _instanceBytes, _offset + fieldOffset, fieldSize);
return true;
}

public ByRefValue GetFieldAddress(FieldDesc field)
Expand Down
Loading