Mac x86_64: Mach exception support

src/trusted/service_runtime/osx/crash_filter.c

Index: src/trusted/service_runtime/osx/crash_filter.c
===================================================================
--- src/trusted/service_runtime/osx/crash_filter.c	(revision 10840)
+++ src/trusted/service_runtime/osx/crash_filter.c	(working copy)
@@ -6,34 +6,62 @@
 
 #include "native_client/src/trusted/service_runtime/osx/crash_filter.h"
 
-#include <mach/mach.h>
-#include <mach/task.h>
+#include <inttypes.h>
 
+#include "native_client/src/include/nacl_compiler_annotations.h"
 #include "native_client/src/include/nacl_macros.h"
+#include "native_client/src/shared/platform/nacl_check.h"
 #include "native_client/src/shared/platform/nacl_log.h"
+#include "native_client/src/trusted/service_runtime/nacl_app_thread.h"
+#include "native_client/src/trusted/service_runtime/nacl_globals.h"
+#include "native_client/src/trusted/service_runtime/nacl_tls.h"
+#include "native_client/src/trusted/service_runtime/osx/mach_thread_map.h"
+#include "native_client/src/trusted/service_runtime/sel_ldr.h"
 #include "native_client/src/trusted/service_runtime/sel_rt.h"
 
 
-/*
- * We could provide a version for x86-64, but it would not get tested
- * because we run only minimal tests for x86-64 Mac.  This function is
- * currently only used in Chromium which only uses x86-32 NaCl on Mac.
- */
-#if NACL_ARCH(NACL_BUILD_ARCH) == NACL_x86 && NACL_BUILD_SUBARCH == 32
+#if NACL_ARCH(NACL_BUILD_ARCH) == NACL_x86
 
 int NaClMachThreadIsInUntrusted(mach_port_t thread_port) {
-  natural_t regs_array[i386_THREAD_STATE_COUNT];
-  mach_msg_type_number_t size = NACL_ARRAY_SIZE(regs_array);
-  i386_thread_state_t *regs = (i386_thread_state_t *) regs_array;
-  kern_return_t rc;
-  uint16_t global_cs = NaClGetGlobalCs();
+  x86_thread_state_t state;
+  thread_state_t statep = (thread_state_t) &state;
+  mach_msg_type_number_t size = x86_THREAD_STATE_COUNT;
+  kern_return_t kr;
+  uint32_t nacl_thread_index;
 
-  rc = thread_get_state(thread_port, i386_THREAD_STATE, regs_array, &size);
-  if (rc != 0) {
-    NaClLog(LOG_FATAL, "NaClMachThreadIsInUntrusted: "
-            "thread_get_state() failed with error %i\n", (int) rc);
+  kr = thread_get_state(thread_port, x86_THREAD_STATE, statep, &size);
+  CHECK(kr == KERN_SUCCESS);

[Mark Seaborn, 2013/02/20 18:33:17]

Nit: The NaClLog() call you're replacing here had a more informative error
message.  Could you restore that?

[Mark Mentovai, 2013/02/20 20:15:48]

Mark Seaborn wrote:
Done, but I think that the more succinct CHECK form is more “do what I mean,” in
that the important thing is the assertion, and not so much that the message gets
logged with an error code.

+
+#if NACL_BUILD_SUBARCH == 32
+  CHECK(state.tsh.flavor == x86_THREAD_STATE32);
+  nacl_thread_index = state.uts.ts32.__gs >> 3;

[Mark Seaborn, 2013/02/20 18:33:17]

As an aside, it's kind of weird that this value doesn't get used.  You could set
nacl_thread_index to anything here and the code would work.  I suppose it
doesn't matter though.

[Mark Mentovai, 2013/02/20 20:15:48]

Mark Seaborn wrote:
I don’t like having this dangling unused variable. As you saw in an earlier
version, I got rid of the extra parameter to the inner function entirely at one
point, but I thought the #ifdef soup was too hairy. If I declare it and don’t
give it a value, it looks like an uninitialized use, so I have to either give it
a fake value or a real value, and I figured, why not a real one?

I’m happy to do this a different way if there’s something you really like, but
none of the solutions are really awesome. Part of the problem is the strict
C89/C90 intolerance of mixed declarations and code, and having to add the one
extra #ifdef in the declarations section tips the balance in my mind against
doing any #ifdefs at all.

+#elif NACL_BUILD_SUBARCH == 64
+  nacl_thread_index = NaClGetThreadIndexForMachThread(thread_port);
+#endif
+
+  /*
+   * If the thread isn't known to Native Client, it's not untrusted (at least
+   * not by Native Client.)
+   */
+  if (nacl_thread_index == NACL_TLS_INDEX_INVALID) {

[Mark Seaborn, 2013/02/20 18:33:17]

See other comment -- for consistency with mach_exception_handler.c, it would be
better to put this inside the "#elif NACL_BUILD_SUBARCH == 64".

+    return 0;
   }
 
+  return NaClMachThreadStateIsInUntrusted(&state, nacl_thread_index);
+}
+
+int NaClMachThreadStateIsInUntrusted(x86_thread_state_t *state,
+                                     size_t nacl_thread_index) {
+#if NACL_BUILD_SUBARCH == 32
+
+  uint16_t global_cs;
+
+  UNREFERENCED_PARAMETER(nacl_thread_index);
+
+  CHECK(state->tsh.flavor == x86_THREAD_STATE32);
+
+  global_cs = NaClGetGlobalCs();
+
   /*
    * If global_cs is 0 (which is not a usable segment selector), the
    * sandbox has not been initialised yet, so there can be no untrusted
@@ -43,7 +71,18 @@
     return 0;
   }
 
-  return regs->__cs != global_cs;
+  return state->uts.ts32.__cs != global_cs;
+
+#elif NACL_BUILD_SUBARCH == 64
+
+  struct NaClAppThread *natp;
+
+  CHECK(state->tsh.flavor == x86_THREAD_STATE64);
+
+  natp = NaClAppThreadGetFromIndex(nacl_thread_index);
+  return NaClIsUserAddr(natp->nap, state->uts.ts64.__rip);
+
+#endif  /* NACL_BUILD_SUBARCH */
 }
 
-#endif
+#endif  /* NACL_ARCH(NACL_BUILD_ARCH) */