Don't os protect with Lisp <-> foreign thread safepoint mechanism

NOTE: The use of locks here may tank performance. Switching to atomics
like CAS is advised.

src/assembly/arm64/tramps.lisp

@@ -172,6 +172,31 @@
           (inst sub csp-tn csp-tn (+ 32 80))
           (inst str zr-tn (@ thread-tn (* thread-control-stack-pointer-slot n-word-bytes))))
         (map-pairs ldp nsp-tn 64 nl-registers :post-index 80 :delta -16)
+        (inst ret))
+
+      #+(and sb-safepoint no-os-protect)
+      (define-assembly-routine (set-csp-around-foreign-call (:return-style :none))
+          ((:temp nl0 unsigned-reg nl0-offset)
+           (:temp nl1 unsigned-reg nl1-offset)
+           (:temp nl3 unsigned-reg nl3-offset))
+        (map-pairs stp nsp-tn 0 nl-registers :pre-index -80)
+
+        ;; Don't add pseudo-atomic here, we don't want to go into the
+        ;; safepoint handler here!!
+        (inst mov nl0 thread-tn)
+        (inst mov nl1 tmp-tn)
+        (inst add csp-tn csp-tn (+ 32 80))
+        (inst stp cfp-tn lr-tn (@ csp-tn -112))
+        (map-pairs stp csp-tn -80 lisp-registers)
+        (map-pairs stp nsp-tn 0 float-registers :pre-index -512 :delta 32)
+
+        (invoke-foreign-routine "set_csp_around_foreign_call" nl3)
+
+        (map-pairs ldp nsp-tn 480 float-registers :post-index 512 :delta -32)
+        (map-pairs ldp csp-tn -16 lisp-registers :delta -16)
+        (inst ldr lr-tn (@ csp-tn -104))
+        (inst sub csp-tn csp-tn (+ 32 80))
+        (map-pairs ldp nsp-tn 64 nl-registers :post-index 80 :delta -16)
         (inst ret)))))
 
 ;; This is kinda like the alloc tramp except we try to spill all

src/compiler/arm64/c-call.lisp

@@ -227,6 +227,11 @@
   (loop for i from 0 to 18 collect i)
   #'equal)
 
+#+no-os-protect
+(defun set-csp-around-foreign-call (value cfunc)
+  (inst mov tmp-tn value)
+  (invoke-asm-routine 'set-csp-around-foreign-call cfunc))
+
 (defun emit-c-call (vop nfp-save temp temp2 cfunc function)
   (let ((cur-nfp (current-nfp-tn vop)))
     (when cur-nfp
@@ -243,7 +248,11 @@
         ;; OK to run GC without stopping this thread from this point
         ;; on.
         #+sb-safepoint
-        (storew csp-tn thread-tn thread-saved-csp-slot)
+        (progn
+          #+no-os-protect
+          (set-csp-around-foreign-call csp-tn cfunc)
+          #-no-os-protect
+          (storew csp-tn thread-tn thread-saved-csp-slot))
         (cond ((stringp function)
                (invoke-foreign-routine function cfunc))
               (t
@@ -267,7 +276,11 @@
                        0))
         ;; No longer OK to run GC except at safepoints.
         #+sb-safepoint
-        (storew zr-tn thread-tn thread-saved-csp-slot)
+        (progn
+          #+no-os-protect
+          (set-csp-around-foreign-call zr-tn cfunc)
+          #-no-os-protect
+          (storew zr-tn thread-tn thread-saved-csp-slot))
         (storew zr-tn thread-tn thread-control-stack-pointer-slot))
       return
       #-sb-thread

src/runtime/arm64-assem.S

@@ -321,8 +321,66 @@ GNAME(call_into_c):
 #endif
 
 #ifdef LISP_FEATURE_SB_SAFEPOINT
+#ifdef LISP_FEATURE_NO_OS_PROTECT
+        // X0-x18 are caller-saved registers.
+
+        stp     x0,x1, [sp,#-360]!
+        stp     x2,x3, [sp,#16]
+        stp     x4,x5, [sp,#32]
+        stp     x6,x7, [sp,#48]
+        stp     x8,x9, [sp,#64]
+        stp     x10,x11, [sp,#80]
+        stp     x12,x13, [sp,#96]
+        stp     x14,x15, [sp,#112]
+        stp     x16,x17, [sp,#128]
+        stp     x18,x30, [sp,#144]
+
+        stp     d0,d1, [sp,#160]
+        stp     d2,d3, [sp,#176]
+        stp     d4,d5, [sp,#192]
+        stp     d6,d7, [sp,#208]
+        stp     d16,d17, [sp,#224]
+        stp     d18,d19, [sp,#240]
+        stp     d20,d21, [sp,#256]
+        stp     d22,d23, [sp,#272]
+        stp     d24,d25, [sp,#288]
+        stp     d26,d27, [sp,#304]
+        stp     d29,d30, [sp,#336]
+        str     d31, [sp,#352]
+
+        mov     x0, reg_THREAD
+        mov     x1, reg_CSP
+        bl      set_csp_around_foreign_call
+
+        // Restore registers.
+
+        ldr     d31, [sp,#352]
+        ldp     d29,d30, [sp,#336]
+        ldp     d26,d27, [sp,#304]
+        ldp     d24,d25, [sp,#288]
+        ldp     d22,d23, [sp,#272]
+        ldp     d20,d21, [sp,#256]
+        ldp     d18,d19, [sp,#240]
+        ldp     d16,d17, [sp,#224]
+        ldp     d6,d7, [sp,#208]
+        ldp     d4,d5, [sp,#192]
+        ldp     d2,d3, [sp,#176]
+        ldp     d0,d1, [sp,#160]
+
+        ldp     x18,x30, [sp,#144]
+        ldp     x16,x17, [sp,#128]
+        ldp     x14,x15, [sp,#112]
+        ldp     x12,x13, [sp,#96]
+        ldp     x10,x11, [sp,#80]
+        ldp     x8,x9, [sp,#64]
+        ldp     x6,x7, [sp,#48]
+        ldp     x4,x5, [sp,#32]
+        ldp     x2,x3, [sp,#16]
+        ldp     x0,x1, [sp],#360
+#else
 	/* OK to run GC without stopping this thread from this point on. */
 	str     reg_CSP, [reg_THREAD, THREAD_SAVED_CSP_OFFSET]
+#endif
 #endif
 
         // And call the C function.
@@ -354,9 +412,69 @@ GNAME(call_into_c):
 #endif
 
 # ifdef LISP_FEATURE_SB_SAFEPOINT
+#ifdef LISP_FEATURE_NO_OS_PROTECT
+
+        // X0-x18 are caller-saved registers.
+
+        stp     x0,x1, [sp,#-360]!
+        stp     x2,x3, [sp,#16]
+        stp     x4,x5, [sp,#32]
+        stp     x6,x7, [sp,#48]
+        stp     x8,x9, [sp,#64]
+        stp     x10,x11, [sp,#80]
+        stp     x12,x13, [sp,#96]
+        stp     x14,x15, [sp,#112]
+        stp     x16,x17, [sp,#128]
+        stp     x18,x30, [sp,#144]
+
+        stp     d0,d1, [sp,#160]
+        stp     d2,d3, [sp,#176]
+        stp     d4,d5, [sp,#192]
+        stp     d6,d7, [sp,#208]
+        stp     d16,d17, [sp,#224]
+        stp     d18,d19, [sp,#240]
+        stp     d20,d21, [sp,#256]
+        stp     d22,d23, [sp,#272]
+        stp     d24,d25, [sp,#288]
+        stp     d26,d27, [sp,#304]
+        stp     d29,d30, [sp,#336]
+        str     d31, [sp,#352]
+
+        mov     x0, reg_THREAD
+        mov     x1, xzr
+        bl      set_csp_around_foreign_call
+
+        // Restore registers.
+
+        ldr     d31, [sp,#352]
+        ldp     d29,d30, [sp,#336]
+        ldp     d26,d27, [sp,#304]
+        ldp     d24,d25, [sp,#288]
+        ldp     d22,d23, [sp,#272]
+        ldp     d20,d21, [sp,#256]
+        ldp     d18,d19, [sp,#240]
+        ldp     d16,d17, [sp,#224]
+        ldp     d6,d7, [sp,#208]
+        ldp     d4,d5, [sp,#192]
+        ldp     d2,d3, [sp,#176]
+        ldp     d0,d1, [sp,#160]
+
+        ldp     x18,x30, [sp,#144]
+        ldp     x16,x17, [sp,#128]
+        ldp     x14,x15, [sp,#112]
+        ldp     x12,x13, [sp,#96]
+        ldp     x10,x11, [sp,#80]
+        ldp     x8,x9, [sp,#64]
+        ldp     x6,x7, [sp,#48]
+        ldp     x4,x5, [sp,#32]
+        ldp     x2,x3, [sp,#16]
+        ldp     x0,x1, [sp],#360
+// FIXME: WE NEED TO BLANK BOXED REGISTERS AGAIN HERE.
+#else
 	/* No longer OK to run GC except at safepoints. */
 	str     xzr, [reg_THREAD, THREAD_SAVED_CSP_OFFSET]
 # endif
+#endif
 
 
         // Restore the Lisp stack and frame pointers

src/runtime/safepoint.c

@@ -266,7 +266,7 @@ set_csp_from_context(struct thread *self, os_context_t *ctx)
      * disconcerting. */
     void **sp = (void **) access_control_stack_pointer(self);
 #endif
-    csp_around_foreign_call(self) = (lispobj) sp;
+    set_csp_around_foreign_call(self, (lispobj) sp);
 }
 
 
@@ -299,13 +299,44 @@ thread_blocks_gc(struct thread *thread)
 static inline bool
 set_thread_csp_access(struct thread* th, bool writable)
 {
+#ifdef LISP_FEATURE_NO_OS_PROTECT
+    pthread_mutex_t* lock = (char*)th - THREAD_CSP_PAGE_SIZE - THREAD_HEADER_SLOTS*N_WORD_BYTES;
+    mutex_acquire(lock);
+    *(((lispobj*)th)-(2+THREAD_HEADER_SLOTS)) = writable;
+    int lock_ret = mutex_release(lock);
+    gc_assert(lock_ret);
+#else
     os_protect((char*)th - (THREAD_HEADER_SLOTS*N_WORD_BYTES) - THREAD_CSP_PAGE_SIZE,
                THREAD_CSP_PAGE_SIZE,
                writable? (OS_VM_PROT_READ|OS_VM_PROT_WRITE)
                : (OS_VM_PROT_READ));
+#endif
     return csp_around_foreign_call(th) != 0;
 }
 
+#ifdef LISP_FEATURE_NO_OS_PROTECT
+void set_csp_around_foreign_call(struct thread *thread, lispobj value) {
+    pthread_mutex_t* lock = (char*)thread - THREAD_CSP_PAGE_SIZE - THREAD_HEADER_SLOTS*N_WORD_BYTES;
+    mutex_acquire(lock);
+    int lock_ret;
+    if (*(((lispobj*)thread)-(2+THREAD_HEADER_SLOTS))) {
+        *(((lispobj*)thread)-(1+THREAD_HEADER_SLOTS)) = value;
+        lock_ret = mutex_release(lock); // Is this safe??
+        gc_assert(lock_ret);
+    } else {
+        lock_ret = mutex_release(lock); // Is this safe???
+        gc_assert(lock_ret);
+        odxprint(safepoints, "setting was not writable");
+        if (!foreign_function_call_active_p(thread))
+            lose("CSP trap in Lisp?");
+        thread_in_safety_transition(NULL);
+        // Now retry setting the csp word, as the CSP trap would do
+        // with hardware page protection.
+        set_csp_around_foreign_call(thread, value);
+    }
+}
+#endif
+
 static inline void gc_notify_early()
 {
     struct thread *self = get_sb_vm_thread(), *p;
@@ -688,7 +719,7 @@ void thread_in_lisp_raised(os_context_t *ctxptr)
                 /* ??? Isn't this already T? */
                 write_TLS(GC_PENDING,LISP_T,self);
             }
-            csp_around_foreign_call(self) = 0;
+            set_csp_around_foreign_call(self, 0);
             check_gc_and_thruptions = 1;
         } else {
             /* This thread isn't a candidate for running the GC
@@ -709,7 +740,7 @@ void thread_in_lisp_raised(os_context_t *ctxptr)
                     gc_advance(GC_NONE,gc_state.phase);
                 else
                     gc_state_wait(GC_NONE);
-                csp_around_foreign_call(self) = 0;
+                set_csp_around_foreign_call(self, 0);
                 check_gc_and_thruptions = 1;
             } else {
                 /* This thread has GC_INHIBIT set, meaning that it's
@@ -771,7 +802,7 @@ void thread_in_safety_transition(os_context_t *ctxptr)
                     gc_advance(GC_NONE,gc_state.phase);
                 else
                     gc_state_wait(GC_NONE);
-                csp_around_foreign_call(self) = 0;
+                set_csp_around_foreign_call(self, 0);
             } else {
                 /* This thread has GC_INHIBIT set, meaning that it's
                  * within a WITHOUT-GCING, so advance from wherever we
@@ -1018,9 +1049,9 @@ void thruption_handler(__attribute__((unused)) int signal,
 #endif
 
     /* In C code.  As a rule, we assume that running thruptions is OK. */
-    csp_around_foreign_call(self) = 0;
+    set_csp_around_foreign_call(self, 0);
     thread_in_lisp_raised(ctx);
-    csp_around_foreign_call(self) = (intptr_t) transition_sp;
+    set_csp_around_foreign_call(self, (intptr_t) transition_sp);
 }
 # endif
 
@@ -1039,7 +1070,10 @@ handle_safepoint_violation(os_context_t *ctx, os_vm_address_t fault_address)
     struct thread *self = get_sb_vm_thread();
 
     if (fault_address == (os_vm_address_t) GC_SAFEPOINT_TRAP_ADDR) {
-#ifndef LISP_FEATURE_NO_OS_PROTECT
+#ifdef LISP_FEATURE_NO_OS_PROTECT
+        lose("Shouldn't get here!");
+#endif
+
 #ifdef LISP_FEATURE_C_STACK_IS_CONTROL_STACK
         /* We're on the altstack and don't want to run Lisp code. */
         arrange_return_to_c_function(ctx, handle_global_safepoint_violation, 0);
@@ -1050,12 +1084,13 @@ handle_safepoint_violation(os_context_t *ctx, os_vm_address_t fault_address)
         undo_fake_foreign_function_call(ctx);
 #endif
         return 1;
-#else
-        lose("Shouldn't get here!");
-#endif
     }
 
     if ((1+THREAD_HEADER_SLOTS)+(lispobj*)fault_address == (lispobj*)self) {
+#ifdef LISP_FEATURE_NO_OS_PROTECT
+        lose("Shouldn't get here!");
+#endif
+
 #ifdef LISP_FEATURE_C_STACK_IS_CONTROL_STACK
         arrange_return_to_c_function(ctx, handle_csp_safepoint_violation, 0);
 #else

src/runtime/thread.c

@@ -446,7 +446,7 @@ init_new_thread(struct thread *th,
      * danger of deadlocking even with SIG_STOP_FOR_GC blocked (which
      * it is not). */
 #ifdef LISP_FEATURE_SB_SAFEPOINT
-    csp_around_foreign_call(th) = (lispobj)scribble;
+    set_csp_around_foreign_call(th, (lispobj)scribble);
 #endif
     __attribute__((unused)) int lock_ret = mutex_acquire(&all_threads_lock);
     gc_assert(lock_ret);
@@ -943,10 +943,18 @@ alloc_thread_struct(void* spaces) {
                                 + THREAD_HEADER_SLOTS*N_WORD_BYTES);
 
 #ifdef LISP_FEATURE_SB_SAFEPOINT
+#ifdef LISP_FEATURE_NO_OS_PROTECT
+    // XXX: The mutex fits, hopefully. The stuff like csp and guard
+    // words are at the end of the page.
+    pthread_mutex_t* mutex = (void*)csp_page;
+    pthread_mutex_init(mutex, 0);
+    *(((lispobj*)th)-(2+THREAD_HEADER_SLOTS)) = 1; // make "writable"
+#else
     // Out of caution I'm supposing that the last thread to use this memory
     // might have left this page as read-only. Could it? I have no idea.
     os_protect(csp_page, THREAD_CSP_PAGE_SIZE, OS_VM_PROT_READ|OS_VM_PROT_WRITE);
 #endif
+#endif
 
 #ifdef LISP_FEATURE_SB_THREAD
     memset(th, 0, sizeof *th);

src/runtime/thread.h

@@ -300,15 +300,25 @@ void wake_thread(struct thread_instance*),
      wake_thread_impl(struct thread_instance*);
 # endif
 
-#define csp_around_foreign_call(thread) *(((lispobj*)thread)-(1+THREAD_HEADER_SLOTS))
+static inline lispobj csp_around_foreign_call(struct thread *thread) {
+    return *(((lispobj*)thread)-(1+THREAD_HEADER_SLOTS));
+}
+
+#ifdef LISP_FEATURE_NO_OS_PROTECT
+void set_csp_around_foreign_call(struct thread *thread, lispobj value);
+#else
+static inline void set_csp_around_foreign_call(struct thread *thread, lispobj value) {
+    *(((lispobj*)thread)-(1+THREAD_HEADER_SLOTS)) = value;
+}
+#endif
 
 static inline
 void push_gcing_safety(struct gcing_safety *into)
 {
     struct thread* th = get_sb_vm_thread();
     asm volatile ("");
     into->csp_around_foreign_call = csp_around_foreign_call(th);
-    csp_around_foreign_call(th) = 0;
+    set_csp_around_foreign_call(th, 0);
     asm volatile ("");
 }
 
@@ -317,7 +327,7 @@ void pop_gcing_safety(struct gcing_safety *from)
 {
     struct thread* th = get_sb_vm_thread();
     asm volatile ("");
-    csp_around_foreign_call(th) = from->csp_around_foreign_call;
+    set_csp_around_foreign_call(th, from->csp_around_foreign_call);
     asm volatile ("");
 }