Skip to content

Commit

Permalink
i#5383: Fix macOS preload on a64 (#7170)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ndrewh authored Jan 16, 2025
1 parent 6d1e544 commit 359868c
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 40 deletions.
90 changes: 59 additions & 31 deletions core/arch/aarch64/proc.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,44 @@
#include "arch.h"
#include "instr.h"

#if defined(MACOS)
# include <sys/sysctl.h>
#endif

static int num_simd_saved;
static int num_simd_registers;
static int num_svep_registers;
static int num_ffr_registers;
static int num_opmask_registers;

#define GET_FEAT_REG(FEATURE) (feature_reg_idx_t)((((ushort)FEATURE) & 0x3F00) >> 8)
#define GET_FEAT_NIBPOS(FEATURE) ((((ushort)FEATURE) & 0x00F0) >> 4)
#define GET_FEAT_VAL(FEATURE) (((ushort)FEATURE) & 0x000F)
#define GET_FEAT_NSFLAG(FEATURE) ((((ushort)FEATURE) & 0x8000) >> 15)
#define GET_FEAT_EXACT_MATCH(FEATURE) ((((ushort)FEATURE) & 0x4000) >> 14)

void
proc_set_feature(feature_bit_t feature_bit, bool enable)
{
uint64 *freg_val = cpu_info.features.isa_features;
ushort feat_nibble = GET_FEAT_NIBPOS(feature_bit);
bool feat_nsflag = GET_FEAT_NSFLAG(feature_bit);
uint64 feat_val = GET_FEAT_VAL(feature_bit);

feature_reg_idx_t feat_reg = GET_FEAT_REG(feature_bit);
freg_val += feat_reg;

/* Clear the current feature state. */
*freg_val &= ~(0xFULL << (feat_nibble * 4));
if (enable) {
/* Write the feature value into the feature nibble. */
*freg_val |= feat_val << (feat_nibble * 4);
} else if (feat_nsflag) {
/* If the not-set flag is 0xF, then that needs manually setting. */
*freg_val |= 0xF << (feat_nibble * 4);
}
}

#ifndef DR_HOST_NOT_TARGET

# define NUM_FEATURE_REGISTERS (sizeof(features_t) / sizeof(uint64))
Expand Down Expand Up @@ -86,7 +118,7 @@ read_feature_regs(uint64 isa_features[])
: "x0");
}

# if !defined(MACOS) // TODO i#5383: Get this working on Mac. */
# if !defined(MACOS)
static void
get_processor_specific_info(void)
{
Expand Down Expand Up @@ -146,6 +178,32 @@ get_processor_specific_info(void)
dr_set_vector_length(256);
# endif
}
# else /* defined(MACOS) */

/* On macOS, MRS appears to be restricted. We'll use sysctl's instead.
* XXX i#5383: Add remaining features from other sysctls.
*/
static void
get_processor_specific_info(void)
{
memset(&cpu_info.features, 0, sizeof(cpu_info.features));

# define SET_FEAT_IF_SYSCTL_EQ(FEATURE, SYSCTL, TY, VAL) \
TY FEATURE##_tmp; \
size_t FEATURE##_tmp_buflen = sizeof(FEATURE##_tmp); \
if (sysctlbyname(SYSCTL, &FEATURE##_tmp, &FEATURE##_tmp_buflen, NULL, 0) == \
-1) { \
ASSERT_CURIOSITY(false && SYSCTL " sysctl failed"); \
SYSLOG_INTERNAL_WARNING("Failed to read " SYSCTL " sysctl"); \
} else if (FEATURE##_tmp_buflen == sizeof(FEATURE##_tmp)) { \
if (FEATURE##_tmp == VAL) { \
proc_set_feature(FEATURE, true); \
} \
}

SET_FEAT_IF_SYSCTL_EQ(FEATURE_PAUTH, "hw.optional.arm.FEAT_PAuth", uint32_t, 1);
SET_FEAT_IF_SYSCTL_EQ(FEATURE_FPAC, "hw.optional.arm.FEAT_FPAC", uint32_t, 1);
}
# endif

# define LOG_FEATURE(feature) \
Expand All @@ -172,7 +230,6 @@ proc_init_arch(void)
}

#ifndef DR_HOST_NOT_TARGET
# if !defined(MACOS) // TODO i#5383: Get this working on Mac. */
get_processor_specific_info();

DOLOG(1, LOG_TOP, {
Expand Down Expand Up @@ -247,38 +304,9 @@ proc_init_arch(void)
cpu_info.features.isa_features[AA64MMFR2]);
LOG_FEATURE(FEATURE_LSE2);
});
# endif
#endif
}

#define GET_FEAT_REG(FEATURE) (feature_reg_idx_t)((((ushort)FEATURE) & 0x3F00) >> 8)
#define GET_FEAT_NIBPOS(FEATURE) ((((ushort)FEATURE) & 0x00F0) >> 4)
#define GET_FEAT_VAL(FEATURE) (((ushort)FEATURE) & 0x000F)
#define GET_FEAT_NSFLAG(FEATURE) ((((ushort)FEATURE) & 0x8000) >> 15)
#define GET_FEAT_EXACT_MATCH(FEATURE) ((((ushort)FEATURE) & 0x4000) >> 14)

void
proc_set_feature(feature_bit_t feature_bit, bool enable)
{
uint64 *freg_val = cpu_info.features.isa_features;
ushort feat_nibble = GET_FEAT_NIBPOS(feature_bit);
bool feat_nsflag = GET_FEAT_NSFLAG(feature_bit);
uint64 feat_val = GET_FEAT_VAL(feature_bit);

feature_reg_idx_t feat_reg = GET_FEAT_REG(feature_bit);
freg_val += feat_reg;

/* Clear the current feature state. */
*freg_val &= ~(0xFULL << (feat_nibble * 4));
if (enable) {
/* Write the feature value into the feature nibble. */
*freg_val |= feat_val << (feat_nibble * 4);
} else if (feat_nsflag) {
/* If the not-set flag is 0xF, then that needs manually setting. */
*freg_val |= 0xF << (feat_nibble * 4);
}
}

void
enable_all_test_cpu_features()
{
Expand Down
2 changes: 0 additions & 2 deletions core/iox.h
Original file line number Diff line number Diff line change
Expand Up @@ -706,9 +706,7 @@ TNAME(d_r_vsnprintf)(TCHAR *s, size_t max, const TCHAR *fmt, va_list ap)
c++;
} else {
const TCHAR *cstart = c;
int nbytes = 0;
while (*c && *c != _T('%')) {
nbytes++;
c++;
}
while (cstart < c) {
Expand Down
4 changes: 3 additions & 1 deletion core/unix/os.c
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,7 @@ static init_fn_t
#else
/* If we're a normal shared object, then we override _init.
*/
int
INITIALIZER_ATTRIBUTES int
_init(int argc, char **argv, char **envp)
{
# ifdef ANDROID
Expand Down Expand Up @@ -1143,6 +1143,8 @@ get_application_name_helper(bool ignore_cache, bool full_path)
#else
/* OSX kernel puts full app exec path above envp */
char *c, **env = our_environ;
ASSERT(our_environ != NULL &&
"our_environ is not set in get_application_name_helper");
do {
env++;
} while (*env != NULL);
Expand Down
10 changes: 10 additions & 0 deletions core/unix/os_public.h
Original file line number Diff line number Diff line change
Expand Up @@ -242,4 +242,14 @@ typedef kernel_sigcontext_t sigcontext_t;
# define SC_RETURN_REG SC_A0
#endif /* X86/ARM */

/* XXX i#5383: macOS 14.4 on ARM64 _init does not seem to get called
* without __attribute__((constructor)) but it inexplicably breaks tests if
* we add this flag on X86. So we add it only on ARM64 for now.
*/
#if defined(MACOS) && defined(AARCH64)
# define INITIALIZER_ATTRIBUTES __attribute__((constructor))
#else
# define INITIALIZER_ATTRIBUTES
#endif

#endif /* _OS_PUBLIC_H_ 1 */
3 changes: 2 additions & 1 deletion core/unix/preload.c
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
#include "configure.h"
#include "globals_shared.h"
#include "../config.h"
#include "os_public.h"
#include <stdio.h>
/* for getpid */
#include <unistd.h>
Expand Down Expand Up @@ -137,7 +138,7 @@ take_over(const char *pname)
return true;
}

int
INITIALIZER_ATTRIBUTES int
#if INIT_BEFORE_LIBC
_init(int argc, char *arg0, ...)
{
Expand Down
26 changes: 22 additions & 4 deletions core/unix/signal_macos.c
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,17 @@ sigcontext_to_mcontext_simd(priv_mcontext_t *mc, sig_full_cxt_t *sc_full)
return;
mc->fpsr = fpc->__fpsr;
mc->fpcr = fpc->__fpcr;
ASSERT(sizeof(mc->simd) == sizeof(fpc->__v));
memcpy(&mc->simd, &fpc->__v, sizeof(mc->simd));
if (proc_has_feature(FEATURE_SVE)) {
/* XXX i#5383: SVE and SVE2 support for MACOS still missing.
*/
ASSERT_NOT_IMPLEMENTED(false);
} else {
/* ARM_NEON64 case.
*/
for (int i = 0; i < proc_num_simd_registers(); i++) {
memcpy(&mc->simd[i].q, &fpc->__v[i], sizeof(mc->simd->q));
}
}
#elif defined(X86)
/* We assume that _STRUCT_X86_FLOAT_STATE* matches exactly the first
* half of _STRUCT_X86_AVX_STATE*, and similarly for AVX and AVX512.
Expand Down Expand Up @@ -200,8 +209,17 @@ mcontext_to_sigcontext_simd(sig_full_cxt_t *sc_full, priv_mcontext_t *mc)
return;
fpc->__fpsr = mc->fpsr;
fpc->__fpcr = mc->fpcr;
ASSERT(sizeof(mc->simd) == sizeof(fpc->__v));
memcpy(&fpc->__v, &mc->simd, sizeof(mc->simd));
if (proc_has_feature(FEATURE_SVE)) {
/* XXX i#5383: SVE and SVE2 support for MACOS still missing.
*/
ASSERT_NOT_IMPLEMENTED(false);
} else {
/* ARM_NEON64 case.
*/
ASSERT((sizeof(mc->simd->q) * proc_num_simd_registers()) == sizeof(fpc->__v));
for (int i = 0; i < proc_num_simd_registers(); i++)
memcpy(&fpc->__v[i], &mc->simd[i].q, sizeof(fpc->__v[i]));
}
#elif defined(X86)
sigcontext_t *sc = sc_full->sc;
int i;
Expand Down
3 changes: 2 additions & 1 deletion ext/drx/drx.c
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@
# define IF_WINDOWS_ELSE(x, y) (y)
#endif

#if defined(X86) || defined(AARCH64)
/* XXX i#5383: PLATFORM_SUPPORTS_SCATTER_GATHER is broken on aarch64 macOS. */
#if (defined(X86) || defined(AARCH64)) && !(defined(MACOS) && defined(AARCH64))
# define PLATFORM_SUPPORTS_SCATTER_GATHER
#endif

Expand Down

0 comments on commit 359868c

Please sign in to comment.