Skip to content

Commit

Permalink
Align parameter default value resolution between coreCLR, Mono and Na…
Browse files Browse the repository at this point in the history 
…tiveAOT runtimes (#75612)

Fixes 
#49806
#76174

Co-authored-by: Jan Kotas <[email protected]>
  • Loading branch information
@ivanpovazan @jkotas
ivanpovazan and jkotas committed Oct 3, 2022
1 parent a299124 commit 05b8bab
Show file tree
Hide file tree
Showing 5 changed files with 540 additions and 164 deletions.
163 changes: 56 additions & 107 deletions src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeParameterInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,40 @@ public override bool HasDefaultValue
return defaultValue;
}

private object? GetDefaultValueFromCustomAttributeData()
{
foreach (CustomAttributeData attributeData in RuntimeCustomAttributeData.GetCustomAttributes(this))
{
Type attributeType = attributeData.AttributeType;
if (attributeType == typeof(DecimalConstantAttribute))
{
return GetRawDecimalConstant(attributeData);
}
else if (attributeType.IsSubclassOf(typeof(CustomConstantAttribute)))
{
if (attributeType == typeof(DateTimeConstantAttribute))
{
return GetRawDateTimeConstant(attributeData);
}
return GetRawConstant(attributeData);
}
}
return DBNull.Value;
}

private object? GetDefaultValueFromCustomAttributes()
{
object[] customAttributes = GetCustomAttributes(typeof(CustomConstantAttribute), false);
if (customAttributes.Length != 0)
return ((CustomConstantAttribute)customAttributes[0]).Value;

customAttributes = GetCustomAttributes(typeof(DecimalConstantAttribute), false);
if (customAttributes.Length != 0)
return ((DecimalConstantAttribute)customAttributes[0]).Value;

return DBNull.Value;
}

// returns DBNull.Value if the parameter doesn't have a default value
private object? GetDefaultValueInternal(bool raw)
{
Expand All @@ -303,32 +337,7 @@ public override bool HasDefaultValue

object? defaultValue = null;

// Why check the parameter type only for DateTime and only for the ctor arguments?
// No check on the parameter type is done for named args and for Decimal.

// We should move this after MdToken.IsNullToken(m_tkParamDef) and combine it
// with the other custom attribute logic. But will that be a breaking change?
// For a DateTime parameter on which both an md constant and a ca constant are set,
// which one should win?
if (ParameterType == typeof(DateTime))
{
if (raw)
{
CustomAttributeTypedArgument value =
RuntimeCustomAttributeData.Filter(
RuntimeCustomAttributeData.GetCustomAttributes(this), typeof(DateTimeConstantAttribute), 0);

if (value.ArgumentType != null)
return new DateTime((long)value.Value!);
}
else
{
object[] dt = GetCustomAttributes(typeof(DateTimeConstantAttribute), false);
if (dt != null && dt.Length != 0)
return ((DateTimeConstantAttribute)dt[0]).Value;
}
}

// Prioritize metadata constant over custom attribute constant
#region Look for a default value in metadata
if (!MdToken.IsNullToken(m_tkParamDef))
{
Expand All @@ -337,46 +346,17 @@ public override bool HasDefaultValue
}
#endregion

// If default value is not specified in metadata, look for it in custom attributes
if (defaultValue == DBNull.Value)
{
#region Look for a default value in the custom attributes
if (raw)
{
foreach (CustomAttributeData attr in CustomAttributeData.GetCustomAttributes(this))
{
Type? attrType = attr.Constructor.DeclaringType;

if (attrType == typeof(DateTimeConstantAttribute))
{
defaultValue = GetRawDateTimeConstant(attr);
}
else if (attrType == typeof(DecimalConstantAttribute))
{
defaultValue = GetRawDecimalConstant(attr);
}
else if (attrType!.IsSubclassOf(typeof(CustomConstantAttribute)))
{
defaultValue = GetRawConstant(attr);
}
}
}
else
{
object[] CustomAttrs = GetCustomAttributes(typeof(CustomConstantAttribute), false);
if (CustomAttrs.Length != 0)
{
defaultValue = ((CustomConstantAttribute)CustomAttrs[0]).Value;
}
else
{
CustomAttrs = GetCustomAttributes(typeof(DecimalConstantAttribute), false);
if (CustomAttrs.Length != 0)
{
defaultValue = ((DecimalConstantAttribute)CustomAttrs[0]).Value;
}
}
}
#endregion
// The resolution of default value is done by following these rules:
// 1. For RawDefaultValue, we pick the first custom attribute holding the constant value
// in the following order: DecimalConstantAttribute, DateTimeConstantAttribute, CustomConstantAttribute
// 2. For DefaultValue, we first look for CustomConstantAttribute and pick the first occurrence.
// If none is found, then we repeat the same process searching for DecimalConstantAttribute.
// IMPORTANT: Please note that there is a subtle difference in order custom attributes are inspected for
// RawDefaultValue and DefaultValue.
defaultValue = raw ? GetDefaultValueFromCustomAttributeData() : GetDefaultValueFromCustomAttributes();
}

if (defaultValue == DBNull.Value)
Expand All @@ -388,44 +368,21 @@ public override bool HasDefaultValue
private static decimal GetRawDecimalConstant(CustomAttributeData attr)
{
Debug.Assert(attr.Constructor.DeclaringType == typeof(DecimalConstantAttribute));

foreach (CustomAttributeNamedArgument namedArgument in attr.NamedArguments)
{
if (namedArgument.MemberInfo.Name.Equals("Value"))
{
// This is not possible because Decimal cannot be represented directly in the metadata.
Debug.Fail("Decimal cannot be represented directly in the metadata.");
return (decimal)namedArgument.TypedValue.Value!;
}
}

ParameterInfo[] parameters = attr.Constructor.GetParameters();
Debug.Assert(parameters.Length == 5);

System.Collections.Generic.IList<CustomAttributeTypedArgument> args = attr.ConstructorArguments;
Debug.Assert(args.Count == 5);

if (parameters[2].ParameterType == typeof(uint))
{
// DecimalConstantAttribute(byte scale, byte sign, uint hi, uint mid, uint low)
int low = (int)(uint)args[4].Value!;
int mid = (int)(uint)args[3].Value!;
int hi = (int)(uint)args[2].Value!;
byte sign = (byte)args[1].Value!;
byte scale = (byte)args[0].Value!;

return new decimal(low, mid, hi, sign != 0, scale);
}
else
return new decimal(
lo: GetConstructorArgument(args, 4),
mid: GetConstructorArgument(args, 3),
hi: GetConstructorArgument(args, 2),
isNegative: ((byte)args[1].Value!) != 0,
scale: (byte)args[0].Value!);

static int GetConstructorArgument(IList<CustomAttributeTypedArgument> args, int index)
{
// DecimalConstantAttribute(byte scale, byte sign, int hi, int mid, int low)
int low = (int)args[4].Value!;
int mid = (int)args[3].Value!;
int hi = (int)args[2].Value!;
byte sign = (byte)args[1].Value!;
byte scale = (byte)args[0].Value!;

return new decimal(low, mid, hi, sign != 0, scale);
// The constructor is overloaded to accept both signed and unsigned arguments
object obj = args[index].Value!;
return (obj is int value) ? value : (int)(uint)obj;
}
}

Expand All @@ -434,20 +391,12 @@ private static DateTime GetRawDateTimeConstant(CustomAttributeData attr)
Debug.Assert(attr.Constructor.DeclaringType == typeof(DateTimeConstantAttribute));
Debug.Assert(attr.ConstructorArguments.Count == 1);

foreach (CustomAttributeNamedArgument namedArgument in attr.NamedArguments)
{
if (namedArgument.MemberInfo.Name.Equals("Value"))
{
return new DateTime((long)namedArgument.TypedValue.Value!);
}
}

// Look at the ctor argument if the "Value" property was not explicitly defined.
return new DateTime((long)attr.ConstructorArguments[0].Value!);
}

private static object? GetRawConstant(CustomAttributeData attr)
{
// We are relying only on named arguments for historical reasons
foreach (CustomAttributeNamedArgument namedArgument in attr.NamedArguments)
{
if (namedArgument.MemberInfo.Name.Equals("Value"))
Expand Down
126 changes: 96 additions & 30 deletions ...coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/General/Helpers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,46 +203,112 @@ public static object[] InstantiateAsArray(this IEnumerable<CustomAttributeData>
return result;
}

public static bool GetCustomAttributeDefaultValueIfAny(IEnumerable<CustomAttributeData> customAttributes, bool raw, out object? defaultValue)
private static object? GetRawDefaultValue(IEnumerable<CustomAttributeData> customAttributes)
{
// Legacy: If there are multiple default value attribute, the desktop picks one at random (and so do we...)
foreach (CustomAttributeData cad in customAttributes)
foreach (CustomAttributeData attributeData in customAttributes)
{
Type attributeType = cad.AttributeType;
if (attributeType.IsSubclassOf(typeof(CustomConstantAttribute)))
Type attributeType = attributeData.AttributeType;
if (attributeType == typeof(DecimalConstantAttribute))
{
if (raw)
{
foreach (CustomAttributeNamedArgument namedArgument in cad.NamedArguments)
{
if (namedArgument.MemberName.Equals("Value"))
{
defaultValue = namedArgument.TypedValue.Value;
return true;
}
}
defaultValue = null;
return false;
}
else
return GetRawDecimalConstant(attributeData);
}
else if (attributeType.IsSubclassOf(typeof(CustomConstantAttribute)))
{
if (attributeType == typeof(DateTimeConstantAttribute))
{
CustomConstantAttribute customConstantAttribute = (CustomConstantAttribute)(cad.Instantiate());
defaultValue = customConstantAttribute.Value;
return true;
return GetRawDateTimeConstant(attributeData);
}
return GetRawConstant(attributeData);
}
if (attributeType.Equals(typeof(DecimalConstantAttribute)))
}
return DBNull.Value;
}

private static decimal GetRawDecimalConstant(CustomAttributeData attr)
{
System.Collections.Generic.IList<CustomAttributeTypedArgument> args = attr.ConstructorArguments;

return new decimal(
lo: GetConstructorArgument(args, 4),
mid: GetConstructorArgument(args, 3),
hi: GetConstructorArgument(args, 2),
isNegative: ((byte)args[1].Value!) != 0,
scale: (byte)args[0].Value!);

static int GetConstructorArgument(IList<CustomAttributeTypedArgument> args, int index)
{
// The constructor is overloaded to accept both signed and unsigned arguments
object obj = args[index].Value!;
return (obj is int value) ? value : (int)(uint)obj;
}
}

private static DateTime GetRawDateTimeConstant(CustomAttributeData attr)
{
return new DateTime((long)attr.ConstructorArguments[0].Value!);
}

// We are relying only on named arguments for historical reasons
private static object? GetRawConstant(CustomAttributeData attr)
{
foreach (CustomAttributeNamedArgument namedArgument in attr.NamedArguments)
{
if (namedArgument.MemberInfo.Name.Equals("Value"))
return namedArgument.TypedValue.Value;
}
return DBNull.Value;
}

private static object? GetDefaultValue(IEnumerable<CustomAttributeData> customAttributes)
{
// we first look for a CustomConstantAttribute, but we will save the first occurrence of DecimalConstantAttribute
// so we don't go through all custom attributes again
CustomAttributeData? firstDecimalConstantAttributeData = null;
foreach (CustomAttributeData attributeData in customAttributes)
{
Type attributeType = attributeData.AttributeType;
if (firstDecimalConstantAttributeData == null && attributeType == typeof(DecimalConstantAttribute))
{
firstDecimalConstantAttributeData = attributeData;
}
else if (attributeType.IsSubclassOf(typeof(CustomConstantAttribute)))
{
// We should really do a non-instanting check if "raw == false" but given that we don't support
// reflection-only loads, there isn't an observable difference.
DecimalConstantAttribute decimalConstantAttribute = (DecimalConstantAttribute)(cad.Instantiate());
defaultValue = decimalConstantAttribute.Value;
return true;
CustomConstantAttribute customConstantAttribute = (CustomConstantAttribute)(attributeData.Instantiate());
return customConstantAttribute.Value;
}
}

defaultValue = null;
return false;
if (firstDecimalConstantAttributeData != null)
{
DecimalConstantAttribute decimalConstantAttribute = (DecimalConstantAttribute)(firstDecimalConstantAttributeData.Instantiate());
return decimalConstantAttribute.Value;
}
else
{
return DBNull.Value;
}
}

public static bool GetCustomAttributeDefaultValueIfAny(IEnumerable<CustomAttributeData> customAttributes, bool raw, out object? defaultValue)
{
// The resolution of default value is done by following these rules:
// 1. For RawDefaultValue, we pick the first custom attribute holding the constant value
// in the following order: DecimalConstantAttribute, DateTimeConstantAttribute, CustomConstantAttribute
// 2. For DefaultValue, we first look for CustomConstantAttribute and pick the first occurrence.
// If none is found, then we repeat the same process searching for DecimalConstantAttribute.
// IMPORTANT: Please note that there is a subtle difference in order custom attributes are inspected for
// RawDefaultValue and DefaultValue.
object? resolvedValue = raw ? GetRawDefaultValue(customAttributes) : GetDefaultValue(customAttributes);
if (resolvedValue != DBNull.Value)
{
defaultValue = resolvedValue;
return true;
}
else
{
defaultValue = null;
return false;
}
}
}
}

0 comments on commit 05b8bab

Please sign in to comment.