Skip to content

[release/v7.4] Move .NET method invocation logging to after the needed type conversion is done for method arguments #25568

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
TravisEz13 merged 2 commits into from
Jun 4, 2025
Merged

[release/v7.4] Move .NET method invocation logging to after the needed type conversion is done for method arguments #25568

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
64d78da
Cherry-pick PR #25022 with conflicts for manual resolution
daxian-dbw Mar 18, 2025
6d968ba
Resolve conflicts
daxian-dbw Jun 3, 2025
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
107 changes: 86 additions & 21 deletions src/System.Management.Automation/engine/runtime/Binding/Binders.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6935,20 +6935,6 @@ internal static DynamicMetaObject InvokeDotNetMethod(
expr = Expression.Block(expr, ExpressionCache.AutomationNullConstant);
}

// Expression block runs two expressions in order:
// - Log method invocation to AMSI Notifications (can throw PSSecurityException)
// - Invoke method
string targetName = methodInfo.ReflectedType?.FullName ?? string.Empty;
expr = Expression.Block(
Expression.Call(
CachedReflectionInfo.MemberInvocationLoggingOps_LogMemberInvocation,
Expression.Constant(targetName),
Expression.Constant(name),
Expression.NewArrayInit(
typeof(object),
args.Select(static e => e.Expression.Cast(typeof(object))))),
expr);

// If we're calling SteppablePipeline.{Begin|Process|End}, we don't want
// to wrap exceptions - this is very much a special case to help error
// propagation and ensure errors are attributed to the correct code (the
Expand Down Expand Up @@ -7111,6 +7097,7 @@ internal static Expression InvokeMethod(MethodBase mi, DynamicMetaObject target,
invocationType != MethodInvocationType.NonVirtual;
var parameters = mi.GetParameters();
var argExprs = new Expression[parameters.Length];
var argsToLog = new List<Expression>(Math.Max(parameters.Length, args.Length));

for (int i = 0; i < parameters.Length; ++i)
{
Expand All @@ -7135,16 +7122,21 @@ internal static Expression InvokeMethod(MethodBase mi, DynamicMetaObject target,

if (expandParameters)
{
argExprs[i] = Expression.NewArrayInit(
paramElementType,
args.Skip(i).Select(
a => a.CastOrConvertMethodArgument(
IEnumerable<Expression> elements = args
.Skip(i)
.Select(a =>
a.CastOrConvertMethodArgument(
paramElementType,
paramName,
mi.Name,
allowCastingToByRefLikeType: false,
temps,
initTemps)));
initTemps))
.ToList();

argExprs[i] = Expression.NewArrayInit(paramElementType, elements);
// User specified the element arguments, so we log them instead of the compiler-created array.
argsToLog.AddRange(elements);
}
else
{
Expand All @@ -7155,13 +7147,18 @@ internal static Expression InvokeMethod(MethodBase mi, DynamicMetaObject target,
allowCastingToByRefLikeType: false,
temps,
initTemps);

argExprs[i] = arg;
argsToLog.Add(arg);
}
}
else if (i >= args.Length)
{
Diagnostics.Assert(parameters[i].IsOptional,
// We don't log the default value for an optional parameter, as it's not specified by the user.
Diagnostics.Assert(
parameters[i].IsOptional,
"if there are too few arguments, FindBestMethod should only succeed if parameters are optional");

var argValue = parameters[i].DefaultValue;
if (argValue == null)
{
Expand Down Expand Up @@ -7192,17 +7189,25 @@ internal static Expression InvokeMethod(MethodBase mi, DynamicMetaObject target,
var psRefValue = Expression.Property(args[i].Expression.Cast(typeof(PSReference)), CachedReflectionInfo.PSReference_Value);
initTemps.Add(Expression.Assign(temp, psRefValue.Convert(temp.Type)));
copyOutTemps.Add(Expression.Assign(psRefValue, temp.Cast(typeof(object))));

argExprs[i] = temp;
argsToLog.Add(temp);
}
else
{
argExprs[i] = args[i].CastOrConvertMethodArgument(
var convertedArg = args[i].CastOrConvertMethodArgument(
parameterType,
paramName,
mi.Name,
allowCastingToByRefLikeType,
temps,
initTemps);

argExprs[i] = convertedArg;
// If the converted arg is a byref-like type, then we log the original arg.
argsToLog.Add(convertedArg.Type.IsByRefLike
? args[i].Expression
: convertedArg);
}
}
}
Expand Down Expand Up @@ -7248,6 +7253,12 @@ internal static Expression InvokeMethod(MethodBase mi, DynamicMetaObject target,
}
}

// We need to add one expression to log the .NET invocation before actually invoking:
// - Log method invocation to AMSI Notifications (can throw PSSecurityException)
// - Invoke method
string targetName = mi.ReflectedType?.FullName ?? string.Empty;
string methodName = mi.Name is ".ctor" ? "new" : mi.Name;

if (temps.Count > 0)
{
if (call.Type != typeof(void) && copyOutTemps.Count > 0)
Expand All @@ -7258,8 +7269,13 @@ internal static Expression InvokeMethod(MethodBase mi, DynamicMetaObject target,
copyOutTemps.Add(retValue);
}

AddMemberInvocationLogging(initTemps, targetName, methodName, argsToLog);
call = Expression.Block(call.Type, temps, initTemps.Append(call).Concat(copyOutTemps));
}
else
{
call = AddMemberInvocationLogging(call, targetName, methodName, argsToLog);
}

return call;
}
Expand Down Expand Up @@ -7551,6 +7567,55 @@ internal static void InvalidateCache()
}
}

#nullable enable
private static Expression AddMemberInvocationLogging(
Expression expr,
string targetName,
string name,
List<Expression> args)
{
#if UNIX
// For efficiency this is a no-op on non-Windows platforms.
return expr;
#else
Expression[] invocationArgs = new Expression[args.Count];
for (int i = 0; i < args.Count; i++)
{
invocationArgs[i] = args[i].Cast(typeof(object));
}

return Expression.Block(
Expression.Call(
CachedReflectionInfo.MemberInvocationLoggingOps_LogMemberInvocation,
Expression.Constant(targetName),
Expression.Constant(name),
Expression.NewArrayInit(typeof(object), invocationArgs)),
expr);
#endif
}

private static void AddMemberInvocationLogging(
List<Expression> exprs,
string targetName,
string name,
List<Expression> args)
{
#if !UNIX
Expression[] invocationArgs = new Expression[args.Count];
for (int i = 0; i < args.Count; i++)
{
invocationArgs[i] = args[i].Cast(typeof(object));
}

exprs.Add(Expression.Call(
CachedReflectionInfo.MemberInvocationLoggingOps_LogMemberInvocation,
Expression.Constant(targetName),
Expression.Constant(name),
Expression.NewArrayInit(typeof(object), invocationArgs)));
#endif
}
#nullable disable

#endregion
}

Expand Down
Loading