[API Proposal]: Add PersistedAssemblyBuilder type #97015
Comments
buyaa-n
added
the
api-suggestion
ghost
added
the
|
Tagging subscribers to this area: @dotnet/area-system-reflection-emit Issue DetailsBackground and motivationAs a new persist able AssemblyBuilder implementation is being added in .NET 9 it also needs an API for setting the Entry point. We could add the same API we have in .NET framework. API Proposalnamespace System.Reflection.Emit;
public partial class AssemblyBuilder
{
// Previously approved new APIs
public static AssemblyBuilder DefinePersistedAssembly(AssemblyName name, Assembly coreAssembly, IEnumerable<CustomAttributeBuilder>? assemblyAttributes = null);
public void Save(Stream stream);
public void Save(string assemblyFileName);
protected abstract void SaveCore(Stream stream);
+ public void SetEntryPoint(MethodInfo entryMethod);
}API UsageAssemblyBuilder ab = AssemblyBuilder.DefinePersistedAssembly(new AssemblyName("MyAssembly"), typeof(object).Assembly, null);
TypeBuilder tb = ab.DefineDynamicModule("MyModule").DefineType("MyType", TypeAttributes.Public | TypeAttributes.Class);
MethodBuilder mb1 = tb.DefineMethod("SumMethod", MethodAttributes.Public | MethodAttributes.Static, typeof(int), [typeof(int), typeof(int)]);
ILGenerator il = mb1.GetILGenerator();
il.Emit(OpCodes.Ldarg_0);
il.Emit(OpCodes.Ldarg_1);
il.Emit(OpCodes.Add);
il.Emit(OpCodes.Ret);
MethodBuilder main = tb.DefineMethod("Main", MethodAttributes.HideBySig | MethodAttributes.Public | MethodAttributes.Static);
ILGenerator il2 = main.GetILGenerator();
il2.Emit(OpCodes.Ldc_I4_S, 10);
il2.Emit(OpCodes.Ldc_I4_1);
il2.Emit(OpCodes.Call, mb1);
il2.Emit(OpCodes.Call, typeof(Console).GetMethod("WriteLine", [typeof(int)])!);
il2.Emit(OpCodes.Ret);
tb.CreateType();
ab.SetEntryPoint(main);
ab.Save("MyAssembly.exe");Alternative DesignsNo response RisksNo response
|
Does it need to be virtual? |
Yes, it needs to be virtual, thank you. Updated the proposal to |
buyaa-n
added
api-ready-for-review
|
Is there a reason why |
|
For now, the assembly is DLL only if no entry point set, if user needs to set more than this we can consider adding API for setting PEHeaderBuilder in the future, but that option doesn't have to be associated with
Please try out the default implementation and let us know what exact scenarios is not covered, and also check PEHeaderBuilder if there is an option that could cover your scenarios and let us know if there is anything not covered. |
namespace System.Reflection.Emit;
+ public enum PEFileKinds
+ {
+ Dll = 0x0001,
+ ConsoleApplication = 0x0002,
+ WindowApplication = 0x0003,
+ }
public abstract partial class AssemblyBuilder
{
// Previously approved new APIs
public static AssemblyBuilder DefinePersistedAssembly(AssemblyName name, Assembly coreAssembly, IEnumerable<CustomAttributeBuilder>? assemblyAttributes = null);
public void Save(Stream stream);
public void Save(string assemblyFileName);
protected abstract void SaveCore(Stream stream);
+ public void SetEntryPoint(MethodInfo entryMethod);
+ public void SetEntryPoint(MethodInfo entryMethod, PEFileKinds fileKind);
+ protected virtual void SetEntryPointCore(MethodInfo entryMethod, PEFileKinds fileKind);
} |
bartonjs
added
api-approved
|
PEFileKinds is not very future proof. There are all sorts of other bits one may want to set on the final binary. The future proof solution would be to give the caller access to System.Reflection.PortableExecutable.PEBuilder that is used to produce the binary and allow them to edit it any way they want. |
Completely agree, PEFileKinds is not enough to cover all options can be set by the user. We should not add a new API for each specific use case
The PEBuilder doesn't seem to provide much options to edit the PR file. We can't add option to set I am thinking to propose options for setting PEHeaderBuilder and CoreFlags, their combination cover all options used to set with |
What would the API look like? Would one be able to also define resources through this API (discussed in #15704 (comment))? I am wondering whether it would be better to introduce a variant of the |
I was thinking to add an overload for Save with defaulted options, instead of adding separate API for each.
That is a great idea, as including the resources it's getting too many options to set. |
API ProposalAs per above discussion the new API could look like this: public abstract partial class AssemblyBuilder
{
// Existing APIs
public void Save(Stream stream) { }
protected virtual void SaveCore(Stream stream) { }
+ public virtual MetadataBuilder GenerateMetadata(out BlobBuilder ilStream, out BlobBuilder mappedFieldData) { }
}API UsageAssemblyBuilder ab = AssemblyBuilder.DefinePersistedAssembly(new AssemblyName("MyAssembly"), typeof(object).Assembly, null);
TypeBuilder tb = ab.DefineDynamicModule("MyModule").DefineType("MyType", TypeAttributes.Public | TypeAttributes.Class);
// ...
MethodBuilder entryPoint = tb.DefineMethod("Main", MethodAttributes.HideBySig | MethodAttributes.Public | MethodAttributes.Static);
ILGenerator il2 = main.GetILGenerator();
// ...
il2.Emit(OpCodes.Ret);
tb.CreateType();
MetadataBuilder metadataBulder = ab.GenerateMetadata(out BlobBuilder ilStream, out BlobBuilder fieldData);
PEHeaderBuilder peHeaderBuilder = PEHeaderBuilder.CreateExecutableHeader();
ManagedPEBuilder peBuilder = new ManagedPEBuilder(
header: peHeaderBuilder,
metadataRootBuilder: new MetadataRootBuilder(metadataBuilder),
ilStream: ilStream,
mappedFieldData: fieldData,
entryPoint: MetadataTokens.EntityHandle(entryPoint.MetadataToken));
BlobBuilder peBlob = new BlobBuilder();
peBuilder.Serialize(peBlob);
// in case saving to a file:
using var fileStream = new FileStream("MyAssembly.exe", FileMode.Create, FileAccess.Write);
peBlob.WriteContentTo(fileStream); I assume we don't want to add this |
buyaa-n
added
api-ready-for-review
|
The general idea looks good. I do not think it can be a virtual method on There are a few options that I can think of: Option 1 (I think that this is the cleanest option):
Option 2:
Option 3:
For the purpose of API discussion, you can enhance the sample to show how one would set a custom properties like StackSize, BaseSize, or the equivalent of
Yes, I agree, |
|
I like Option 1 even without the cited issues, as it moves Save from somewhere it will typically throw to somewhere it will typically work. Presumably the members on PersistedAssemblyBuilder would still need to be virtual if we want to support extensibility of it just as we do with AssemblyBuilder? |
Good point, forgot that, thank you! I will add another proposal incorporating all suggestions. |
I think |
API Proposalnamespace System.Reflection.Emit;
public abstract partial class AssemblyBuilder
{
// Previously approved new APIs
- public static AssemblyBuilder DefinePersistedAssembly(AssemblyName name, Assembly coreAssembly, IEnumerable<CustomAttributeBuilder>? assemblyAttributes = null);
- public void Save(Stream stream);
- public void Save(string assemblyFileName);
- protected abstract void SaveCore(Stream stream);
}
+public sealed class PersistedAssemblyBuilder : AssemblyBuilder
{
+ public PersistedAssemblyBuilder(AssemblyName name, Assembly coreAssembly, IEnumerable<CustomAttributeBuilder>? assemblyAttributes = null);
+ public void Save(Stream stream);
+ public void Save(string assemblyFileName);
+ public MetadataBuilder GenerateMetadata(out BlobBuilder ilStream, out BlobBuilder mappedFieldData);
}API UsagePersistedAssemblyBuilder ab = new PersistedAssemblyBuilder(new AssemblyName("MyAssembly"), typeof(object).Assembly);
TypeBuilder tb = ab.DefineDynamicModule("MyModule").DefineType("MyType", TypeAttributes.Public | TypeAttributes.Class);
// ...
MethodBuilder entryPoint = tb.DefineMethod("Main", MethodAttributes.HideBySig | MethodAttributes.Public | MethodAttributes.Static);
ILGenerator il2 = entryPoint .GetILGenerator();
// ...
il2.Emit(OpCodes.Ret);
tb.CreateType();
MetadataBuilder metadataBulder = ab.GenerateMetadata(out BlobBuilder ilStream, out BlobBuilder fieldData);
PEHeaderBuilder peHeaderBuilder = new PEHeaderBuilder(
// Header for PEFileKinds.WindowApplication imageCharacteristics and subsystem need to be set
imageBase: 0x00400000, // this is the default value, was not necessarily needed to set.
imageCharacteristics: Characteristics.ExecutableImage,
subsystem: Subsystem.WindowsGui);
ManagedPEBuilder peBuilder = new ManagedPEBuilder(
header: peHeaderBuilder,
metadataRootBuilder: new MetadataRootBuilder(metadataBuilder),
ilStream: ilStream,
mappedFieldData: fieldData,
entryPoint: MetadataTokens.MethodDefinitionHandle(entryPoint.MetadataToken));
BlobBuilder peBlob = new BlobBuilder();
peBuilder.Serialize(peBlob);
// in case saving to a file:
using var fileStream = new FileStream("MyAssembly.exe", FileMode.Create, FileAccess.Write);
peBlob.WriteContentTo(fileStream);
fileStream.Close(); |
This can be a regular constructor. I do not think that a factory method serves any purpose here. |
buyaa-n
added
the
blocking
buyaa-n
changed the title
|
I have moved the API proposal to the description as it became completely different proposal than the original one, folded the old proposal in the description. Added blocking to get it reviewed ASAP. |
|
Right, in the new example I changed the |
|
Breaking API compatibility from .NET Framework (by having Save somewhere other than on AssemblyBuilder) is unfortunate; but seems tolerable based on expected usage. Given that, looks good as proposed. namespace System.Reflection.Emit;
public abstract partial class AssemblyBuilder
{
// Previously approved new APIs
- public static AssemblyBuilder DefinePersistedAssembly(AssemblyName name, Assembly coreAssembly, IEnumerable<CustomAttributeBuilder>? assemblyAttributes = null);
- public void Save(Stream stream);
- public void Save(string assemblyFileName);
- protected abstract void SaveCore(Stream stream);
}
+public sealed class PersistedAssemblyBuilder : AssemblyBuilder
{
+ public PersistedAssemblyBuilder(AssemblyName name, Assembly coreAssembly, IEnumerable<CustomAttributeBuilder>? assemblyAttributes = null);
+ public void Save(Stream stream);
+ public void Save(string assemblyFileName);
+ public MetadataBuilder GenerateMetadata(out BlobBuilder ilStream, out BlobBuilder mappedFieldData);
} |
bartonjs
added
api-approved
dotnet-policy-service
bot
added
the
in-pr
and others
Background and motivation
We added a new persisted AssemblyBuilder implementation and related new APIs in .NET 9 preview 1. While discussing adding the
SetEntryPointmethod questions raised for addingSetEntryPoint(MethodInfo, PEFileKinds)the overload that sets PEFileKinds option.Further we would need to add more options that used in .NET framework, like PortableExecutableKinds or ImageFileMachine that used to set with
Save(String, PortableExecutableKinds, ImageFileMachine)overload, plus would need to add APIs for settingAddResourceFile,DefineResource,DefineUnmanagedResource.Instead of adding all old .NET framework APIs we prefer to let the customers handle their assembly building process by themselves using the PEHeaderBuilder and
System.Reflection.PortableExecutable.PEBuilderimplementation ManagedPEBuilder options.runtime/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaderBuilder.cs
Lines 46 to 65 in 27214a0
runtime/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/ManagedPEBuilder.cs
Lines 37 to 48 in 27214a0
These options are used to produce the assembly and allow users to configure it any way they want. These covers all options that existed in .NET framework, (but not exactly same way), plus provide many other options that not available in .NET framework that customers may want to set on the final binary.
In order to achieve this, we would need to provide all metadata information produced with Reflection.Emit APIs (MetadataBuilder and ILStreams) so that user could embed them into the corresponding section of
PEBuidler. But because the MetadataBuilder and BlobBuilder types are not accessible from within CoreLib we decided to make thePersistedAssemblyBuildertype public and reintroduce the related new APIs there, remove the new APIs from the baseAssemblyBuildertype.API Proposal
namespace System.Reflection.Emit; public abstract partial class AssemblyBuilder { // Previously approved new APIs - public static AssemblyBuilder DefinePersistedAssembly(AssemblyName name, Assembly coreAssembly, IEnumerable<CustomAttributeBuilder>? assemblyAttributes = null); - public void Save(Stream stream); - public void Save(string assemblyFileName); - protected abstract void SaveCore(Stream stream); } +public sealed class PersistedAssemblyBuilder : AssemblyBuilder { + public PersistedAssemblyBuilder(AssemblyName name, Assembly coreAssembly, IEnumerable<CustomAttributeBuilder>? assemblyAttributes = null); + public void Save(Stream stream); + public void Save(string assemblyFileName); + public MetadataBuilder GenerateMetadata(out BlobBuilder ilStream, out BlobBuilder mappedFieldData); }API Usage
Old
AssemblyBuilder.SetEntryPoint(MethodInfo)Proposal folded below:Add AssemblyBuilder.SetEntryPoint(MethodInfo) method
### Background and motivationAs a new persist able AssemblyBuilder implementation is being added in .NET 9 it also needs an API for setting the Entry point. We could add similar but virtual API we have in .NET framework.
API Proposal
namespace System.Reflection.Emit; public abstract partial class AssemblyBuilder { // Previously approved new APIs public static AssemblyBuilder DefinePersistedAssembly(AssemblyName name, Assembly coreAssembly, IEnumerable<CustomAttributeBuilder>? assemblyAttributes = null); public void Save(Stream stream); public void Save(string assemblyFileName); protected abstract void SaveCore(Stream stream); + public virtual void SetEntryPoint(MethodInfo entryMethod); }API Usage
Alternative Designs
Or keep the same pattern (have protected virtual
*Coremethod).namespace System.Reflection.Emit; public abstract partial class AssemblyBuilder { // Previously approved new APIs public static AssemblyBuilder DefinePersistedAssembly(AssemblyName name, Assembly coreAssembly, IEnumerable<CustomAttributeBuilder>? assemblyAttributes = null); public void Save(Stream stream); public void Save(string assemblyFileName); protected abstract void SaveCore(Stream stream); + public void SetEntryPoint(MethodInfo entryMethod); + protected virtual void SetEntryPointCore(MethodInfo entryMethod); }The text was updated successfully, but these errors were encountered: