-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Add support for emitting Mach-O R2R images #121186
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
base: main
Are you sure you want to change the base?
Conversation
…PE writer, don't do that for other formats where we don't have the PE Optional Header.
… little different).
…y only one valid way to write the symbol range
179146b to
9a08c72
Compare
|
/azp run runtime-ioslike |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-ioslike |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
|
/azp run runtime-ioslike |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-ioslike |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-ioslike |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
This is ready for review, but we might need to turn off the MachO emit to merge this to avoid breaking the perf pipeline, not sure. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request adds support for producing ReadyToRun (R2R) images in Mach-O format for Apple mobile platforms, in addition to the existing PE format support. The key purpose is to enable native code loading on Apple mobile platforms where PE files cannot be loaded.
Key changes:
- Added a new
ReadyToRunContainerFormat.MachOenum value and infrastructure to support multiple container formats - Implemented Mach-O linking targets in MSBuild to convert crossgen2 output to dylib format
- Updated crossgen2 to produce Mach-O object files when targeting Apple platforms
- Modified section placement logic to handle format-specific requirements (e.g., managed code in
__TEXT,__managedcodefor Mach-O)
Reviewed Changes
Copilot reviewed 38 out of 39 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| RunReadyToRunCompiler.cs | Added Crossgen2ContainerFormat property to pass container format to crossgen2 |
| PrepareForReadyToRunCompilation.cs | Handles post-processing for Mach-O format, tracking intermediate and final output paths |
| Microsoft.NET.CrossGen.targets | Added _LinkReadyToRunMachO target and Mach-O toolchain discovery logic |
| MetadataKeys.cs | Added metadata keys for native linking requirements |
| Microsoft.NETCore.App.Runtime.CoreCLR.sfxproj | Configured Mach-O format for Apple mobile platforms |
| Directory.Build.props | Added platform manifest entry for composite R2R dylib |
| Resources.resx | Added error message for non-composite format validation |
| Program.cs | Added validation that non-PE formats require composite mode |
| Crossgen2RootCommand.cs | Added "macho" option to output format parser |
| ILCompiler.ReadyToRun.csproj | Renamed file reference for generalized symbol range support |
| ReadyToRunCodegenCompilationBuilder.cs | Passed format parameter to NodeFactory |
| ReadyToRunCodegenCompilation.cs | Added platform-native image flag logic and named parameters |
| ReadyToRunCodegenNodeFactory.cs | Added Format property and parameter to constructor |
| Win32ResourcesNode.cs | Made section placement format-dependent |
| SymbolNodeRange.cs | Generalized from DelayLoadMethodCallThunkNodeRange to support multiple range types |
| RuntimeFunctionsTableNode.cs | Made PData section usage conditional on PE format |
| RuntimeFunctionsGCInfoNode.cs | Made XData section usage conditional on PE format |
| MethodWithGCInfo.cs | Made TextSection usage conditional on PE format |
| MethodColdCodeNode.cs | Made TextSection usage conditional on PE format |
| ImportThunk.cs | Added ISortableSymbolNode interface and updated callback |
| CopiedStrongNameSignatureNode.cs | Made CorMeta section usage conditional on PE format |
| CopiedMethodILNode.cs | Made CorMeta section usage conditional on PE format |
| CopiedMetadataBlobNode.cs | Made CorMeta section usage conditional on PE format |
| CopiedManagedResourcesNode.cs | Made CorMeta section usage conditional on PE format |
| CopiedFieldRvaNode.cs | Made CorMeta section usage conditional on PE format |
| CopiedCorHeaderNode.cs | Made CorMeta section usage conditional on PE format |
| ReadyToRunObjectWriter.cs | Refactored to support multiple object writers via factory methods |
| ReadyToRunContainerFormat.cs | Added MachO enum value |
| ReadyToRunConstants.cs | Added READYTORUN_FLAG_PlatformNativeImage flag |
| ModuleHeaders.cs | Bumped R2R minor version to 16.1 |
| UnixObjectWriter.cs | Added outputInfoBuilder parameter to constructor |
| ObjectWriter.cs | Refactored symbol range emission to support format-specific implementations |
| MachObjectWriter.cs | Implemented Mach-O specific symbol range handling and relocations |
| ModuleHeaders.h | Bumped R2R minor version to 16.1 |
| readytorun.h | Bumped R2R minor version and added READYTORUN_FLAG_PLATFORM_NATIVE_IMAGE flag |
| crossgen-corelib.proj | Refactored to support Mach-O linking for Apple mobile platforms |
| library-servicing.md | Fixed trailing whitespace |
| readytorun-platform-native-envelope.md | Added comprehensive documentation of Mach-O design decisions |
| readytorun-format.md | Documented READYTORUN_FLAG_PLATFORM_NATIVE_IMAGE flag |
Comments suppressed due to low confidence (1)
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/SymbolNodeRange.cs:13
- Corrected spelling of 'machanism' to 'mechanism'.
.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Show resolved
Hide resolved
| <PlatformManifestFileEntry Include="System.Private.Xml.dll" /> | ||
| <PlatformManifestFileEntry Include="System.Private.Xml.Linq.dll" /> | ||
| <!-- Platform-Native Composite R2R images --> | ||
| <PlatformManifestFileEntry Include="Microsoft.NETCore.App.Runtime.ios-arm64.r2r.dylib" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to have the RID in the filename?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll look into why the default naming targets added the RID (the name is calculated by the .NET SDK targets).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found it: It's from the Shared Framework SDK. When I onboarded that SDK, we needed to set some fake assembly name as the "Main assembly". I picked the package name. As we were not using composite mode at the time, this name didn't show up anywhere. With this PR it does. I'll change this in the SharedFramework SDK to not have the RID.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix at dotnet/arcade#16276. After that PR, the file will be Microsoft.NETCore.App.r2r.dylib.
|
/azp run runtime-ioslike |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-ioslike |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Adds support in crossgen2 for emitting Mach-O objects.
Does not yet do the runtime support as that depends on #120777