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.

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

More usable reflection-free mode #67193

Closed
kant2002 opened this issue Jul 15, 2021 · 30 comments
Closed

More usable reflection-free mode #67193

kant2002 opened this issue Jul 15, 2021 · 30 comments

Comments

@kant2002
Copy link
Contributor

I notice that dotnet/corert#8216 indicating that this may be available in NativeAOT context.
With that method available, or even better Enum.IsDefined, WinForms can be usable in reflection-free mode.

There 19 usages of that method in WinForms, some of them in assertions, some in tests, but couple in legit places which guard public API, so cannot be easily removed/replaced.

@MichalStrehovsky
Copy link
Member

Yes, we can make enums work at some point. The plan for that is to:

  • Delete System.Private.DisabledReflection
  • Introduce a feature switch (along the lines of e.g. Make it possible to disable non-filestream support in XmlDownloadManager runtimelab#619) and put it wherever needed in System.Private.Reflection.Core/System.Private.Reflection.Execution. The goal is to prevent all the code related to RuntimeMethodInfo from being generated.
  • Introduce a new flag to the compiler that is similar to disable reflection, but instead disables all reflection generation for fields/methods/properties/events.

That way we'll not duplicate the reflection stack and we'll have a mode where method-level reflection support can be trimmed away.

@jkotas
Copy link
Member

jkotas commented Jul 15, 2021

Let's use this as the tracking issue for the more usable reflection-free mode.

@illnyang
Copy link

It seems that Enum.GetValues is yet to be supported in reflection-free mode. Unfortunately, even .NET libraries that otherwise strive against the use of reflection for performance reasons are forced to call it (i.e. Silk.NET). The following is a solution that I came up with in order to temporarily mitigate this issue (as an end-user) until the underlying issue is resolved. Here's an outline of what it does:

  1. Compile managed assemblies for a Project and copy all referenced assemblies to the output directory.

  2. Manually evaluate every call to Enum.GetValues using library called dnlib.

    • Inserts static array field into callee's Type
    • Inserts List.Add calls into Type's static constructor with actual enum values
    • Changes all calls to Enum.GetValues such that the newly inserted static array field is used in place of the call's return value
  3. Rewire compiler response files to use weaved assemblies instead of grabbing them from their original paths.

  4. Compile and link object file ourselves

    • This ensures that the compiler response file won't get overwritten by dotnet publish
  5. Publish without building and get final binaries

Of course, this won't be useful for you - maintainers, but hopefully will prevent discouragement in NativeAOT's early adopters!

Project's property group

Code
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
    <Nullable>enable</Nullable>
    <LangVersion>preview</LangVersion>
    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
    <RuntimeIdentifier>win-x64</RuntimeIdentifier>
    <IlcDisableReflection>true</IlcDisableReflection>
    <CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>
    <StaticallyLinked>true</StaticallyLinked>
    <IlcOptimizationPreference>Size</IlcOptimizationPreference>
    <RestoreProjectStyle>PackageReference</RestoreProjectStyle>
    <RunAOTCompilation>true</RunAOTCompilation>
    <InvariantGlobalization>true</InvariantGlobalization>
    <IlcGenerateStackTraceData>false</IlcGenerateStackTraceData>
    <EnableAggressiveTrimming>true</EnableAggressiveTrimming>
    <TrimMode>link</TrimMode>
    <NativeCompilationDuringPublish>true</NativeCompilationDuringPublish>
  </PropertyGroup>

Build script

Code
rem First build will fail because compiler responses are not there yet
@echo off
del /S /Q build\*
dotnet clean

dotnet build Program -c Release
dotnet build EnumValuesEvaluatorWeaver -c Release
dotnet build CompilerResponseRewriter -c Release

dotnet EnumValuesEvaluatorWeaver.dll Project\bin\Release\net6.0\win-x64\ build\
dotnet CompilerResponseRewirer.dll Project\obj\Release\net6.0\win-x64\native\Project.ilc.rsp build\
pushd Program
ilc @"obj\Release\net6.0\win-x64\native\Project.ilc.rsp"
link" @"obj\Release\net6.0\win-x64\native\link.rsp"
popd

dotnet publish Project --no-build -c Release -o dist -r win-x64

Compiler response rewirer

Code
using System.IO;
using System.Linq;
using System.Text;

namespace CompilerResponseRewirer
{
    internal static class Program
    {
        // [0] = path to compiler response file
        // [1] = path to directory with weaved assemblies
        internal static int Main(string[] args)
        {
            var responseLines = File.ReadAllLines(Path.GetFullPath(args[0]));
            var weavedAssemblies = Directory.GetFiles(Path.GetFullPath(args[1]), "*.dll");

            for (var i = 0; i < responseLines.Length; i++)
            {
                var weavedAssembly =
                    weavedAssemblies.SingleOrDefault(x =>
                    {
                        var basename = Path.GetFileName(x);
                        var result = responseLines[i].EndsWith(basename);
                        return result;
                    });
                
                if (weavedAssembly == default)
                    continue;

                var prefixes = new string[]
                {
                    "-r:",
                    "--conditionalroot:",
                    "--root:"
                };

                foreach (var prefix in prefixes)
                {
                    if (!responseLines[i].StartsWith(prefix))
                        continue;
                    
                    responseLines[i] = prefix + weavedAssembly;
                    goto exit;
                }

                responseLines[i] = weavedAssembly;
                
                exit:
                // ReSharper disable once RedundantJumpStatement
                continue;
            }

            File.WriteAllLines(Path.GetFullPath(args[0]), responseLines, Encoding.UTF8);
            
            return 0;
        }
    }
}

EnumValuesEvaluatorWeaver

Code
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using dnlib.DotNet;
using dnlib.DotNet.Emit;

namespace EnumValuesEvaluatorWeaver
{
    internal static class Program
    {
        // [0] = path to assemblies
        // [1] = output path
        internal static int Main(string[] args)
        {
            foreach (var assemblyPath in Directory.GetFiles(args[0], "*.dll"))
            {
                try
                {
                    var saveModule = false;
                    ModuleContext modCtx = ModuleDef.CreateModuleContext();
                    ModuleDefMD mod = ModuleDefMD.Load(assemblyPath, modCtx);
                    var importer = new Importer(mod);
                    foreach (var type in mod.GetTypes().ToArray())
                    {
                        foreach (var method in type.Methods.ToArray())
                        {
                            if (!method.HasBody)
                                continue;

                            if (!method.Body.HasInstructions)
                                continue;

                            var initialInstructions = method.Body.Instructions.ToArray();
                            
                            for (var i = 0; i < initialInstructions.Length; i++)
                            {
                                var curInstr = initialInstructions[i];
                                
                                if (curInstr.Operand == null)
                                    continue;
                                
                                if (curInstr.OpCode != OpCodes.Call)
                                    continue;
                                    
                                var callToGetValues = (curInstr.Operand as IMethodDefOrRef);
                                        
                                if (!(callToGetValues?.FullName?.Equals(
                                    "System.Array System.Enum::GetValues(System.Type)") ?? false)) continue;
                                        
                                var arrayRef = importer.ImportAsTypeSig(typeof(int[]));
                                var listTypeDef = importer.ImportAsTypeSig(typeof(List<int>));
                                            
                                var enumDef = (method.Body.Instructions[i - 2].Operand as TypeRef)?.Resolve();

                                if (enumDef is not { IsEnum: true })
                                    continue;

                                var valueStubName = enumDef.ReflectionFullName.Replace(".", "_") + "_ValueStub";
                                            
                                var values = (from field in enumDef.Fields.ToArray() where field.Constant?.Value != null select (int)field.Constant.Value).ToArray();
                 
                                var stubField = new FieldDefUser(valueStubName, new FieldSig(arrayRef))
                                {
                                    Access = FieldAttributes.Private,
                                    Attributes = FieldAttributes.Static
                                };

                                type.Fields.Add(stubField);
                                
                                method.Body.Instructions.RemoveAt(i);
                                method.Body.Instructions.RemoveAt(i - 1);
                                method.Body.Instructions.RemoveAt(i - 2);
                                method.Body.Instructions.Insert(i - 2, OpCodes.Ldsfld.ToInstruction(stubField));
                                method.Body.UpdateInstructionOffsets();
                                            
                                var listTypeSpec = new TypeSpecUser(listTypeDef);
                                var listCtor = new MemberRefUser(mod, ".ctor", MethodSig.CreateInstance(mod.CorLibTypes.Void), listTypeSpec);
                                            
                                var listAdd = new MemberRefUser(mod, "Add",
                                    MethodSig.CreateInstance(mod.CorLibTypes.Void, new GenericVar(0)),
                                    listTypeSpec);
                                            
                                var listToArray = new MemberRefUser(mod, "ToArray",
                                    MethodSig.CreateInstance(new SZArraySig(new GenericVar(0))),
                                    listTypeSpec);

                                var typeCCtor = type.FindOrCreateStaticConstructor();
                                var localList = typeCCtor.Body.Variables.Add(
                                    new Local(listTypeDef, valueStubName + "_temp"));
                                
                                var instructions = new List<Instruction>();
                                instructions.Add(OpCodes.Newobj.ToInstruction(listCtor));
                                instructions.Add(OpCodes.Stloc.ToInstruction(localList));
                                foreach (var val in values)
                                {
                                    instructions.Add(OpCodes.Ldloc.ToInstruction(localList));
                                    instructions.Add(OpCodes.Ldc_I4.ToInstruction(val));
                                    instructions.Add(OpCodes.Call.ToInstruction(listAdd));
                                }
                                instructions.Add(OpCodes.Ldloc.ToInstruction(localList));
                                instructions.Add(OpCodes.Callvirt.ToInstruction(listToArray));
                                instructions.Add(OpCodes.Stsfld.ToInstruction(stubField));
                                
                                for (var j = instructions.Count - 1; j >= 0; j--)
                                    typeCCtor.Body.Instructions.Insert(0, instructions[j]);

                                typeCCtor.Body.Instructions.UpdateInstructionOffsets();
                                            
                                saveModule = true;
                            }
                        }
                    }

                    if (!saveModule)
                        continue;
                    
                    Console.WriteLine($"Processed: {Path.GetFileName(assemblyPath)}");
                    mod.Write(Path.Combine(args[1], Path.GetFileName(assemblyPath)));
                }
                catch (BadImageFormatException)
                {
                    Console.WriteLine("Skipping bad image: " + Path.GetFileName(assemblyPath));
                }
            }
            
            return 0;
        }
    }
}

@illnyang
Copy link

As a side note, is there any better way to setup a weaving pipeline for NativeAOT-targeted patches like that in the batch script above?

@kant2002
Copy link
Contributor Author

@kvdrrrrr I think you should inject your weaving into regular dotnet publish process without ILC, after that I think ILC will pickup what you produce.

@tomrus88
Copy link

tomrus88 commented Feb 5, 2022

Would be really nice for enums to not break in reflection free mode. Including cases where they are used for displaying text to user, like this:

enum Foo { Bar }
Foo x = Foo.Bar;
Console.WriteLine($"Something {x}");

Currently in reflection free mode it will print "Something 0" instead of "Something Bar" which is incorrect and unexpected...

@jkotas jkotas transferred this issue from dotnet/runtimelab Mar 26, 2022
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Mar 26, 2022
@MichalStrehovsky MichalStrehovsky added area-NativeAOT-coreclr and removed area-NativeAOT untriaged New issue has not been triaged by the area owner labels Mar 28, 2022
@MichalStrehovsky MichalStrehovsky added this to the Future milestone Mar 28, 2022
@smhmhmd
Copy link
Contributor

smhmhmd commented Apr 6, 2022

I did not see any warnings while compiling with reflection-free mode, I can file an issue with full description, if needed.
Some tests for generics and delegates also seem to fail.

@jkotas
Copy link
Member

jkotas commented Apr 7, 2022

The reflection-free mode is an experiment to demonstrate how far can reflection stripping get us. I do not expect we are going to build full end-to-end experience around it, including warnings, analyzers, etc. The reflection use in .NET libraries is too wide spread for the reflection-free mode to be useful for real-world projects.

It may be more interesting to work on improving IlcTrimMetadata=true mode that only trims reflection data that are provably not used. This mode has much better chance of being usable by real-world projects with confidence.

@kant2002
Copy link
Contributor Author

From what I see feature switch System.Reflection.IsReflectionExecutionAvailable already defined

<IlcArg Condition="$(IlcDisableReflection) == 'true'" Include="--feature:System.Reflection.IsReflectionExecutionAvailable=false" />

It's used only in System.Private.StackTraceMetadata, but I think it can be used for System.Private.Reflection.Core/System.Private.Reflection.Execution merge too. Is this fine?

@MichalStrehovsky
Copy link
Member

The IsReflectionExecutionAvailable feature switch would disappear once System.Private.DisabledReflection is replaced by regular reflection stack. The switch only exists to make sure S.P.Stacktracemetadata doesn't attempt to call into uninitialized System.Private.Reflection.Execution (we didn't initialize it in reflection disabled mode - the --autoinitassembly line for it is not passed to ILC when reflection is disabled).

@kant2002
Copy link
Contributor Author

kant2002 commented Jun 6, 2022

Additional code which should be handled by this mode (probably)

Module module = typeof(Application).Module;
var name = module.Name;

and

Assembly module = typeof(Application).Assembly;
var name = module.GetName().Name;

@kant2002
Copy link
Contributor Author

I was unexpected for me, but following methods Type.IsGenericTypeDefinition, Type.IsAbstract and Type.IsInterface also do not work in reflection free mode, so they are good candidates for this mode as well.

@aromaa
Copy link
Contributor

aromaa commented Aug 13, 2022

Surprisingly the Process.Start doesn't work under reflection free mode. It ends up calling SerializationGuard which then uses reflection here in a bit odd way. Not sure why the class isn't just caching the Switch.System.Runtime.Serialization.SerializationGuard.AllowProcessCreation context switch to readonly static?

@stephentoub
Copy link
Member

Surprisingly the Process.Start doesn't work under reflection free mode. It ends up calling SerializationGuard

cc: @GrabYourPitchforks, @bartonjs

@MichalStrehovsky
Copy link
Member

FWIW, I ifdef out all of serialization guard for bflat so reflection disabled with Process.Start works there just fine. The reflection as it is there wouldn't even work in the "more usable reflection-free mode" that this issue is tracking.

@MichalStrehovsky
Copy link
Member

If we do the work outlined in #80165, it's possible we'll be able to just Won't fix this issue and delete the reflection-free mode.

The size of the output will proportionally scale with how much reflection is used by the app and apps that don't use it won't have to pay for it much. Enums will work and will be cheap. typeof(Foo).Assembly will work and will be cheap. More advanced things that access members of types will bring in the full reflection stack.

@kekekeks
Copy link

kekekeks commented Jan 4, 2023

Would one still be able to forcefully disable that "full reflection stack"?

@MichalStrehovsky
Copy link
Member

Would one still be able to forcefully disable that "full reflection stack"?

I'd like to know more about your scenario for that.

The situation right now is that there's some code in the very low levels of the framework that has a dependency on reflection and therefore we can't trim it away. Reflection disabled just says "feel free to break this code, I want small size". It's a hack. No user knows whether it's safe to do do that and how much they'll be broken. People are often broken.

If we do #80165, the reflection from those places would go away. One would get small size automatically if there's no reflection. If reflection does show up, there will be a size hit, but things will continue working. Some of what would normally be called reflection is not an expensive reflection (e.g. enum stringification), so that one would come along for free.

What are the scenarios where we would still need a switch for "please break reflection", "please break enum stringification", "please break retrieving owning assembly" (the latter two would already be included in a hello world and working).

@kekekeks
Copy link

kekekeks commented Jan 5, 2023

What are the scenarios where we would still need a switch for "please break reflection"

There are several use cases for force removing method and property information:

  1. to identify places in the code base that are still using the expensive part of the reflection stack
  2. to produce a binary with absolute bare minimum meta information to make reverse-engineering efforts harder. Reflection usage is the main enemy of anti reverse engineering protection even without NativeAOT

I don't see cases where one would want to break enum stringification and typeof(Foo).Assembly

@teo-tsirpanis
Copy link
Contributor

More advanced things that access members of types will bring in the full reflection stack.

Does typeof(T).TypeHandle or just an ldtoken staying on the RuntimeTypeHandle bring the full reflection stack? Similar with field and method handles.

@MichalStrehovsky
Copy link
Member

More advanced things that access members of types will bring in the full reflection stack.

Does typeof(T).TypeHandle or just an ldtoken staying on the RuntimeTypeHandle bring the full reflection stack? Similar with field and method handles.

It doesn't - this would stay a "cheap" operation.

But field and method handles would not be very usable because actually grabbing anything with a MethodBase or FieldInfo would bring pretty much all of the reflection stack.

@teo-tsirpanis
Copy link
Contributor

My class will store pointers to RVA fields while holding the RuntimeFieldHandle in the class to prevent unloading. The field handle will be created through a generated ldtoken instruction.

Since NativeAOT does not support unloading and its runtime handles are just IntPtrs, there would be no reason to bring the entire reflection stack. That's why I'm asking.

@MichalStrehovsky
Copy link
Member

My class will store pointers to RVA fields while holding the RuntimeFieldHandle in the class to prevent unloading. The field handle will be created through a generated ldtoken instruction.

Since NativeAOT does not support unloading and its runtime handles are just IntPtrs, there would be no reason to bring the entire reflection stack. That's why I'm asking.

If you're doing it through generated code, keeping the RuntimeTypeHandle of the owning type is very likely less overhead. typeof is super optimized both in codegen and elsewhere. It will definitely be less size on NativeAOT.

@MarcoRossignoli
Copy link
Member

@MichalStrehovsky @jkotas we're building NativeAOT aware code, is there a good list of the current api allowed by Type somewhere? Can this document https://github.com/dotnet/runtime/blob/main/src/coreclr/nativeaot/docs/reflection-free-mode.md be updated or removed for something more up to date?Or is it still valid?

@MichalStrehovsky
Copy link
Member

@MarcoRossignoli If you're building for net7.0, just add:

<PropertyGroup>
    <EnableTrimAnalyzer>true</EnableTrimAnalyzer>
    <EnableAotAnalyzer>true</EnableAotAnalyzer>
</PropertyGroup>

to the project file.

To test this works, try:

public class Class1
{
    static Type t;

    static void Frob()
        => t.MakeGenericType(Array.Empty<Type>());
}

You should see IL3050 and IL2055 warnings when you build (or green squiggles in VS).

This is general guidance for trimming and AOT.

If you're asking specifically for reflection disabled mode (that this issue tracks), do no worry about that one. It's unlikely to ever ship in a different shape than right now (the shape right now is, "enable this and watch APIs that have nothing to do with reflection be broken (like Process.Start)").

@MichalStrehovsky
Copy link
Member

I don't think we're going to invest into this. We're not deleting System.Private.DisabledReflection, but at this point there's very minimal size difference between reflection enabled and disabled mode (300-400 kB if you don't actually call into reflection APIs).

The only advantage of reflection-disabled mode is obfuscation and people who use it for obfuscation are not going to appreciate putting more data into the executable to support enum stringification or assembly names/manifest metadata so this should work for them as-is.

@MichalStrehovsky MichalStrehovsky closed this as not planned Won't fix, can't repro, duplicate, stale Jun 13, 2023
@aromaa
Copy link
Contributor

aromaa commented Jun 13, 2023

I brought it up earlier but what about Process.Start which brings the whole reflection stack with it? This adds about 1MB to the binary size. Is this something that can be looked at and is it worthwhile to open a new issue about it?

@MichalStrehovsky
Copy link
Member

I brought it up earlier but what about Process.Start which brings the whole reflection stack with it? This adds about 1MB to the binary size. Is this something that can be looked at and is it worthwhile to open a new issue about it?

You can try. #29212 was a similar issue that was won't fixed. I would be happy if "serialization guard" went away, especially on runtimes like NativeAOT where the "you can't reflect on arbitrary stuff because the compiler simply doesn't make it available" comes from the design. But the call on that is not for me to make.

@stephentoub
Copy link
Member

I would be happy if "serialization guard" went away, especially on runtimes like NativeAOT where the "you can't reflect on arbitrary stuff because the compiler simply doesn't make it available" comes from the design. But the call on that is not for me to make.

I'd be happy if it went away entirely. I wasn't happy it was added in the first place.

@GrabYourPitchforks, with where we are for 8 with the status of BinaryFormatter deprecation, can we kill this now?

@jkotas
Copy link
Member

jkotas commented Jun 13, 2023

#87470 was opened for the serialization guard topic. Let's continue the discussion there.

@ghost ghost locked as resolved and limited conversation to collaborators Jul 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests