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] Do not strip exported symbols from executables on Apple platforms #85293

Merged
merged 2 commits into from
Apr 25, 2023

Conversation

ivanpovazan
Copy link
Member

@ivanpovazan ivanpovazan commented Apr 25, 2023

StripSymbols project property is now set by default when building with NativeAOT: #85139

However, this breaks a special use case on Apple platforms when an executable dynamically looks-up symbols in the current executable image (self-referencing) reported here: #85139 (comment)

This PR:

  • fixes this issue by adding a StripFlag for this special use case, which will ignore exported symbols when stripping the executable
  • reenables stripping in the unit test in question
  • renames: StripFlag into _StripFlag as it is an internal property

cc: @kotlarmilos

@ghost
Copy link

ghost commented Apr 25, 2023

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

Issue Details

StripSymbols project property is now set by default when building with NativeAOT: #85139

However, this breaks a special use case on Apple platforms when an executable dynamically looks-up symbols in the current executable image (self-referencing) reported here: #85139 (comment)

This PR:

  • fixes this issue by adding a StripFlag for this special use case, which will ignore exported symbols when stripping the executable
  • reenables stripping in the unit test in question

cc: @kotlarmilos

Author: ivanpovazan
Assignees: ivanpovazan
Labels:

area-NativeAOT-coreclr

Milestone: -

Copy link
Member

@am11 am11 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Copy link
Member

@kotlarmilos kotlarmilos left a comment

Choose a reason for hiding this comment

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

LGTM!

@ivanpovazan ivanpovazan requested a review from jkotas April 25, 2023 13:58
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thanks!

@jkotas jkotas merged commit 32ecd85 into dotnet:main Apr 25, 2023
jkotas added a commit to jkotas/runtime that referenced this pull request May 1, 2023
jkotas added a commit that referenced this pull request May 4, 2023
@ghost ghost locked as resolved and limited conversation to collaborators May 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants