i#1979: steps towards building on MacOS 64-bit #2269

shawndenbow · 2017-03-07T01:23:28Z

This commit gets core DR building on MacOS 64-bit.
+ change error notifications to warning to allow build to continue
+ define SC_FIELD for 64-bit registers r8-r15 and update references
+ update dynamorio_mach_dep_syscall to use syscall for 64-bit
+ update dynamorio_mach_syscall to use syscall for 64-bit
+ change code referencing eflags to xflags for cross platform compat
+ misc code fix-ups

This commit gets core DR building on MacOS 64-bit. + change error notifications to warning to allow build to continue + define SC_FIELD for 64-bit registers r8-r15 and update references + update dynamorio_mach_dep_syscall to use syscall for 64-bit + update dynamorio_mach_syscall to use syscall for 64-bit + change code referencing eflags to xflags for cross platform compat + misc code fix-ups

derekbruening

I think the TLS read change needs to read a 64-bit value

derekbruening · 2017-03-07T02:28:08Z

core/arch/x86/x86.asm

 #  ifdef X64
+        mov      REG_XAX, QWORD [REG_XAX] /* load mach_synch_t->sem */


This can stay where it was, shared between the two, via s/DWORD/PTRSZ/

That's convenient. Done.

derekbruening · 2017-03-07T02:30:10Z

core/arch/x86/x86_shared.asm

@@ -42,6 +42,9 @@

 #include "../asm_defines.asm"
 #include "x86_asm_defines.asm" /* PUSHGPR, POPGPR, etc. */
+#ifdef MACOS
+# include <include/syscall_mach.h> /* SYSCALL_NUM_MARKER_MACH */


"" rather than <> as this is our include, not from /usr/include

derekbruening · 2017-03-07T02:44:41Z

core/unix/os.c

    ASSERT(is_thread_tls_initialized());
    DOCHECK(1, {
-        thread_id_t old_tid;
+        int32_t old_tid; /* can't use thread_id_t since it's 64-bits on x64 */


Wait -- os_local_state_t.tid is thread_id_t though, so we need to read a pointer-sized value via READ_TLS_SLOT_IMM, rather than changing these locals to ints. Maybe have a READ_TLS_TIDSZ_SLOT_IMM or sthg.

Right. Ok, for this case I think using a pointer-sized w/ READ_TLS_SLOT_IMM works. That's mentioned in the comments too where the macro is defined.

But you'll read too much for Linux: pid_t is 32 bits even for x86_64. Do you mean you'll read extra into a local and then truncate it (and assume little-endian)?

derekbruening · 2017-03-07T02:46:17Z

core/unix/signal.c

@@ -2794,11 +2794,9 @@ fixup_rtframe_pointers(dcontext_t *dcontext, int sig,
 # endif
    /* 32-bit kernel copies to aligned buf first */
    IF_X64(ASSERT(ALIGNED(f_new->uc.uc_mcontext.fpstate, 16)));
-#elif defined(MACOS)
-# ifndef X64
+#elif defined(MACOS) && !defined(X64) /* XXX: macos 64-bit support needs work */


Should X64 have an ASSERT_NOT_IMPLEMENTED?

derekbruening · 2017-03-07T02:46:25Z

core/unix/signal.c

@@ -2964,7 +2962,7 @@ copy_frame_to_stack(dcontext_t *dcontext, int sig, sigframe_rt_t *frame, byte *s
    }
 #endif /* X86 && LINUX */

-#ifdef MACOS
+#if defined(MACOS) && !defined(X64)


Should X64 have an ASSERT_NOT_IMPLEMENTED?

derekbruening · 2017-03-08T03:40:40Z

core/unix/os.c

        ASSERT(old_tid == old);
    });
-    WRITE_TLS_INT_SLOT_IMM(TLS_THREAD_ID_OFFSET, new_tid);
+    WRITE_TLS_SLOT_IMM(TLS_THREAD_ID_OFFSET, new_tid);


This will overflow for Linux x64 where pid_t (== thread_id_t) is 32 bits.

derekbruening · 2017-03-08T03:42:02Z

core/unix/os.c

-        int32_t old_tid; /* can't use thread_id_t since it's 64-bits on x64 */
-        READ_TLS_INT_SLOT_IMM(TLS_THREAD_ID_OFFSET, old_tid);
+        ptr_int_t old_tid;
+        READ_TLS_SLOT_IMM(TLS_THREAD_ID_OFFSET, old_tid);


This will read the adjacent field (or maybe padding, did not check the struct): it would have to be truncated (or have a custom READ_TLS_SLOT_TIDSZ_IMM as suggested in original comment, or have series of ifdefs or sizeof checks here).

Ok...I wasn't thinking on this one. Done.

A series of small changes to build with XCode 10.1 64-bit: + Updates -max_supported_os_version on Mac to 18. + Updates 64-bit Mac frame field. + Removes , from runtime options in template defines. + Removes 32-bit-only tests from the OSX label list for 64-bit. + Increases the core's preferred base to 0x1'71000000 to stay above the default 0x1' _PAGEZERO. + Adds missing DECL_EXTERNs in mangle_suspend and mangle_asynch tests. + Adds missing GLOBAL_REFs in common.decode test. + Re-defines sigcontext_t and uc_mcontext in tools.h to match this toolchain. + Adds 64-bit libelftc.a and libdwarf.a Mach-O libraries along with instructions on how they were built. + Adds -mmacosx-version-min=10.9 to drcachesim to match core/. + Fixes a format string warning in drcachesim. Includes a number of similar small changes from Shawn Denbow's first 2 commits on the project-mac64 branch in PR #2269 and PR #2273, updated for clang-format: + Move -vm_base above 4GB since __PAGEZERO takes up first 4GB by default + Increase MEMQUERY_INTERNAL_DATA_LEN + Update Mach-O parsing to check for LC_SEGMENT_64 + Change error notifications to warning to allow build to continue + Define SC_FIELD for 64-bit registers r8-r15 and update references + Update dynamorio_mach_dep_syscall to use syscall for 64-bit + Update dynamorio_mach_syscall to use syscall for 64-bit + Change code referencing eflags to xflags for cross platform compat + Misc code fix-ups Issue: #1979

A series of small changes to build with XCode 10.1 64-bit (from PR #3514 on project-mac64): + Updates -max_supported_os_version on Mac to 18. + Updates 64-bit Mac frame field. + Removes , from runtime options in template defines. + Removes 32-bit-only tests from the OSX label list for 64-bit. + Increases the core's preferred base to 0x1'71000000 to stay above the default 0x1' _PAGEZERO. + Adds missing DECL_EXTERNs in mangle_suspend and mangle_asynch tests. + Adds missing GLOBAL_REFs in common.decode test. + Re-defines sigcontext_t and uc_mcontext in tools.h to match this toolchain. + Adds 64-bit libelftc.a and libdwarf.a Mach-O libraries along with instructions on how they were built. + Adds -mmacosx-version-min=10.9 to drcachesim to match core/. + Fixes a format string warning in drcachesim. Includes a number of similar small changes from Shawn Denbow's first 2 commits on the project-mac64 branch in PR #2269 and PR #2273, updated for clang-format: + Move -vm_base above 4GB since __PAGEZERO takes up first 4GB by default + Increase MEMQUERY_INTERNAL_DATA_LEN + Update Mach-O parsing to check for LC_SEGMENT_64 + Change error notifications to warning to allow build to continue + Define SC_FIELD for 64-bit registers r8-r15 and update references + Update dynamorio_mach_dep_syscall to use syscall for 64-bit + Update dynamorio_mach_syscall to use syscall for 64-bit + Change code referencing eflags to xflags for cross platform compat + Misc code fix-ups Issue: #1979

derekbruening requested changes Mar 7, 2017

View reviewed changes

Addresses comments

421174c

derekbruening requested changes Mar 8, 2017

View reviewed changes

Correctly Read/Write TLS slot depending on OS

ffccfea

derekbruening approved these changes Mar 8, 2017

View reviewed changes

shawndenbow merged commit 4975170 into project-mac64 Mar 8, 2017

shawndenbow deleted the i58-mac64-build branch March 8, 2017 23:26

derekbruening mentioned this pull request Apr 15, 2019

i#1979 Mac64: Add 64-bit Mojave + XCode 10.1 build support (#3514) #3541

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

i#1979: steps towards building on MacOS 64-bit #2269

i#1979: steps towards building on MacOS 64-bit #2269

shawndenbow commented Mar 7, 2017

derekbruening left a comment

derekbruening Mar 7, 2017

shawndenbow Mar 7, 2017

derekbruening Mar 7, 2017

shawndenbow Mar 7, 2017

derekbruening Mar 7, 2017

shawndenbow Mar 8, 2017

derekbruening Mar 8, 2017

derekbruening Mar 7, 2017

shawndenbow Mar 8, 2017

derekbruening Mar 7, 2017

shawndenbow Mar 8, 2017

derekbruening Mar 8, 2017

shawndenbow Mar 8, 2017

derekbruening Mar 8, 2017

shawndenbow Mar 8, 2017

		# ifdef X64
		mov REG_XAX, QWORD [REG_XAX] /* load mach_synch_t->sem */

i#1979: steps towards building on MacOS 64-bit #2269

i#1979: steps towards building on MacOS 64-bit #2269

Conversation

shawndenbow commented Mar 7, 2017

derekbruening left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment