Skip to content

Refactor MachObjectFile signature blobs #116566

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

Merged
merged 2 commits into from
Jun 12, 2025

Conversation

jtschuster
Copy link
Member

Decided to split up #115800 into a refactor PR with no behavioral changes and a separate PR with the behavior changes. Unfortunately this still ended up being pretty large, but much of the diff is moving code or forwarding method calls in a wrapper.

This PR makes the following changes

  • Uses classes instead of structs for blobs, with methods to read and write them from/to a file.
  • Create IBlob interface as the common "Blob" base.
  • Remove MachObjectFile.CodeSignature, and replace its usage with EmbeddedSignatureBlob.
  • Create IMachFileAccess to abstract reading and writing to disk. There are classes that implement reading and writing to a Stream or MemoryMappedViewAccessor. This allows the blobs to be easily written to a memory mapped file, or a MemoryStream for hashing.

@github-actions github-actions bot added the area-HostModel Microsoft.NET.HostModel issues label Jun 11, 2025
Copy link
Contributor

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

- Use classes instead of structs for blobs
- Create IBlob interface for blob writing
- Remove MachObjectFile.CodeSignature
- Create IMachFileAccess to abstract reading and writing to disk
@jtschuster jtschuster force-pushed the MachOSignatureRefactor branch from 095fc30 to df505ca Compare June 12, 2025 16:08
@jtschuster jtschuster marked this pull request as ready for review June 12, 2025 18:12
Copy link
Contributor

@Copilot Copilot AI left a 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 PR refactors the handling of Mach-O signature blobs by replacing structs with classes and introducing a common IBlob interface, while also consolidating and removing redundant code files. Key changes include:

  • Converting blob representations to classes with read/write methods.
  • Introducing the IBlob interface and reorganizing blob types.
  • Abstracting file access via the new IMachOFileAccess interface.

Reviewed Changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
FileUtils/EndianConversionExtensions.cs Adds ulong conversion method for consistency.
Enums/HashType.cs Adds HashTypeExtensions for creating hash algorithms and obtaining hash sizes.
Enums/CodeDirectorySpecialSlot.cs Updates comment with a reference link.
Enums/BlobMagic.cs Adjusts ordering of enum members without altering underlying values.
BinaryFormat/SymbolTableLoadCommand.cs Renames the struct for clarity and adjusts default check accordingly.
BinaryFormat/LinkEditLoadCommand.cs Renames the struct and updates the constructor and default check.
BinaryFormat/Blobs/SuperBlob.cs Introduces SuperBlob for managing composite blobs and validates blob offsets.
BinaryFormat/Blobs/SimpleBlob.cs Implements a simple blob type for binary read/write operations.
BinaryFormat/Blobs/RequirementsBlob.cs Wraps a SuperBlob to represent an empty Requirements blob.
BinaryFormat/Blobs/EmbeddedSignatureBlob.cs Implements an EmbeddedSignature blob that aggregates sub-blobs.
BinaryFormat/Blobs/CodeDirectoryBlob.cs Implements a CodeDirectoryBlob with header parsing and blob writing logic.
BinaryFormat/Blobs/CmsWrapperBlob.cs Implements a CmsWrapperBlob using a SimpleBlob as inner storage.
BinaryFormat/Blobs/BlobParser.cs Provides logic to parse a blob based on its magic number.
BinaryFormat/Blobs/BlobIndex.cs Adds a constant for blob index size.
AppHost/HostWriter.cs Updates Mach-O file access to use the new IMachOFileAccess interface.

Copy link
Member

@agocke agocke 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!

@jtschuster
Copy link
Member Author

I'm going to go ahead and merge. @elinor-fung I'll apply any review feedback to the follow up PR.

@jtschuster jtschuster merged commit e7c1960 into dotnet:main Jun 12, 2025
76 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 13, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-HostModel Microsoft.NET.HostModel issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants