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

i#5383: Fix macOS arm64 test build/run #7171

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ndrewh
Copy link
Contributor

@ndrewh ndrewh commented Dec 27, 2024

This makes tests build and run properly on macOS (14.4) arm64 (M3). Many tests do not pass, but you can at least run them now.

  • Disable selfmod tests due to build failures on M3
  • Fix macOS build for mangle_pauth test (test still fails for some reason)
  • Mark dump_ucontext as NYI on macOS
  • Adds -vm_size 500M to client tests on macOS ARM64 to avoid reachablity assert
  • Fixes NULL sigcontext_t in os_forge_exception (the mangle_pauth test was hitting this)

- Disable selfmod tests due to build failures on M3
- Fix macOS build for mangle_pauth test (test still fails for some reason)
- Mark dump_ucontext as NYI on macOS

- Fixes NULL sigcontext_t in os_forge_exception
- Adds -vm_size 500M to client tests to avoid reachablity assert
@ndrewh
Copy link
Contributor Author

ndrewh commented Dec 27, 2024

only a64 failure is raw-zlib #5635, doesn't seem to be my fault

@ndrewh ndrewh changed the title i5383: Fix macOS arm64 test build/run i#5383: Fix macOS arm64 test build/run Dec 27, 2024
Copy link
Contributor

@edeiana edeiana left a comment

Choose a reason for hiding this comment

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

Thank you for improving MacOS tests!

Please add "Issue #5383" at the bottom of your PR description.

@@ -6040,7 +6047,9 @@ if (NOT ARM) # FIXME i#1551: fix bugs on ARM
endif ()
endif (NOT ARM)

if (X86 OR AARCH64) # FIXME i#1551: port asm to ARM
# FIXME i#1551: port asm to ARM
# FIXME i#xxx: selfmod tests fail to build on macOS + ARM64
Copy link
Contributor

Choose a reason for hiding this comment

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

Change i#xxx with i#5383 (this falls within that issue).

/* Since SIGCXT_FROM_UCXT just accesses the uc->uc_mcontext ptr field on
* macOS, sc will be NULL below if we do not initialize uc_mcontext first
*/
uc->IF_X64_ELSE(uc_mcontext64, uc_mcontext) = (void *)&frame->mc;
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems weird.
frame is zeroed-out anyway, isn't this just changing sc to be a "different" NULL?
Also, shouldn't sc being NULL break more than MACOS?

Copy link
Contributor

Choose a reason for hiding this comment

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

frame is zeroed-out anyway,

frame is indeed zeroed out; which means frame->uc.uc_mcontext is NULL before L7717.

isn't this just changing sc to be a "different" NULL?

L7717 seems to set frame->uc.uc_mcontext (which was NULL before) to &frame->mc; the address of mc within the frame is not NULL.

I'm not familiar with these Mac structs. I wonder if it is simply a requirement for frame->uc.uc_mcontext to point to frame->mc.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ndrewh The comment at L7714 justifies that uc->IF_X64_ELSE(uc_mcontext64, uc_mcontext) must be set to something, but it doesn't explain why &frame->mc is the correct intended value. Can you add that reasoning to the comment?

@edeiana edeiana requested a review from abhinav92003 January 7, 2025 19:46
# On macOS + arm64 tests will randomly fail depending on
# whether mmap returns a region satisfying 32-bit reachability constraints.
# Reducing the vm_size appears to eliminate this issue.
set(dr_test_ops -vm_size 500M ${dr_test_ops})
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be useful to file a new issue with details of failures you saw.

edeiana pushed a commit that referenced this pull request Jan 16, 2025
This fixes several build and runtime issues under macOS on ARM64 on
macOS 14.4.1 (Sonoma).

- `FEATURE_PAUTH` is now detected on macOS using a sysctl (capability
MSRs cannot be read in userspace on M3 at least).
- In the preload library and os.c, `_init` needs
`__attribute__((constructor))` to be run by dyld.
- `PLATFORM_SUPPORTS_SCATTER_GATHER` is broken, disabled for now
on a64 macOS.
- Fixed sigcontext/mcontext conversion which assumes old `simd` size and
was failing on an assert.

Known issues that remain:
- VM allocation frequently but nondeterministically fails reachability
constraints. `-vm_size 500M` seems to fix this. #7171 adds this flag to
tests on macOS+ARM64. Another option is to make this the default on
macOS+ARM64.
- Sometimes some bookkeeping asserts in `vmh_exit` are reached upon
process exit.

The end result is that the `bbcount` tool works on a simple toy program
on macOS, in debug mode, without hitting any asserts:
```
bin64/drrun -debug -vm_size 500M -c api/bin/libbbcount.dylib -- ./main
```

Issue #5383
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.

3 participants