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

Respect IsDynamicCodeSupported in more places in Linq.Expressions #88539

Merged
merged 4 commits into from
Jul 11, 2023
Merged

Respect IsDynamicCodeSupported in more places in Linq.Expressions #88539

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7f9228e
Respect IsDynamicCodeSupported in more places in Linq.Expressions
eerhardt Mar 3, 2023
4643eae
Work around DynamicDelegateAugments.CreateObjectArrayDelegate not exi…
eerhardt Jul 7, 2023
b57a897
Adjust CompileWorksWhenDynamicCodeNotSupported to test in more scenar…
eerhardt Jul 7, 2023
514bd95
Address PR feedback
eerhardt Jul 11, 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
201 changes: 115 additions & 86 deletions src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/DelegateHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@
using System.Diagnostics.CodeAnalysis;
using System.Reflection;
using System.Reflection.Emit;
using System.Runtime.CompilerServices;
using System.Text;
using System.Threading;

namespace System.Dynamic.Utils
{
internal static class DelegateHelpers
{
// This can be flipped to true using feature switches at publishing time
// This can be flipped to false using feature switches at publishing time
internal static bool CanEmitObjectArrayDelegate => true;
Comment on lines -14 to 16
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to change this into

internal static bool CanEmitObjectArrayDelegate => RuntimeFeature.CanEmitObjectArrayDelegate;

If it remains a constant value it will still cause issues for iOS-like platforms as described in: #87924

My last comment: #87924 (comment) breaks down the benefits of having these conditional variables as feature switches.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that would be a good enhancement to make in a future PR. It isn't necessary to fix the issue this PR is fixing - #81803.


// Separate class so that the it can be trimmed away and doesn't get conflated
Expand All @@ -21,14 +22,23 @@ private static class DynamicDelegateLightup
public static Func<Type, Func<object?[], object?>, Delegate> CreateObjectArrayDelegate { get; }
= CreateObjectArrayDelegateInternal();

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:UnrecognizedReflectionPattern",
Justification = "Works around https://github.com/dotnet/linker/issues/2392")]
private static Func<Type, Func<object?[], object?>, Delegate> CreateObjectArrayDelegateInternal()
=> Type.GetType("Internal.Runtime.Augments.DynamicDelegateAugments")!
.GetMethod("CreateObjectArrayDelegate")!
.CreateDelegate<Func<Type, Func<object?[], object?>, Delegate>>();
}

private static class ForceAllowDynamicCodeLightup
{
public static Func<IDisposable>? ForceAllowDynamicCodeDelegate { get; }
= ForceAllowDynamicCodeDelegateInternal();

private static Func<IDisposable>? ForceAllowDynamicCodeDelegateInternal()
=> typeof(AssemblyBuilder)
.GetMethod("ForceAllowDynamicCode", BindingFlags.NonPublic | BindingFlags.Static)
?.CreateDelegate<Func<IDisposable>>();
}

internal static Delegate CreateObjectArrayDelegate(Type delegateType, Func<object?[], object?> handler)
{
if (CanEmitObjectArrayDelegate)
Expand Down Expand Up @@ -186,113 +196,132 @@ private static Delegate CreateObjectArrayDelegateRefEmit(Type delegateType, Func

if (thunkMethod == null)
{
int thunkIndex = Interlocked.Increment(ref s_ThunksCreated);
Type[] paramTypes = new Type[parameters.Length + 1];
paramTypes[0] = typeof(Func<object[], object>);

StringBuilder thunkName = new StringBuilder();
thunkName.Append("Thunk");
thunkName.Append(thunkIndex);
if (hasReturnValue)
IDisposable? forceAllowDynamicCodeScope = null;
if (!RuntimeFeature.IsDynamicCodeSupported)
{
thunkName.Append("ret_");
thunkName.Append(returnType.Name);
// Force 'new DynamicMethod' to not throw even though RuntimeFeature.IsDynamicCodeSupported is false.
// If we are running on a runtime that supports dynamic code, even though the feature switch is off,
// for example when running on CoreClr with PublishAot=true, this will allow IL to be emitted.
// If we are running on a runtime that really doesn't support dynamic code, like NativeAOT,
// CanEmitObjectArrayDelegate will be flipped to 'false', and this method won't be invoked.
forceAllowDynamicCodeScope = ForceAllowDynamicCodeLightup.ForceAllowDynamicCodeDelegate?.Invoke();
}

for (int i = 0; i < parameters.Length; i++)
try
eerhardt marked this conversation as resolved.
Show resolved Hide resolved
{
thunkName.Append('_');
thunkName.Append(parameters[i].ParameterType.Name);
paramTypes[i + 1] = parameters[i].ParameterType;
}

eerhardt marked this conversation as resolved.
Show resolved Hide resolved
DynamicMethod dynamicThunkMethod = new DynamicMethod(thunkName.ToString(), returnType, paramTypes);
thunkMethod = dynamicThunkMethod;
ILGenerator ilgen = dynamicThunkMethod.GetILGenerator();
int thunkIndex = Interlocked.Increment(ref s_ThunksCreated);
Type[] paramTypes = new Type[parameters.Length + 1];
paramTypes[0] = typeof(Func<object[], object>);

LocalBuilder argArray = ilgen.DeclareLocal(typeof(object[]));
LocalBuilder retValue = ilgen.DeclareLocal(typeof(object));
StringBuilder thunkName = new StringBuilder();
thunkName.Append("Thunk");
thunkName.Append(thunkIndex);
if (hasReturnValue)
{
thunkName.Append("ret_");
thunkName.Append(returnType.Name);
}

// create the argument array
if (parameters.Length == 0)
{
ilgen.Emit(OpCodes.Call, s_ArrayEmpty);
}
else
{
ilgen.Emit(OpCodes.Ldc_I4, parameters.Length);
ilgen.Emit(OpCodes.Newarr, typeof(object));
}
ilgen.Emit(OpCodes.Stloc, argArray);
for (int i = 0; i < parameters.Length; i++)
{
thunkName.Append('_');
thunkName.Append(parameters[i].ParameterType.Name);
paramTypes[i + 1] = parameters[i].ParameterType;
}

// populate object array
bool hasRefArgs = false;
for (int i = 0; i < parameters.Length; i++)
{
bool paramIsByReference = parameters[i].ParameterType.IsByRef;
Type paramType = parameters[i].ParameterType;
if (paramIsByReference)
paramType = paramType.GetElementType()!;
DynamicMethod dynamicThunkMethod = new DynamicMethod(thunkName.ToString(), returnType, paramTypes);
thunkMethod = dynamicThunkMethod;
ILGenerator ilgen = dynamicThunkMethod.GetILGenerator();

hasRefArgs = hasRefArgs || paramIsByReference;
LocalBuilder argArray = ilgen.DeclareLocal(typeof(object[]));
LocalBuilder retValue = ilgen.DeclareLocal(typeof(object));

ilgen.Emit(OpCodes.Ldloc, argArray);
ilgen.Emit(OpCodes.Ldc_I4, i);
ilgen.Emit(OpCodes.Ldarg, i + 1);
// create the argument array
if (parameters.Length == 0)
{
ilgen.Emit(OpCodes.Call, s_ArrayEmpty);
}
else
{
ilgen.Emit(OpCodes.Ldc_I4, parameters.Length);
ilgen.Emit(OpCodes.Newarr, typeof(object));
}
ilgen.Emit(OpCodes.Stloc, argArray);

if (paramIsByReference)
// populate object array
bool hasRefArgs = false;
for (int i = 0; i < parameters.Length; i++)
{
ilgen.Emit(OpCodes.Ldobj, paramType);
bool paramIsByReference = parameters[i].ParameterType.IsByRef;
Type paramType = parameters[i].ParameterType;
if (paramIsByReference)
paramType = paramType.GetElementType()!;

hasRefArgs = hasRefArgs || paramIsByReference;

ilgen.Emit(OpCodes.Ldloc, argArray);
ilgen.Emit(OpCodes.Ldc_I4, i);
ilgen.Emit(OpCodes.Ldarg, i + 1);

if (paramIsByReference)
{
ilgen.Emit(OpCodes.Ldobj, paramType);
}
Type boxType = ConvertToBoxableType(paramType);
ilgen.Emit(OpCodes.Box, boxType);
ilgen.Emit(OpCodes.Stelem_Ref);
}
Type boxType = ConvertToBoxableType(paramType);
ilgen.Emit(OpCodes.Box, boxType);
ilgen.Emit(OpCodes.Stelem_Ref);
}

if (hasRefArgs)
{
ilgen.BeginExceptionBlock();
}
if (hasRefArgs)
{
ilgen.BeginExceptionBlock();
}

// load delegate
ilgen.Emit(OpCodes.Ldarg_0);
// load delegate
ilgen.Emit(OpCodes.Ldarg_0);

// load array
ilgen.Emit(OpCodes.Ldloc, argArray);
// load array
ilgen.Emit(OpCodes.Ldloc, argArray);

// invoke Invoke
ilgen.Emit(OpCodes.Callvirt, s_FuncInvoke);
ilgen.Emit(OpCodes.Stloc, retValue);
// invoke Invoke
ilgen.Emit(OpCodes.Callvirt, s_FuncInvoke);
ilgen.Emit(OpCodes.Stloc, retValue);

if (hasRefArgs)
{
// copy back ref/out args
ilgen.BeginFinallyBlock();
for (int i = 0; i < parameters.Length; i++)
if (hasRefArgs)
{
if (parameters[i].ParameterType.IsByRef)
{
Type byrefToType = parameters[i].ParameterType.GetElementType()!;

// update parameter
ilgen.Emit(OpCodes.Ldarg, i + 1);
ilgen.Emit(OpCodes.Ldloc, argArray);
ilgen.Emit(OpCodes.Ldc_I4, i);
ilgen.Emit(OpCodes.Ldelem_Ref);
ilgen.Emit(OpCodes.Unbox_Any, byrefToType);
ilgen.Emit(OpCodes.Stobj, byrefToType);
// copy back ref/out args
ilgen.BeginFinallyBlock();
for (int i = 0; i < parameters.Length; i++)
{
if (parameters[i].ParameterType.IsByRef)
{
Type byrefToType = parameters[i].ParameterType.GetElementType()!;

// update parameter
ilgen.Emit(OpCodes.Ldarg, i + 1);
ilgen.Emit(OpCodes.Ldloc, argArray);
ilgen.Emit(OpCodes.Ldc_I4, i);
ilgen.Emit(OpCodes.Ldelem_Ref);
ilgen.Emit(OpCodes.Unbox_Any, byrefToType);
ilgen.Emit(OpCodes.Stobj, byrefToType);
}
}
ilgen.EndExceptionBlock();
}
ilgen.EndExceptionBlock();
}

if (hasReturnValue)
if (hasReturnValue)
{
ilgen.Emit(OpCodes.Ldloc, retValue);
ilgen.Emit(OpCodes.Unbox_Any, ConvertToBoxableType(returnType));
}

ilgen.Emit(OpCodes.Ret);
}
finally
{
ilgen.Emit(OpCodes.Ldloc, retValue);
ilgen.Emit(OpCodes.Unbox_Any, ConvertToBoxableType(returnType));
forceAllowDynamicCodeScope?.Dispose();
}

ilgen.Emit(OpCodes.Ret);
}

s_thunks[delegateType] = thunkMethod;
Expand Down
2 changes: 1 addition & 1 deletion ...raries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/CallInstruction.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ internal abstract partial class CallInstruction : Instruction
/// </summary>
public abstract int ArgumentCount { get; }

private static bool CanCreateArbitraryDelegates => true;
private static bool CanCreateArbitraryDelegates => RuntimeFeature.IsDynamicCodeSupported;

#region Construction

Expand Down
52 changes: 44 additions & 8 deletions src/libraries/System.Linq.Expressions/tests/CompilerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Reflection;
using System.Runtime.CompilerServices;
using System.Text.RegularExpressions;
using Microsoft.DotNet.RemoteExecutor;
Expand Down Expand Up @@ -442,16 +443,51 @@ public static void CompileWorksWhenDynamicCodeNotSupported()

using RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke(static () =>
{
ParameterExpression param = Expression.Parameter(typeof(int));
CompileWorksWhenDynamicCodeNotSupportedInner();
}, options);
}

Func<int, int> typedDel =
Expression.Lambda<Func<int, int>>(Expression.Add(param, Expression.Constant(4)), param).Compile();
Assert.Equal(304, typedDel(300));
// run the same test code as the above CompileWorksWhenDynamicCodeNotSupported test
// to ensure this test works correctly on all platforms - even if RemoteExecutor is not supported
[Fact]
public static void CompileWorksWhenDynamicCodeNotSupportedInner()
{
ParameterExpression param = Expression.Parameter(typeof(int));

Delegate del =
Expression.Lambda(Expression.Add(param, Expression.Constant(5)), param).Compile();
Assert.Equal(305, del.DynamicInvoke(300));
}, options);
Func<int, int> typedDel =
Expression.Lambda<Func<int, int>>(Expression.Add(param, Expression.Constant(4)), param).Compile();
Assert.Equal(304, typedDel(300));

Delegate del =
Expression.Lambda(Expression.Add(param, Expression.Constant(5)), param).Compile();
Assert.Equal(305, del.DynamicInvoke(300));

// testing more than 2 parameters is important because because it follows a different code path in Compile.
Expression<Func<int, int, int, int, int, int>> fiveParameterExpression = (a, b, c, d, e) => a + b + c + d + e;
Func<int, int, int, int, int, int> fiveParameterFunc = fiveParameterExpression.Compile();
Assert.Equal(6, fiveParameterFunc(2, 2, 1, 1, 0));

Expression<Func<int, int, int>> callExpression = (a, b) => Add(a, b);
Func<int, int, int> callFunc = callExpression.Compile();
Assert.Equal(29, callFunc(20, 9));

MethodCallExpression methodCallExpression = Expression.Call(
instance: null,
typeof(CompilerTests).GetMethod("Add4", BindingFlags.NonPublic | BindingFlags.Static),
Expression.Constant(5), Expression.Constant(6), Expression.Constant(7), Expression.Constant(8));

Func<int> methodCallDelegate = Expression.Lambda<Func<int>>(methodCallExpression).Compile();
Assert.Equal(26, methodCallDelegate());
}

private static int Add(int a, int b)
{
return a + b;
}

private static int Add4(int a, int b, int c, int d)
{
return a + b + c + d;
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.LibraryBuild.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
<!-- Used by VS4Mac via reflection to symbolize stack traces -->
<method name="GetMethodFromNativeIP" />
</type>
<type fullname="System.Reflection.Emit.AssemblyBuilder">
<!-- Used by System.Linq.Expressions via reflection -->
<method name="ForceAllowDynamicCode" />
</type>
<type fullname="System.Runtime.Serialization.SerializationInfo">
<!-- Used by System.Runtime.Serialization.Formatters via reflection -->
<method name="UpdateValue" />
Expand Down
33 changes: 32 additions & 1 deletion src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/AssemblyBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ namespace System.Reflection.Emit
{
public abstract partial class AssemblyBuilder : Assembly
{
[ThreadStatic]
private static bool t_allowDynamicCode;

protected AssemblyBuilder()
{
}
Expand Down Expand Up @@ -81,12 +84,40 @@ public override string[] GetManifestResourceNames() =>

internal static void EnsureDynamicCodeSupported()
{
if (!RuntimeFeature.IsDynamicCodeSupported)
if (!RuntimeFeature.IsDynamicCodeSupported && !t_allowDynamicCode)
{
ThrowDynamicCodeNotSupported();
}
}

/// <summary>
/// Allows dynamic code even though RuntimeFeature.IsDynamicCodeSupported is false.
/// </summary>
/// <returns>An object that, when disposed, will revert allowing dynamic code back to its initial state.</returns>
/// <remarks>
/// This is useful for cases where RuntimeFeature.IsDynamicCodeSupported returns false, but
/// the runtime is still capable of emitting dynamic code. For example, when generating delegates
/// in System.Linq.Expressions while PublishAot=true is set in the project. At debug time, the app
/// uses the non-AOT runtime with the IsDynamicCodeSupported feature switch set to false.
/// </remarks>
internal static IDisposable ForceAllowDynamicCode() => new ForceAllowDynamicCodeScope();

private sealed class ForceAllowDynamicCodeScope : IDisposable
{
private readonly bool _previous;

public ForceAllowDynamicCodeScope()
{
_previous = t_allowDynamicCode;
t_allowDynamicCode = true;
}

public void Dispose()
{
t_allowDynamicCode = _previous;
}
}

private static void ThrowDynamicCodeNotSupported() =>
throw new PlatformNotSupportedException(SR.PlatformNotSupported_ReflectionEmit);
}
Expand Down