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

NativeAOT exports marked functions for exe apps. #78738

Merged
merged 4 commits into from
Nov 23, 2022

Conversation

JamesMenetrey
Copy link
Contributor

Dear all,

This PR fixes the behavior observed in #78663.

To summarize the issue, functions decorated with the attribute UnmanagedCallersOnlyAttribute are not inserted in the export table of the compiled file when the .NET project is an executable (i.e., as opposed to a library) when compiled with NativeAOT.

This change removes the difference between a library and an executable regarding the handling of the attribute UnmanagedCallersOnlyAttribute.

As such, decorated functions with this attribute are now adequately exported for libraries and executables.

Functions decorated with the attribute UnmanagedCallersOnlyAttribute
are not inserted in the export table of the compiled file when the
.NET project is an executable (i.e., as opposed to a library).
This change removes the difference between a library and an executable
regarding the handling of the attribute UnmanagedCallersOnlyAttribute.
As such, decorated functions with this attribute are now adequately
exported for libraries and executables.

Fix dotnet#78663
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Nov 22, 2022
@ghost
Copy link

ghost commented Nov 22, 2022

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Dear all,

This PR fixes the behavior observed in #78663.

To summarize the issue, functions decorated with the attribute UnmanagedCallersOnlyAttribute are not inserted in the export table of the compiled file when the .NET project is an executable (i.e., as opposed to a library) when compiled with NativeAOT.

This change removes the difference between a library and an executable regarding the handling of the attribute UnmanagedCallersOnlyAttribute.

As such, decorated functions with this attribute are now adequately exported for libraries and executables.

Author: JamesMenetrey
Assignees: -
Labels:

community-contribution, area-NativeAOT-coreclr

Milestone: -

@JamesMenetrey
Copy link
Contributor Author

FYI @agocke, @jkotas, @MichalStrehovsky.

@jkotas
Copy link
Member

jkotas commented Nov 22, 2022

I do not think this behavior should be enabled by default for .exe. And in case we end up enabling by default, there should be a way to disable it.

@JamesMenetrey
Copy link
Contributor Author

JamesMenetrey commented Nov 22, 2022

I do not think this behavior should be enabled by default for .exe. And in case we end up enabling by default, there should be a way to disable it.

The usage of the attribute UnmanagedCallersOnlyAttribute without setting the property EntryPoint does not export the symbol, according to the existing MSDN documentation of that property. Therefore, one can expect to see their functions exported when this property is set, regardless of the type of the project, which is not the case currently.

I think setting the property EntryPoint is a convenient way to opt-in for exporting the symbol of the marked functions. Otherwise, this means the usage of that property would have no effect when used in the context of an executable, which may be counter-intuitive for developers.

On the other hand, if you prefer an explicit opt-in (or opt-out) in the project file, I can think of adding a new tag for that particular purpose.

WDYT?

Copy link
Member

@MichalStrehovsky MichalStrehovsky left a comment

Choose a reason for hiding this comment

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

Thank you for the pull request! Jan's argument in #78663 (comment) convinced me that we shouldn't do this by default. How about this? You'll be able to set <IlcExportUnmanagedEntrypoints>true</IlcExportUnmanagedEntrypoints> to opt in. (I'm open to suggestions at naming this - we prefix properties that control not-quite-documented behaviors with Ilc so that part is motivated by that.)

Co-authored-by: Michal Strehovský <MichalStrehovsky@users.noreply.github.com>
@JamesMenetrey
Copy link
Contributor Author

Hello @jkotas, and @MichalStrehovsky, thanks for the review! This solution is also fine for me. :)

@MichalStrehovsky
Copy link
Member

The test failure is an unrelated test that got deadlettered. Infra issue.

@MichalStrehovsky MichalStrehovsky merged commit 78b3f9d into dotnet:main Nov 23, 2022
@JamesMenetrey
Copy link
Contributor Author

Thanks for the support over this PR!

@ghost ghost locked as resolved and limited conversation to collaborators Dec 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-NativeAOT-coreclr community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants