Skip to content

Commit

Permalink
Allow initonly field during ldflda and stfld import (#57385)
Browse files Browse the repository at this point in the history
  • Loading branch information
@am11
am11 authored Aug 18, 2021
1 parent a53ec50 commit b7eb828
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 41 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -711,7 +711,7 @@ private bool TryGetUnmanagedCallingConventionFromModOpt(MethodSignature signatur
// Default to managed since in the modopt case we need to differentiate explicitly using a calling convention that matches the default
// and not specifying a calling convention at all and using the implicit default case in P/Invoke stub inlining.
callConv = CorInfoCallConvExtension.Managed;
if (!signature.HasEmbeddedSignatureData || signature.GetEmbeddedSignatureData() == null)
if (!signature.HasEmbeddedSignatureData)
return false;

bool found = false;
Expand Down
34 changes: 25 additions & 9 deletions src/coreclr/tools/ILVerification/ILImporter.Verify.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,9 @@ private void FindEnclosingExceptionRegions()
}
}
// Check if offset is within the range [FilterOffset, HandlerOffset[
if (r.FilterOffset != -1 && r.FilterOffset <= offset && offset < r.HandlerOffset )
if (r.FilterOffset != -1 && r.FilterOffset <= offset && offset < r.HandlerOffset)
{
if(!basicBlock.FilterIndex.HasValue)
if (!basicBlock.FilterIndex.HasValue)
{
basicBlock.FilterIndex = j;
}
Expand Down Expand Up @@ -301,7 +301,7 @@ private void InitialPass()
ILOpcode opCode = (ILOpcode)ReadILByte();

previousWasPrefix = false;
again:
again:
switch (opCode)
{
// Check this pointer modification
Expand Down Expand Up @@ -1601,7 +1601,7 @@ void ImportCall(ILOpcode opcode, int token)
}

// To support direct calls on readonly byrefs, just pretend declaredThis is readonly too
if(declaredThis.Kind == StackValueKind.ByRef && (actualThis.Kind == StackValueKind.ByRef && actualThis.IsReadOnly))
if (declaredThis.Kind == StackValueKind.ByRef && (actualThis.Kind == StackValueKind.ByRef && actualThis.IsReadOnly))
{
declaredThis.SetIsReadOnly();
}
Expand Down Expand Up @@ -2107,8 +2107,9 @@ void ImportAddressOfField(int token, bool isStatic)
isPermanentHome = actualThis.Kind == StackValueKind.ObjRef || actualThis.IsPermanentHome;
instance = actualThis.Type;

if (field.IsInitOnly)
Check(_method.IsConstructor && field.OwningType == _method.OwningType && actualThis.IsThisPtr, VerifierError.InitOnly);
// TODO: verification of readonly references https://github.com/dotnet/runtime/issues/57444
// if (field.IsInitOnly)
// Check(_method.IsConstructor && field.OwningType == _method.OwningType && actualThis.IsThisPtr, VerifierError.InitOnly);
}

Check(_method.OwningType.CanAccess(field, instance), VerifierError.FieldAccess);
Expand All @@ -2122,10 +2123,9 @@ void ImportStoreField(int token, bool isStatic)
ClearPendingPrefix(Prefix.Volatile);

var value = Pop();

var field = ResolveFieldToken(token);

TypeDesc instance;

if (isStatic)
{
Check(field.IsStatic, VerifierError.ExpectedStaticField);
Expand Down Expand Up @@ -2154,7 +2154,8 @@ void ImportStoreField(int token, bool isStatic)
instance = actualThis.Type;

if (field.IsInitOnly)
Check(_method.IsConstructor && field.OwningType == _method.OwningType && actualThis.IsThisPtr, VerifierError.InitOnly);
Check(field.OwningType == _method.OwningType && actualThis.IsThisPtr &&
(_method.IsConstructor || HasIsExternalInit(_method.Signature)), VerifierError.InitOnly);
}

// Check any constraints on the fields' class --- accessing the field might cause a class constructor to run.
Expand Down Expand Up @@ -2647,6 +2648,21 @@ void CheckPendingPrefix(Prefix mask)
Check((mask & Prefix.Constrained) == 0, VerifierError.Constrained);
}

static bool HasIsExternalInit(MethodSignature signature)
{
if (signature.HasEmbeddedSignatureData)
{
foreach (var data in signature.GetEmbeddedSignatureData())
{
if (data.type is MetadataType mdType && mdType.Namespace == "System.Runtime.CompilerServices" && mdType.Name == "IsExternalInit" &&
data.index == MethodSignature.IndexOfCustomModifiersOnReturnType)
return true;
}
}

return false;
}

bool HasPendingPrefix(Prefix prefix)
{
return (_pendingPrefix & prefix) != 0;
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/SignatureTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ public SignatureTests(ITestOutputHelper output)

private static string GetModOptMethodSignatureInfo(MethodSignature signature)
{
if (!signature.HasEmbeddedSignatureData || signature.GetEmbeddedSignatureData() == null)
return "";
if (!signature.HasEmbeddedSignatureData)
return string.Empty;

StringBuilder sb = new StringBuilder();
foreach (EmbeddedSignatureData data in signature.GetEmbeddedSignatureData())
Expand Down
34 changes: 23 additions & 11 deletions src/tests/ilverify/ILTests/FieldTests.il
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,17 @@
.field public initonly int32 InstanceInitonlyField
.field public static initonly int32 StaticInitonlyField

.field private initonly int32 InstanceExternalInitField

.method public hidebysig specialname instance void modreq([System.Runtime]System.Runtime.CompilerServices.IsExternalInit) 'special.StoreExternalInitField.set_MyProperty_Valid'(int32 'value') cil managed { ret }
.method public hidebysig specialname instance void modreq([System.Runtime]System.Runtime.CompilerServices.IsExternalInit) set_MyProperty(int32 'value') cil managed
{
ldarg.0
ldarg.1
stfld int32 FieldTestsType::InstanceExternalInitField
ret
}

.method public instance void Stsfld.UnsatisfiedParentConstraints_Invalid_UnsatisfiedFieldParentInst() cil managed
{
ldc.i4.0
Expand All @@ -43,7 +54,8 @@
ret
}

.method public instance void Ldflda.InitonlyFieldOutsideCtor_Invalid_InitOnly() cil managed
// TODO: verification of readonly references https://github.com/dotnet/runtime/issues/57444
.method public instance void Ldflda.InitonlyFieldOutsideCtor_Valid() cil managed
{
ldarg.0
ldflda int32 FieldTestsType::InstanceInitonlyField
Expand All @@ -52,7 +64,7 @@
}

.method public hidebysig instance void 'special.StoreInitonlyField..ctor_Valid'() cil managed { ret }
.method public hidebysig specialname rtspecialname instance void .ctor() cil managed
.method public hidebysig specialname rtspecialname instance void .ctor() cil managed
{
ldarg.0
call instance void [System.Runtime]System.Object::.ctor()
Expand All @@ -63,7 +75,7 @@
}

.method public hidebysig instance void 'special.LoadAddrInitonlyField..ctor_Valid'(int32) cil managed { ret }
.method public hidebysig specialname rtspecialname instance void .ctor(int32) cil managed
.method public hidebysig specialname rtspecialname instance void .ctor(int32) cil managed
{
ldarg.0
call instance void [System.Runtime]System.Object::.ctor()
Expand All @@ -74,7 +86,7 @@
}

.method public hidebysig instance void 'special.LoadAddrInitonlyField..ctor_Valid'(int64) cil managed { ret }
.method public hidebysig specialname rtspecialname instance void .ctor(int64) cil managed
.method public hidebysig specialname rtspecialname instance void .ctor(int64) cil managed
{
ldarg.0
call instance void [System.Runtime]System.Object::.ctor()
Expand All @@ -85,7 +97,7 @@
}

.method public hidebysig instance void 'special.StoreInitonlyFieldOtherType..ctor_Invalid_InitOnly'(class OtherType c) cil managed { ret }
.method public hidebysig specialname rtspecialname instance void .ctor(class OtherType c) cil managed
.method public hidebysig specialname rtspecialname instance void .ctor(class OtherType c) cil managed
{
ldarg.0
call instance void [System.Runtime]System.Object::.ctor()
Expand All @@ -96,7 +108,7 @@
}

.method public hidebysig instance void 'special.StoreInitonlyFieldOtherInstance..ctor_Invalid_InitOnly'(class FieldTestsType c) cil managed { ret }
.method public hidebysig specialname rtspecialname instance void .ctor(class FieldTestsType c) cil managed
.method public hidebysig specialname rtspecialname instance void .ctor(class FieldTestsType c) cil managed
{
ldarg.0
call instance void [System.Runtime]System.Object::.ctor()
Expand All @@ -107,7 +119,7 @@
}

.method public hidebysig instance void 'special.StsfldInitonlyInCtor..ctor_Invalid_InitOnly'(bool) cil managed { ret }
.method public hidebysig specialname rtspecialname instance void .ctor(bool) cil managed
.method public hidebysig specialname rtspecialname instance void .ctor(bool) cil managed
{
ldarg.0
call instance void [System.Runtime]System.Object::.ctor()
Expand All @@ -117,7 +129,7 @@
}

.method public hidebysig instance void 'special.LdsfldInitonlyInCtor..ctor_Invalid_InitOnly'(char) cil managed { ret }
.method public hidebysig specialname rtspecialname instance void .ctor(char) cil managed
.method public hidebysig specialname rtspecialname instance void .ctor(char) cil managed
{
ldarg.0
call instance void [System.Runtime]System.Object::.ctor()
Expand All @@ -127,7 +139,7 @@
}

.method public hidebysig instance void 'special.LdsfldStslfdInitonlyCctor..cctor_Valid'() cil managed { ret }
.method public hidebysig specialname rtspecialname instance void .cctor() cil managed
.method public hidebysig specialname rtspecialname instance void .cctor() cil managed
{
ldsflda int32 FieldTestsType::StaticInitonlyField
pop
Expand All @@ -145,8 +157,8 @@
.field public initonly int32 InstanceInitonlyField
.field public static initonly int32 StaticInitonlyField

.method public hidebysig instance void 'special.LdfldStlfdInitonlyCctor..cctor_Invalid_InitOnly.InitOnly'() cil managed { ret }
.method public hidebysig specialname rtspecialname instance void .cctor() cil managed
.method public hidebysig instance void 'special.LdfldStlfdInitonlyCctor..cctor_Invalid_InitOnly'() cil managed { ret }
.method public hidebysig specialname rtspecialname instance void .cctor() cil managed
{
ldsfld class OtherType OtherType::Instance
ldflda int32 OtherType::InstanceInitonlyField
Expand Down
2 changes: 1 addition & 1 deletion src/tests/ilverify/ILTests/FunctionPointerTests.il
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
ret
}

.method public hidebysig instance void Call.SendFunctionPointerWithInaccessibleReturnType_Invalid_MethodAccess_MethodAccess() cil managed
.method public hidebysig instance void Call.SendFunctionPointerWithInaccessibleReturnType_Invalid_MethodAccess.MethodAccess() cil managed
{
newobj instance void C::.ctor()
ldftn class D/D_Private D::StaticMethodReturningPrivateTypeInstance(int32)
Expand Down
51 changes: 34 additions & 17 deletions src/tests/ilverify/TestDataLoader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
namespace ILVerification.Tests
{
/// <summary>
/// Parses the methods in the test assemblies.
/// Parses the methods in the test assemblies.
/// It loads all assemblies from the test folder defined in <code>TestDataLoader.TestAssemblyPath</code>
/// This class feeds the xunit Theories
/// </summary>
Expand Down Expand Up @@ -106,8 +106,8 @@ private static TheoryData<TestCase> GetTestTypeFromDll(Func<string[], TypeDefini
/// <summary>
/// Returns all methods that contain valid IL code based on the following naming convention:
/// [FriendlyName]_Valid
/// The method must contain 1 '_'. The part before the '_' is a friendly name describing what the method does.
/// The word after the '_' has to be 'Valid' (Case sensitive)
/// The method must contain 1 '_'. The part before the '_' is a friendly name describing what the method does.
/// The word after the '_' has to be 'Valid' (Case sensitive)
/// E.g.: 'SimpleAdd_Valid'
/// </summary>
public static TheoryData<TestCase> GetMethodsWithValidIL()
Expand All @@ -129,9 +129,9 @@ public static TheoryData<TestCase> GetMethodsWithValidIL()
/// The method name must contain 2 '_' characters.
/// 1. part: a friendly name
/// 2. part: must be the word 'Invalid' (Case sensitive)
/// 3. part: the expected VerifierErrors as string separated by '.'.
/// 3. part: the expected VerifierErrors as string separated by '.'.
/// E.g.: SimpleAdd_Invalid_ExpectedNumericType
/// </summary>
/// </summary>
public static TheoryData<TestCase> GetMethodsWithInvalidIL()
{
var methodSelector = new Func<string[], MethodDefinitionHandle, TestCase>((mparams, methodHandle) =>
Expand Down Expand Up @@ -176,20 +176,37 @@ private static TheoryData<TestCase> GetTestMethodsFromDll(Func<string[], MethodD
var method = (EcmaMethod)testModule.GetMethod(methodHandle);
var methodName = method.Name;

if (!String.IsNullOrEmpty(methodName) && methodName.Contains("_"))
if (!methodName.Contains('_', StringComparison.Ordinal))
continue;

var index = methodName.LastIndexOf("_Valid", StringComparison.Ordinal);
if (index < 0)
index = methodName.LastIndexOf("_Invalid", StringComparison.Ordinal);
if (index < 0)
continue;

var substring = methodName.Substring(index + 1);
var split = substring.Split('_');
string[] mparams = new string[split.Length + 1];
split.CopyTo(mparams, 1);
mparams[0] = methodName.Substring(0, index);
// examples of methodName to mparams transformation:
// * `get_Property` -> [ 'get_Property' ]
// * `CheckSomething_Valid` -> [ 'CheckSomething', 'Valid' ]
// * 'WrongMethod_Invalid_BranchOutOfTry' -> [ 'WrongMethod', 'Invalid', 'BranchOutOfTry' ]
// * 'MoreWrongMethod_Invalid_TypeAccess.InitOnly' -> [ 'MoreWrongMethod', 'Invalid', 'TypeAccess', 'InitOnly' ]
// * 'special.set_MyProperty.set_MyProperty_Invalid_InitOnly' -> [ 'special.set_MyProperty.set_MyProperty', 'Invalid', 'InitOnly' ]

var specialMethodHandle = HandleSpecialTests(mparams, method);
var newItem = methodSelector(mparams, specialMethodHandle);

if (newItem != null)
{
var mparams = methodName.Split('_');
var specialMethodHandle = HandleSpecialTests(mparams, method);
var newItem = methodSelector(mparams, specialMethodHandle);

if (newItem != null)
{
newItem.TestName = mparams[0];
newItem.MethodName = methodName;
newItem.ModuleName = testDllName;
newItem.TestName = mparams[0];
newItem.MethodName = methodName;
newItem.ModuleName = testDllName;

retVal.Add(newItem);
}
retVal.Add(newItem);
}
}
}
Expand Down

0 comments on commit b7eb828

Please sign in to comment.