Skip to content

Handle operations with ByRef-like types gracefully in PowerShell #7533

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

Merged
daxian-dbw merged 4 commits into from
Aug 26, 2018
Merged

Handle operations with ByRef-like types gracefully in PowerShell #7533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0eb7f56
[Feature] Handle ByRef-like types gracefully in PowerShell
daxian-dbw Aug 15, 2018
0ee369a
[Feature] Add more tests
daxian-dbw Aug 15, 2018
427392b
[Feature] Minor fix
daxian-dbw Aug 15, 2018
7d3b300
[Feature] Handle ByRef-like type in PSGetIndexerBinder, also update e…
daxian-dbw Aug 23, 2018
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
1 change: 1 addition & 0 deletions src/Microsoft.PowerShell.Commands.Utility/commands/utility/AddType.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -947,6 +947,7 @@ private void SourceCodeProcessing()
}
else
{
parseOptions = CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.Latest);
Copy link
Collaborator

@iSazonov iSazonov Aug 16, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Should we document the change?

Copy link
Member Author

@daxian-dbw daxian-dbw Aug 21, 2018

Choose a reason for hiding this comment

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

Added the document label.

compilationOptions = GetDefaultCompilationOptions();
}

Expand Down
31 changes: 23 additions & 8 deletions src/Microsoft.PowerShell.Commands.Utility/commands/utility/new-object.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,27 +143,42 @@ protected override void BeginProcessing()
if (e.InnerException != null && e.InnerException is TypeResolver.AmbiguousTypeException)
{
ThrowTerminatingError(
new ErrorRecord(
e,
"AmbiguousTypeReference",
ErrorCategory.InvalidType, null));
new ErrorRecord(
e,
"AmbiguousTypeReference",
ErrorCategory.InvalidType,
targetObject: null));
}

mshArgE = PSTraceSource.NewArgumentException(
"TypeName",
NewObjectStrings.TypeNotFound,
TypeName);
"TypeName",
NewObjectStrings.TypeNotFound,
TypeName);

ThrowTerminatingError(
new ErrorRecord(
mshArgE,
"TypeNotFound",
ErrorCategory.InvalidType, null));
ErrorCategory.InvalidType,
targetObject: null));
}
throw e;
}

Diagnostics.Assert(type != null, "LanguagePrimitives.TryConvertTo failed but returned true");

if (type.IsByRefLike)
{
ThrowTerminatingError(
new ErrorRecord(
PSTraceSource.NewInvalidOperationException(
NewObjectStrings.CannotInstantiateBoxedByRefLikeType,
type),
nameof(NewObjectStrings.CannotInstantiateBoxedByRefLikeType),
ErrorCategory.InvalidOperation,
targetObject: null));
}

if (Context.LanguageMode == PSLanguageMode.ConstrainedLanguage)
{
if (!CoreTypes.Contains(type))
Expand Down
3 changes: 3 additions & 0 deletions src/Microsoft.PowerShell.Commands.Utility/resources/NewObjectStrings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@
<data name="CannotInstantiateWinRTType" xml:space="preserve">
<value>Creating instances of attribute and delegated Windows RT types is not supported.</value>
</data>
<data name="CannotInstantiateBoxedByRefLikeType" xml:space="preserve">
<value>Cannot create instances of the ByRef-like type "{0}". ByRef-like types are not supported in PowerShell.</value>
</data>
<data name="CannotCreateTypeConstrainedLanguage" xml:space="preserve">
<value>Cannot create type. Only core types are supported in this language mode.</value>
</data>
Expand Down
88 changes: 70 additions & 18 deletions src/System.Management.Automation/engine/CoreAdapter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1920,6 +1920,38 @@ internal MethodInformation(bool hasvarargs, bool hasoptional, ParameterInformati

internal object Invoke(object target, object[] arguments)
{
// There may be parameters of ByRef-like types, but they will be taken care of
// when we resolve overloads to find the best methods -- proper exception will
// be thrown when converting arguments to the ByRef-like parameter types.
//
// So when reaching here, we only care about (1) if the method return type is
// BeRef-like; (2) if it's a constrcutor of a ByRef-like type.

if (method is ConstructorInfo ctor)
{
if (ctor.DeclaringType.IsByRefLike)
{
throw new MethodException(
nameof(ExtendedTypeSystem.CannotInstantiateBoxedByRefLikeType),
innerException: null,
ExtendedTypeSystem.CannotInstantiateBoxedByRefLikeType,
ctor.DeclaringType);
}

return ctor.Invoke(arguments);
}

var methodInfo = (MethodInfo) method;
if (methodInfo.ReturnType.IsByRefLike)
{
throw new MethodException(
nameof(ExtendedTypeSystem.CannotCallMethodWithByRefLikeReturnType),
innerException: null,
ExtendedTypeSystem.CannotCallMethodWithByRefLikeReturnType,
methodInfo.Name,
methodInfo.ReturnType);
}

if (target is PSObject)
{
if (!method.DeclaringType.IsAssignableFrom(target.GetType()))
Expand All @@ -1932,20 +1964,15 @@ internal object Invoke(object target, object[] arguments)
{
if (_methodInvoker == null)
{
if (!(method is MethodInfo))
{
_useReflection = true;
}
else
{
_methodInvoker = GetMethodInvoker((MethodInfo)method);
}
_methodInvoker = GetMethodInvoker(methodInfo);
}

if (_methodInvoker != null)
{
return _methodInvoker(target, arguments);
}
}

return method.Invoke(target, arguments);
}

Expand Down Expand Up @@ -2455,13 +2482,12 @@ internal PropertyCacheEntry(PropertyInfo property)
// require different delegates
// The same is true for generics, COM Types.
Type declaringType = property.DeclaringType;
Type propertyType = property.PropertyType;

if (declaringType.IsValueType ||
propertyType.IsGenericType ||
declaringType.IsGenericType ||
property.DeclaringType.IsCOMObject ||
property.PropertyType.IsCOMObject)
declaringType.IsCOMObject ||
propertyType.IsCOMObject)
{
this.readOnly = property.GetSetMethod() == null;
this.writeOnly = property.GetGetMethod() == null;
Expand Down Expand Up @@ -2511,7 +2537,9 @@ internal PropertyCacheEntry(FieldInfo field)
private void InitGetter()
{
if (writeOnly || useReflection)
{
return;
}

var parameter = Expression.Parameter(typeof(object));
Expression instance = null;
Expand Down Expand Up @@ -2568,7 +2596,9 @@ private void InitGetter()
private void InitSetter()
{
if (readOnly || useReflection)
{
return;
}

var parameter = Expression.Parameter(typeof(object));
var value = Expression.Parameter(typeof(object));
Expand Down Expand Up @@ -3621,16 +3651,29 @@ protected override string PropertyToString(PSProperty property)
protected override object PropertyGet(PSProperty property)
{
PropertyCacheEntry adapterData = (PropertyCacheEntry)property.adapterData;

if (adapterData.propertyType.IsByRefLike)
{
throw new GetValueException(
nameof(ExtendedTypeSystem.CannotAccessByRefLikePropertyOrField),
innerException: null,
ExtendedTypeSystem.CannotAccessByRefLikePropertyOrField,
adapterData.member.Name,
adapterData.propertyType);
}

PropertyInfo propertyInfo = adapterData.member as PropertyInfo;
if (propertyInfo != null)
{
if (adapterData.writeOnly)
{
throw new GetValueException("WriteOnlyProperty",
null,
throw new GetValueException(
nameof(ExtendedTypeSystem.WriteOnlyProperty),
innerException: null,
ExtendedTypeSystem.WriteOnlyProperty,
propertyInfo.Name);
}

if (adapterData.useReflection)
{
return propertyInfo.GetValue(property.baseObject, null);
Expand Down Expand Up @@ -3664,12 +3707,23 @@ protected override void PropertySet(PSProperty property, object setValue, bool c

if (adapterData.readOnly)
{
throw new SetValueException(nameof(ExtendedTypeSystem.ReadOnlyProperty),
null,
throw new SetValueException(
nameof(ExtendedTypeSystem.ReadOnlyProperty),
innerException: null,
ExtendedTypeSystem.ReadOnlyProperty,
adapterData.member.Name);
}

if (adapterData.propertyType.IsByRefLike)
{
throw new SetValueException(
nameof(ExtendedTypeSystem.CannotAccessByRefLikePropertyOrField),
innerException: null,
ExtendedTypeSystem.CannotAccessByRefLikePropertyOrField,
adapterData.member.Name,
adapterData.propertyType);
}

PropertyInfo propertyInfo = adapterData.member as PropertyInfo;
if (propertyInfo != null)
{
Expand Down Expand Up @@ -3755,9 +3809,7 @@ internal static object AuxiliaryConstructorInvoke(MethodInformation methodInform
#pragma warning disable 56500
try
{
// We cannot call MethodBase's Invoke on a constructor
// because it requires a target we don't have.
returnValue = ((ConstructorInfo)methodInformation.method).Invoke(arguments);
returnValue = methodInformation.Invoke(target: null, arguments);
}
catch (TargetInvocationException ex)
{
Expand Down
27 changes: 22 additions & 5 deletions src/System.Management.Automation/engine/LanguagePrimitives.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4243,8 +4243,7 @@ public object Invoke(object valueToConvert, Type resultType, bool recurse, PSObj
}
}

private static Dictionary<ConversionTypePair, ConversionData> s_converterCache
= new Dictionary<ConversionTypePair, ConversionData>(256);
private static Dictionary<ConversionTypePair, ConversionData> s_converterCache = new Dictionary<ConversionTypePair, ConversionData>(256);

private static ConversionData CacheConversion<T>(Type fromType, Type toType, PSConverter<T> converter, ConversionRank rank)
{
Expand Down Expand Up @@ -4687,9 +4686,13 @@ internal static object ConvertTo(object valueToConvert,
bool debase;
var conversion = FigureConversion(valueToConvert, resultType, out debase);

return conversion.Invoke(debase ? PSObject.Base(valueToConvert) : valueToConvert,
resultType, recursion, debase ? (PSObject)valueToConvert : null,
formatProvider, backupTypeTable);
return conversion.Invoke(
debase ? PSObject.Base(valueToConvert) : valueToConvert,
resultType,
recursion,
debase ? (PSObject)valueToConvert : null,
formatProvider,
backupTypeTable);
}
}

Expand All @@ -4704,6 +4707,14 @@ internal static object ConvertTo(object valueToConvert,
internal static Tuple<string, string> GetInvalidCastMessages(object valueToConvert, Type resultType)
{
string errorId, errorMsg;
if (resultType.IsByRefLike)
{
typeConversion.WriteLine("Cannot convert to ByRef-Like types as they should be used on stack only.");
errorId = nameof(ExtendedTypeSystem.InvalidCastToByRefLikeType);
errorMsg = StringUtil.Format(ExtendedTypeSystem.InvalidCastToByRefLikeType, resultType);
return Tuple.Create(errorId, errorMsg);
}

if (PSObject.Base(valueToConvert) == null)
{
if (resultType.IsEnum)
Expand Down Expand Up @@ -5443,6 +5454,12 @@ internal static ConversionData FigureConversion(Type fromType, Type toType)
toType == fromType ? ConversionRank.Identity : ConversionRank.Assignable);
}

if (fromType.IsByRefLike || toType.IsByRefLike)
{
// ByRef-like types are not boxable and should be used on stack only.
return CacheConversion(fromType, toType, ConvertNoConversion, ConversionRank.None);
}

if (typeof(PSObject).IsAssignableFrom(fromType) && typeof(InternalPSObject) != fromType)
{
// We don't attempt converting PSObject (or derived) to anything else,
Expand Down
22 changes: 21 additions & 1 deletion src/System.Management.Automation/engine/parser/Compiler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,8 @@ internal static class CachedReflectionInfo

internal static readonly MethodInfo PSCreateInstanceBinder_IsTargetTypeNonPublic =
typeof(PSCreateInstanceBinder).GetMethod(nameof(PSCreateInstanceBinder.IsTargetTypeNonPublic), staticFlags);
internal static readonly MethodInfo PSCreateInstanceBinder_IsTargetTypeByRefLike =
typeof(PSCreateInstanceBinder).GetMethod(nameof(PSCreateInstanceBinder.IsTargetTypeByRefLike), staticFlags);
internal static readonly MethodInfo PSCreateInstanceBinder_GetTargetTypeName =
typeof(PSCreateInstanceBinder).GetMethod(nameof(PSCreateInstanceBinder.GetTargetTypeName), staticFlags);

Expand Down Expand Up @@ -596,7 +598,7 @@ internal static Expression Convert(this Expression expr, Type type)
return Expression.Convert(expr, type);
}

if (type.ContainsGenericParameters)
if (type.ContainsGenericParameters || type.IsByRefLike)
{
return Expression.Call(
CachedReflectionInfo.LanguagePrimitives_ThrowInvalidCastException,
Expand Down Expand Up @@ -5504,6 +5506,22 @@ public object VisitMemberExpression(MemberExpressionAst memberExpressionAst)
var propertyInfo = memberInfo[0] as PropertyInfo;
if (propertyInfo != null)
{
if (propertyInfo.PropertyType.IsByRefLike)
{
// ByRef-like types are not boxable and should be used only on stack.
return Expression.Throw(
Expression.New(
CachedReflectionInfo.GetValueException_ctor,
Expression.Constant(nameof(ExtendedTypeSystem.CannotAccessByRefLikePropertyOrField)),
Expression.Constant(null, typeof(Exception)),
Expression.Constant(ExtendedTypeSystem.CannotAccessByRefLikePropertyOrField),
Expression.NewArrayInit(
typeof(object),
Expression.Constant(propertyInfo.Name),
Expression.Constant(propertyInfo.PropertyType, typeof(Type)))),
typeof(object));
}

if (propertyInfo.CanRead)
{
return Expression.Property(null, propertyInfo);
Expand All @@ -5512,6 +5530,8 @@ public object VisitMemberExpression(MemberExpressionAst memberExpressionAst)
}
else
{
// Field cannot be of a ByRef-like type unless it's an instance member of a ref struct.
// So we don't need to check 'IsByRefLike' for static field access.
return Expression.Field(null, (FieldInfo)memberInfo[0]);
}
}
Expand Down
Loading