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

Support equivalent of AssemblyBuilder.Save to save in-memory IL to an assembly #62956

Closed
14 of 24 tasks
steveharter opened this issue Dec 17, 2021 · 45 comments
Closed
14 of 24 tasks
Assignees
Labels
area-System.Reflection.Emit Cost:L Work that requires one engineer up to 4 weeks Priority:2 Work that is important, but not critical for the release Team:Libraries tenet-compatibility Incompatibility with previous versions or .NET Framework User Story A single user-facing feature. Can be grouped under an epic.
Milestone

Comments

@steveharter
Copy link
Member

steveharter commented Dec 17, 2021

The most common feature request in reflection, with 100+ upvotes, is to add support for equivalent AssemblyBuilder.Save() functionality. Since this exists in .NET Desktop, some consider this an adoption blocker.

Prototyping done in .NET 7.0 to investigate feasibility of adding the Save() via an adapter from AssemblyBuilder to the newer MetadataBuilder which supports writing IL to a Stream\file. Related issues:

System.Reflection.Emit.AssemblyBuilder.Save
Unable to set EntryPoint on generated assemblies with System.Reflection.Emit

We plan to abstract out Reflection.Emit APIs in order to have two implementations, one that having old code to support all downlevel runtime, and a new implementation that replaces the current Reflection.Emit implementation with AssemblyBuilder.Save() support, steps:

  • Abstract out Reflection.Emit APIs, refactor old code that support all downlevel runtime versions.
  • Move the prototyping work into runtime with necessary refactoring
  • Have the new APIs for ParameterBuilder and AssemblyBuilder.Save(string/stream) reviewed and approved
  • Implement the parts that needed for MVP (Minimum Viable Product) AssemblyBuilder.Save(string/stream)
    • Add support for saving different types like struct/class etc. and their references
    • Add support for saving CustomAttributes for all members like assembly/module (PR)
    • Have an APIs proposal for abstracting ILGenerator and other related APIs reviewed and approved.
    • Abstract out ILGenerator accordingly
    • Save interface implementation, nested types for a Type (in PR)
    • Abstract out ParameterBuilder in runtime, add ParameterBuilderImpl and save it to file/stream
    • Add EnumBuilderImpl and save it to file/stream
    • Add support for more complicated signatures for each member (generics/array/byRef/pointer etc)
      • Add GenericTypeParameterBuilderImpl and save a generic Type/Method
    • Save constant value for a Field

Remaining work that out of .NET 8.0:

  • Implement remaining parts needed for AssemblyBuilder.Save(string/stream) (mostly will rely on community contributions)
    • ILGenerator implementation
    • Add support for remaining members
      • Add ConstructorBuilderImpl and save it to file/stream (this needs minimal IL support)
      • Add PropertyBuilderImpl and save it to file/stream (this also needs minimal IL support)
      • Add EventBuildterImpl and save it to file/stream
      • Add implementation for remaining APIs for a MethodBuilder/TypeBuilder/FieldBuilder (like SignatureCallingConvention, *RequiredCustomModifiers, *OptionalCustomModifiers)
    • Have an API for creating PDBs and implement it
    • Entry point support
  • Add support for Running the assembly
@steveharter steveharter added area-System.Reflection tenet-compatibility Incompatibility with previous versions or .NET Framework User Story A single user-facing feature. Can be grouped under an epic. Priority:2 Work that is important, but not critical for the release Cost:L Work that requires one engineer up to 4 weeks Bottom Up Work Not part of a theme, epic, or user story Team:Libraries labels Dec 17, 2021
@steveharter steveharter added this to the 7.0.0 milestone Dec 17, 2021
@steveharter steveharter self-assigned this Dec 17, 2021
@ghost
Copy link

ghost commented Dec 17, 2021

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

Issue Details

The most common feature request in reflection, with 100+ upvotes, is to add support for equivalent AssemblyBuilder.Save() functionality. Since this exists in .NET Desktop, some consider this an adoption blocker.

Currently prototyping has started to investigate feasibility of adding the Save() via an adapter from AssemblyBuilder to the newer MetadataBuilder which supports writing IL to a Stream\file.

Author: steveharter
Assignees: steveharter
Labels:

area-System.Reflection, tenet-compatibility, User Story, Priority:2, Cost:L, Bottom Up Work, Team:Libraries

Milestone: 7.0.0

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Dec 17, 2021
@lbargaoanu
Copy link

lbargaoanu commented Dec 18, 2021

Would that make LambdaExpression.CompileToMethod (or smth along those lines) more likely? :) I know System.Linq.Expressions is archived, but apparently things are going to happen.

@buyaa-n buyaa-n removed the untriaged New issue has not been triaged by the area owner label Dec 20, 2021
@steveharter steveharter removed the Bottom Up Work Not part of a theme, epic, or user story label Jan 5, 2022
@MichaelKetting
Copy link
Contributor

@steveharter I have not fully investigated MetadataBuilder at this time, but is there a 'cheap' way to get all the IL needed and just pipe it into MetadataBuilder or do we need to do this IL-statement by IL-statement?

Also, thanks @danmoseley for mentioning this API. I've been looking for this forever.

@steveharter steveharter removed their assignment May 12, 2022
@MSDN-WhiteKnight
Copy link
Contributor

I have not fully investigated MetadataBuilder at this time, but is there a 'cheap' way to get all the IL needed and just pipe it into MetadataBuilder or do we need to do this IL-statement by IL-statement?

It is done currently statement-by-statement, you need to create InstructionEncoder and emit statements into it, then pass it to MethodBodyStreamEncoder.AddMethodBody. I suppose, we could add new AddMethodBody overload that takes array of IL bytes and emits it as a whole if it would be found useful. But the important thing about that proposed adapter is that it should not just mechanically copy the contents of in-memory assembly into MetadataBuilder, it needs to do some changes - replace every reference to System.Private.CoreLib to System.Runtime, for example. So the ability to emit the whole IL array might not simplify the task significantly.

@MichaelKetting
Copy link
Contributor

@MSDN-WhiteKnight Thanks for the details. So, it's definitely not "cheap" as in "copy all IL bytes inside the assembly from A to B" but requires a more structured approach. Good to know because for me this means it's best to wait for the official API to catch up instead of doing it in my own code and then switch later on once the official API also supports it (aside from the question if/when/how best to contribute to this).

@steveharter
Copy link
Member Author

Moving to 8.0. There is currently active prototyping work for this.

@dotnet dotnet deleted a comment from KicsA Jul 21, 2023
@MichaelKetting
Copy link
Contributor

@buyaa-n Thank you for the update!

Since IL is not supported in the first drop and you mentioned testing out the APIs via Reflection: My scenario is to use runtime code generation to build subclass proxies and then persisting the generated assemblies so we don't pay for it during each application start. Right now, the Save-code is enabled only for the .NET Framework targeted assembly of our library.

If I read the current state of the implementation correctly, there is no helpful information to gain if I try the current state via reflection. Since the complete feature is still marked for the .NET 8 release, I'll happily wait for the drop with IL support and test once my scenario becomes available. If there is anything I should test (and provide feedback on) before that point, please let me know.

For completeness sake: right now I'm expecting that I can just call the same APIs again as I do in .NET Framework once the feature is production ready (AssemblyBuilder.Save() and ILGenerator.MarkSequencePoint and the ISymbolDocumentWriter interface's API)

Thanks, Michael

@masonwheeler
Copy link
Contributor

OK, sorry about that. Let's try this again, a bit less confrontationally.

I was under the impression that the main reason AssemblyBuilder.Save was not originally included in .NET Core was a technical one: generating cross-platform PE files and PDB files is not as simple as generating Windows-only binaries. Therefore, I was kind of expecting that the Core implementation would keep about 90% of the existing, tried-and-tested Save code from .NET Framework and focus on that specific challenge.

The not-supported features described above all fall under the category of "generate IL and metadata," which doesn't feel like something that needs to be reimplemented in a cross-platform manner. (There is no platform-dependent code in System.Reflection.Metadata.IL.MethodBodyBlock, for example.) But apparently this is not the case. So what am I missing?

@MSDN-WhiteKnight
Copy link
Contributor

For completeness sake: right now I'm expecting that I can just call the same APIs again as I do in .NET Framework once the feature is production ready

This expectation can't be met. It is already decided that API surface is different, the new API takes additional parameter for core assembly, while .NET Framework API always implied the current implementation's core assembly.

Therefore, I was kind of expecting that the Core implementation would keep about 90% of the existing, tried-and-tested Save code from .NET Framework and focus on that specific challenge.

Same goes for this expectation, too. It was already discussed at length, maintainers stated that existing code is native and tied to Windows too much, so it's too hard to bring it cross platform. So the current work entirely reimplements everything as managed implementation on top of System.Reflection.Metadata.

@MichaelKetting
Copy link
Contributor

@masonwheeler

Therefore, I was kind of expecting that the Core implementation would keep about 90% of the existing, tried-and-tested Save code from .NET Framework and focus on that specific challenge.

Let me paraphrase: AssemblyBuilder.Save() in .NET 8 is already able to persist a complete .NET assembly (the DLL file, personally, I don't care about doing it with EXE files) to disk after the types (including method bodies) have been created at runtime in memory?

The not-supported features described above all fall under the category of "generate IL and metadata," which doesn't feel like something that needs to be reimplemented in a cross-platform manner. (There is no platform-dependent code in System.Reflection.Metadata.IL.MethodBodyBlock, for example.)

This sentence actually confuses me a bit right now:

The not-supported features described above

Do you mean the features I mentioned or the features discussed in the announcement post ? In the announcement post, method bodies are listed as not (yet) supported.

Thanks, Michael

@AaronRobinsonMSFT
Copy link
Member

generating cross-platform PE files and PDB files is not as simple as generating Windows-only binaries

No, this is easily handled by Roslyn and the runtime can handle this as well. Windows PDBs (non-portable) are hard, but that is why Portable PDBs exist.

But apparently this is not the case. So what am I missing?

The original was deep within the runtime with a lot of unmanaged code. This is a complete rewrite in pure managed so that it can be independent of the runtime implementation and shared with other runtimes (i.e., NativeAOT mono etc).

Also note that in .NET 8 the goal was for an MVP see the original announcement - #62956 (comment).

We have completed our goal to add MVP (Minimum Viable Product) support in .NET 8

This is not ready for everyone to start using as if it is full support in .NET 8. It was meant as a friendly update on status with a brief outline of current support. The goal is to have the ability to generate arbitrary .NET Assemblies in a similar way in .NET Core, but that isn't the current level as it is a full rewrite and reconciliation of how an assembly is generated.

There will be slight API changes for all the reasons mentioned in #62956 (comment).

@MichaelKetting
Copy link
Contributor

@MSDN-WhiteKnight

This expectation can't be met. It is already decided that API surface is different, the new API takes additional parameter for core assembly, while .NET Framework API always implied the current implementation's core assembly.

Okay, that's not a problem. Probably should have been a bit more highlevel with "expect". What I intended to express was, that there's a way similiar to AssemblyBuilder.Save(...) that allows persisting the runtime generated assembly including methods with their bodies. And for added value, it would also be great to support the generation Debug symbols.

@masonwheeler
Copy link
Contributor

@MichaelKetting

Do you mean the features I mentioned or the features discussed in the announcement post?

The latter.

@AaronRobinsonMSFT

The original was deep within the runtime with a lot of unmanaged code.

OK, that would do it. Thanks for clarifying!

@MichaelKetting
Copy link
Contributor

@AaronRobinsonMSFT and now I'm coming full circle and I'm terribly sorry for repeating the same question. I'll try super simple / unabiguous ones:

  1. Can we use AssemblyBuilder right now to persist runtime generated types, including their method bodies? (from reading Support equivalent of AssemblyBuilder.Save to save in-memory IL to an assembly #62956 (comment), the answer would be "no")
  2. If the answer to 1) is "no", is it planned for RC of .NET 8 and is there something I should try as a preview via reflection to give feedback?
  3. If the answer to 2) is "no", do you have a milestone for it that are are allowed to communicate?

Thanks, Michael

@masonwheeler
Copy link
Contributor

If the answer to 1) is "no", is it planned for RC of .NET 8 and is there something I should try as a preview via reflection to give feedback?

I'll take this one step further. I have relevant experience in implementing stuff like this. Is there any way I can contribute to get this into .NET 8? The community's been waiting 7 years for this feature already. If I can keep from adding another whole year onto that total, I'd like to.

@AaronRobinsonMSFT
Copy link
Member

  1. Can we use AssemblyBuilder right now to persist runtime generated types, including their method bodies? (from reading Support equivalent of AssemblyBuilder.Save to save in-memory IL to an assembly #62956 (comment), the answer would be "no")

No.

  1. If the answer to 1) is "no", is it planned for RC of .NET 8 and is there something I should try as a preview via reflection to give feedback?

Sadly no. It took a substantial amount of time to detangle the current implementation and we are not up against the code complete time frame.

  1. If the answer to 2) is "no", do you have a milestone for it that are are allowed to communicate?

This is likely a .NET 9 deliverable at the moment. There just hasn't been enough time for the team to make progress on it and handle the Reflection related work in flight.

I'll take this one step further. I have relevant experience in implementing stuff like this. Is there any way I can contribute to get this into .NET 8?

@masonwheeler I don't think we have the time this release, but I'll let @buyaa-n @steveharter or @jkotas comment specifically on whether help could change that calculus.

@MichaelKetting
Copy link
Contributor

@AaronRobinsonMSFT Thank you for the answers! To me, the details on the updated milestones are very helpful :)

@buyaa-n
Copy link
Contributor

buyaa-n commented Jul 21, 2023

Sorry seems my update was not clear about remaining work will not happen in .NET 8. I had mentioned it in the description:
Remaining work that out of .NET 8.0:

Remaining work that out of .NET 8.0:

  • Implement remaining parts needed for AssemblyBuilder.Save(string/stream) (mostly will rely on community contributions)
    • ILGenerator implementation
    • Add support for remaining members
      • Add ConstructorBuilderImpl and save it to file/stream (this needs minimal IL support)
      • Add PropertyBuilderImpl and save it to file/stream (this also needs minimal IL support)
      • Add EventBuildterImpl and save it to file/stream
      • Add implementation for remaining APIs for a MethodBuilder/TypeBuilder/FieldBuilder (like SignatureCallingConvention, *RequiredCustomModifiers, *OptionalCustomModifiers)
    • Have an API for creating PDBs and implement it
    • Entry point support

The above will be planned for .NET 9

I'll take this one step further. I have relevant experience in implementing stuff like this. Is there any way I can contribute to get this into .NET 8? The community's been waiting 7 years for this feature already. If I can keep from adding another whole year onto that total, I'd like to.

Thanks @masonwheeler, bare estimate of remaining work is listed above. You are welcome to contribute to any of those, though most of these needs IL support. Now we don't have a resource to put on this effort within .NET 8 timeframe. Therefore, I don't believe all of these could finish within .NET 8 timeframe with community contributions. Though your contribution could help the feature complete successfully in .NET 9

@masonwheeler
Copy link
Contributor

@buyaa-n Where in the repo would I find the relevant work-in-progress?

Also, are you guys aware of ILPack, which implements most of this already? Have you looked at it and found any good reasons why its code couldn't be pulled into the Core codebase? (I've tried using it and found it to be an imperfect replacement for AssemblyBuilder.Save. It doesn't do everything, but it gets you about 85% of the way there.)

@buyaa-n
Copy link
Contributor

buyaa-n commented Jul 21, 2023

@buyaa-n Where in the repo would I find the relevant work-in-progress?

This issue is for tracking all relevant work and you can see relevant PRs linked to this issue. The last PR was #88503

We are aware of ILPack and it is saves Assembly not AssemblyBuilder, Module not ModuleBuilder etc there is some similarity but not directly usable.

@leppie
Copy link
Contributor

leppie commented Jul 21, 2023

@buyaa-n thanks for the contribution.

I think most people here are just interested in dumping runtime generated code. This does not mean the code has to to run when reloaded. We have the tools to inspect the IL. While falling back to .NET desktop is generally good enough, there are cases, it makes it impossible to debug codegen on .NET core.

So, is it possible to dump some (possible malformed) IL to a stream for analysis?

@MSDN-WhiteKnight
Copy link
Contributor

So, is it possible to dump some (possible malformed) IL to a stream for analysis?

From within the process that generated it, or outside? If the latter, ClrMD could be used to inspect dynamic modules from external process, it represents them as ClrModule with IsDynamic property set to true. I have my project CIL View that attempts to implement viewing dynamic assemblies from external process, but it does not handle everything and it is not tested with newer CLR versions (only tested with .NET Core 3.1 as of now).

@buyaa-n
Copy link
Contributor

buyaa-n commented Jul 26, 2023

So, is it possible to dump some (possible malformed) IL to a stream for analysis?

We could consider adding a bare minimum (or empty) IL support within .NET 8 RC1 period (by August 14th). If a community willing to contribute, else currently we don't have a resource to work on it.

@buyaa-n
Copy link
Contributor

buyaa-n commented Aug 9, 2023

Closing the issue, we will open a new issue(s) for remaining work for .NET 9 planning.

@buyaa-n buyaa-n closed this as completed Aug 9, 2023
@markusschaber
Copy link

turn string into compiled program

use appropriate tool (Roslyn API)

How does the Roslyn API help with IronPython? (See IronLanguages/ironpython2#351)

As far as I know, the Rosyln API is rather centric to C#, and does not support the different interpretations of the DLR needed by Python.

@markusschaber
Copy link

Closing the issue, we will open a new issue(s) for remaining work for .NET 9 planning.

Could you link those new issues here, so we can follow them? Thanks!

@buyaa-n
Copy link
Contributor

buyaa-n commented Aug 15, 2023

Could you link those new issues here, so we can follow them? Thanks!

I will, when the issue(s) created.

@ForNeVeR
Copy link
Contributor

ForNeVeR commented Aug 15, 2023

My dear friends, I am extremely happy that this issue gets some traction, but I feel like it is getting a little bit mishandled.

Perhaps new issues should've been created before we close the root one, the one that got a fair number of upvotes and people actually watching it?

And also, perhaps the issue that asks for a working AssemblyBuilder.Save() should only be closed after we get a working AssemblyBuilder.Save() in the public API? Even if we ignore the fact that the current solution doesn't yet cover all the use cases, it is only callable via reflection. While perfectly okay as a temporary measure, it shouldn't be considered as the target here.

@buyaa-n
Copy link
Contributor

buyaa-n commented Aug 15, 2023

Perhaps new issues should've been created before we close the root one, the one that got a fair number of upvotes and people actually watching it?

And also, perhaps the issue that asks for a working AssemblyBuilder.Save() should only be closed after we get a working AssemblyBuilder.Save() in the public API? Even if we ignore the fact that the current solution doesn't yet cover all the use cases, it is only callable via reflection. While perfectly okay as a temporary measure, it shouldn't be considered as the target here.

FYI the root issue is still open: #15704, this was only for tracking the work for 8.0

@ForNeVeR
Copy link
Contributor

Ah, I'm sorry, my bad then. Thanks for your work!

@ghost ghost locked as resolved and limited conversation to collaborators Sep 15, 2023
@buyaa-n
Copy link
Contributor

buyaa-n commented Nov 17, 2023

Could you link those new issues here, so we can follow them? Thanks!

UPDATE: we are making progress here, see the Tracking issue for detail

@buyaa-n
Copy link
Contributor

buyaa-n commented Jan 3, 2024

UPDATE: we are making a good progress:

  • Added ILGenerator implementation
  • Added support for remaining members
    • ConstructorBuilderImpl
    • PropertyBuilderImpl
    • EventBuilderImpl
  • Added implementations for other unimplemented APIs in AssemblyBuilderImpl, TypeBuilderImp, MethodBuilderImpl
    • Still need to add a few APIs like CreateGlobalFuntions, DefineGlobalMethod etc. The work is in progress

Planning to finish the main functionality (excluding entry point and PDB) and make related APIs public in preview 1. Here is the tracking issue for details: #92975.

Now it would be great to test the implementation with a real-life scenario. I would really appreciate if you could:

  • Prepare a simple app with your use case and share with me (in this issue or email me)
  • Provide a sample assembly similar what your app generates, we can use for testing.
  • Dogfood the daily build and try out the new AssemblyBuilder implementation yourself and give feedback. (The APIs are still internal so need to use reflection for now as mentioned in the sample). As this issue is closed please add your feedback in the tracking issue

Thank you in advance!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Reflection.Emit Cost:L Work that requires one engineer up to 4 weeks Priority:2 Work that is important, but not critical for the release Team:Libraries tenet-compatibility Incompatibility with previous versions or .NET Framework User Story A single user-facing feature. Can be grouped under an epic.
Projects
None yet
Development

No branches or pull requests