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

Skip BA2021 Analysis on .NET R2R & NativeAOT PE on non-Windows Platforms #1013

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

steveisok
Copy link

This change skips analysis when it finds non-Windows .NET R2R & NativeAOT PE's. The reason for this is the DoNotMarkWritableSectionsAsExecutable is Windows specific and R2R/NativeAOT do not follow Windows layout rules on non-Windows platforms.

Fixes #970

This change skips analysis when it finds non-Windows .NET R2R & NativeAOT PE's. The reason for this is the DoNotMarkWritableSectionsAsExecutable
is Windows specific and R2R/NativeAOT do not follow Windows layout rules on non-Windows platforms.

Fixes microsoft#970
@steveisok
Copy link
Author

@michaelcfanning @marmegh when either of you have a moment, can you please review?

@AllDwarf AllDwarf requested review from AllDwarf and a team and removed request for marmegh October 16, 2024 15:04
@AllDwarf
Copy link
Collaborator

Basically once there will be tests it's good to go. What would be nice to have some files for testing it locally, can you provide them please? @steveisok cc @mkacmar

@AllDwarf AllDwarf added this to the Release v4.3.2 milestone Jan 24, 2025
@steveisok
Copy link
Author

steveisok commented Feb 14, 2025

Basically once there will be tests it's good to go. What would be nice to have some files for testing it locally, can you provide them please? @steveisok cc @mkacmar

@AllDwarf I think I amended the correct test and provided an artifact. If there's more I need to do, please let me know. #Resolved

@michaelcfanning
Copy link
Member

Two easy suggestions, move your non windows helper into pe.cs and double-check your list of windows machine values to ensure they cover all legacy generated PE's not just today's relevant machine set.


In reply to: 2659942241

@steveisok steveisok force-pushed the fix-machine-only-target-windows branch from 5b0ad55 to d54a955 Compare February 18, 2025 20:59
Copy link
Member

@michaelcfanning michaelcfanning left a comment

Choose a reason for hiding this comment

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

🕐

Copy link
Member

@michaelcfanning michaelcfanning left a comment

Choose a reason for hiding this comment

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

:shipit:

@steveisok
Copy link
Author

@michaelcfanning I don't have the power to merge. If CI looks ok, can you do that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Combability with .NET ReadyToRun and Self-Contained
3 participants