Skip to content

Conversation

davidwrighton
Copy link
Member

@davidwrighton davidwrighton commented May 19, 2025

  • This logic always assumed that the lowest address of a mapped file was its "base address", but in the presence of Create a single copy of stub templates runtime#114462 that is no longer correct. We may map stubs at lower addresses.
  • Fix by checking the start of the mapped region for the magic value for the start of a MachO binary

- This logic always assumed that the lowest address of a mapped file was its "base address", but in the presence of #114462 that is no longer correct. We may map stubs at lower addresses.
- Fix by checking the start of the mapped region for the magic value for the start of a MachO binary
@davidwrighton davidwrighton requested a review from a team as a code owner May 19, 2025 21:28
thaystg
thaystg previously approved these changes May 19, 2025
ssize_t bytesRead = read(fd, &magic, sizeof(magic));
if (bytesRead == sizeof(magic))
{
if (magic == 0xFEEDFACF)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (magic == 0xFEEDFACF)
if (magic == 0xFEEDFACF || magic == 0xFEEDFACE)

@hoyosjs
Copy link
Member

hoyosjs commented May 20, 2025

Looks like a few of the mappings have 0-offset entries:

/usr/local/share/dotnet/shared/Microsoft.NETCore.App/8.0.5/System.Threading.dll
 address: 1091d0000 offset: 0
 protection: 1, max_protection: 7
 inheritance: 1
/usr/local/share/dotnet/shared/Microsoft.NETCore.App/8.0.5/System.Threading.dll
 address: 10920c000 offset: 0
 protection: 3, max_protection: 7
 inheritance: 1
/usr/local/share/dotnet/shared/Microsoft.NETCore.App/8.0.5/System.Threading.dll
 address: 109230000 offset: 10000
 protection: 1, max_protection: 7
 inheritance: 1
usr/local/share/dotnet/shared/Microsoft.NETCore.App/8.0.5/libcoreclr.dylib
 address: 1054c4000 offset: 0
 protection: 5, max_protection: 7
 inheritance: 1
/usr/local/share/dotnet/shared/Microsoft.NETCore.App/8.0.5/libcoreclr.dylib
 address: 10596c000 offset: 0
 protection: 1, max_protection: 7
 inheritance: 1
/usr/local/share/dotnet/shared/Microsoft.NETCore.App/8.0.5/libcoreclr.dylib
 address: 10598c000 offset: 0
 protection: 3, max_protection: 7
 inheritance: 1
/usr/local/share/dotnet/shared/Microsoft.NETCore.App/8.0.5/libcoreclr.dylib
 address: 1059b8000 offset: 4d0000
 protection: 1, max_protection: 7
 inheritance: 1

And attaching with a debugger - only the first one has the machO magic, despite all claiming offset 0.

@hoyosjs
Copy link
Member

hoyosjs commented May 20, 2025

yeah - we considered this as a "next possible thing". Remote reads just come with their own cost. We have similar logic in: https://github.com/dotnet/diagnostics/blob/main/src/Microsoft.FileFormats/MachO/MachOFatHeaderStructures.cs#L29

@hoyosjs hoyosjs merged commit 84b028b into dotnet:main May 22, 2025
21 checks passed
@hoyosjs
Copy link
Member

hoyosjs commented May 22, 2025

Note in case this bites us in the future: flags isn't the most reliable here. There's the "cheaper" way of checking the remote bytes for the magic. The other option is using vm_* APIs to recognize the __TEXT section load.

@github-actions github-actions bot locked and limited conversation to collaborators Jun 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants