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.

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

Jump to bottom

Add tests for UnsafeAccessor on fields on generic types #92657

Merged
merged 2 commits into from
Sep 27, 2023
Merged

Add tests for UnsafeAccessor on fields on generic types #92657

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0cb1d26
Add tests for field access on generics
AaronRobinsonMSFT Sep 26, 2023
cb81128
Fix ILC to compile UnsafeAccessorsTests
AaronRobinsonMSFT Sep 26, 2023
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
18 changes: 8 additions & 10 deletions src/coreclr/tools/Common/TypeSystem/IL/UnsafeAccessors.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,12 @@ public static MethodIL TryGetIL(EcmaMethod method)
return GenerateAccessorBadImageFailure(method);
}

const string ctorName = ".ctor";
context.TargetType = ValidateTargetType(retType);
if (context.TargetType == null)
if (!ValidateTargetType(retType, out context.TargetType))
{
return GenerateAccessorBadImageFailure(method);
}

const string ctorName = ".ctor";
if (!TrySetTargetMethod(ref context, ctorName, out isAmbiguous))
{
return GenerateAccessorSpecificFailure(ref context, ctorName, isAmbiguous);
Expand All @@ -100,8 +99,7 @@ public static MethodIL TryGetIL(EcmaMethod method)
return GenerateAccessorBadImageFailure(method);
}

context.TargetType = ValidateTargetType(firstArgType);
if (context.TargetType == null)
if (!ValidateTargetType(firstArgType, out context.TargetType))
{
return GenerateAccessorBadImageFailure(method);
}
Expand Down Expand Up @@ -132,8 +130,7 @@ public static MethodIL TryGetIL(EcmaMethod method)
return GenerateAccessorBadImageFailure(method);
}

context.TargetType = ValidateTargetType(firstArgType);
if (context.TargetType == null)
if (!ValidateTargetType(firstArgType, out context.TargetType))
{
return GenerateAccessorBadImageFailure(method);
}
Expand Down Expand Up @@ -221,7 +218,7 @@ private struct GenerationContext
public FieldDesc TargetField;
}

private static TypeDesc ValidateTargetType(TypeDesc targetTypeMaybe)
private static bool ValidateTargetType(TypeDesc targetTypeMaybe, out TypeDesc validated)
{
TypeDesc targetType = targetTypeMaybe.IsByRef
? ((ParameterizedType)targetTypeMaybe).ParameterType
Expand All @@ -232,10 +229,11 @@ private static TypeDesc ValidateTargetType(TypeDesc targetTypeMaybe)
if ((targetType.IsParameterizedType && !targetType.IsArray)
|| targetType.IsFunctionPointer)
{
ThrowHelper.ThrowBadImageFormatException();
targetType = null;
}

return targetType;
validated = targetType;
return validated != null;
}

private static bool DoesMethodMatchUnsafeAccessorDeclaration(ref GenerationContext context, MethodDesc method, bool ignoreCustomModifiers)
Expand Down
63 changes: 62 additions & 1 deletion src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ private static void _MVV() {}
private void _mvv() {}

// The "init" is important to have here - custom modifier test.
// The signature of set_Prop is
// The signature of set_Prop is
// instance void modreq([System.Runtime]System.Runtime.CompilerServices.IsExternalInit) set_Prop ( string 'value')
private string Prop { get; init; }

Expand Down Expand Up @@ -85,6 +85,33 @@ struct UserDataValue
public string GetFieldValue() => _f;
}

class UserDataGenericClass<T>
{
public const string StaticGenericFieldName = nameof(_GF);
public const string GenericFieldName = nameof(_gf);
public const string StaticGenericMethodName = nameof(_GM);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you plan to add method tests to this PR too so that these strings and methods are used?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These will be used when we enable more, not in this PR. I didn't use them right here as there is no need, but they represent the pattern. I didn't see any reason to not include them because that is what they will be. I'd prefer to leave them there instead of kicking off the CI again.

public const string GenericMethodName = nameof(_gm);

public const string StaticFieldName = nameof(_F);
public const string FieldName = nameof(_f);
public const string StaticMethodName = nameof(_M);
public const string MethodName = nameof(_m);

private static T _GF;
private T _gf;

private static string _F = PrivateStatic;
private string _f;

public UserDataGenericClass() { _f = Private; }

private static string _GM(T s, ref T sr, in T si) => typeof(T).ToString();
private string _gm(T s, ref T sr, in T si) => typeof(T).ToString();

private static string _M(string s, ref string sr, in string si) => s;
private string _m(string s, ref string sr, in string si) => s;
}

[UnsafeAccessor(UnsafeAccessorKind.Constructor)]
extern static UserDataClass CallPrivateConstructorClass();

Expand Down Expand Up @@ -188,6 +215,23 @@ public static void Verify_AccessFieldClass()
extern static ref string GetPrivateField(UserDataClass d);
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/92633")]
AaronRobinsonMSFT marked this conversation as resolved.
Show resolved Hide resolved
public static void Verify_AccessStaticFieldGenericClass()
{
Console.WriteLine($"Running {nameof(Verify_AccessStaticFieldGenericClass)}");

Assert.Equal(PrivateStatic, GetPrivateStaticFieldInt((UserDataGenericClass<int>)null));

Assert.Equal(PrivateStatic, GetPrivateStaticFieldString((UserDataGenericClass<string>)null));

[UnsafeAccessor(UnsafeAccessorKind.StaticField, Name=UserDataGenericClass<int>.StaticFieldName)]
extern static ref string GetPrivateStaticFieldInt(UserDataGenericClass<int> d);

[UnsafeAccessor(UnsafeAccessorKind.StaticField, Name=UserDataGenericClass<string>.StaticFieldName)]
extern static ref string GetPrivateStaticFieldString(UserDataGenericClass<string> d);
}

[Fact]
public static void Verify_AccessStaticFieldValue()
{
Expand Down Expand Up @@ -215,6 +259,23 @@ public static void Verify_AccessFieldValue()
extern static ref string GetPrivateField(ref UserDataValue d);
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/92633")]
public static void Verify_AccessFieldGenericClass()
{
Console.WriteLine($"Running {nameof(Verify_AccessFieldGenericClass)}");

Assert.Equal(Private, GetPrivateFieldInt(new UserDataGenericClass<int>()));

Assert.Equal(Private, GetPrivateFieldString(new UserDataGenericClass<string>()));

[UnsafeAccessor(UnsafeAccessorKind.Field, Name=UserDataGenericClass<int>.FieldName)]
extern static ref string GetPrivateFieldInt(UserDataGenericClass<int> d);

[UnsafeAccessor(UnsafeAccessorKind.Field, Name=UserDataGenericClass<string>.FieldName)]
extern static ref string GetPrivateFieldString(UserDataGenericClass<string> d);
}

[Fact]
public static void Verify_AccessStaticMethodClass()
{
Expand Down