Skip to content

Commit

Permalink
Avoid throwing when attempting to set IInternalModelingOnlyValue (#94404
Browse files Browse the repository at this point in the history 
)

The cctor interpreter represents some things that can only be used within the interpreter (but cannot be serialized to frozen blobs) as `IInternalModelingOnlyValue`. There was a piece of lazy code that was considering field assignments with these as invalid IL (we throw on invalid IL that we then catch later). Use the usual `Status.Fail` instead to reduce first chance exceptions. We don't want first chance exceptions in valid code.
  • Loading branch information
@MichalStrehovsky
MichalStrehovsky authored Nov 6, 2023
1 parent 1ddb513 commit f609282
Showing 1 changed file with 19 additions and 16 deletions.
35 changes: 19 additions & 16 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/TypePreinit.cs
View file
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

0 comments on commit f609282

Please sign in to comment.