From: Nikodemus Siivola <nikodemus@random-state.net>
Date: Mon, 2 Apr 2007 15:20:39 +0000 (+0000)
Subject: 1.0.4.18: trap handling cleanup continues
X-Git-Url: http://repo.macrolet.net/gitweb/?a=commitdiff_plain;h=23124951022f995aace9e7f17e650cd23b83c591;p=sbcl.git

1.0.4.18: trap handling cleanup continues

 * handle_trap is even better then maybe_handle_trap: if we can't
   handle the trap sanely pass the buck to unhandled_trap_error
   instead of generic signal handling routines.

 * implement Windows trap handling on top of handle_trap, not
   sigtrap_handler: some copy-paste from x86-arch.c later Windows is
   now almost free of the "signal handling" remnants.

 * Replace the bogus lossage messages saying
   "fake_foreign_function_call fell through" with things that actually
   make sense.
---

diff --git a/src/code/interr.lisp b/src/code/interr.lisp
index 8614280..260e105 100644
--- a/src/code/interr.lisp
+++ b/src/code/interr.lisp
@@ -481,3 +481,13 @@
 (defun memory-fault-error ()
   (error 'memory-fault-error
          :address current-memory-fault-address))
+
+;;; This is SIGTRAP / EXCEPTION_BREAKPOINT that runtime could not deal
+;;; with. Prior to Windows we just had a Lisp side handler for
+;;; SIGTRAP, but now we need to deal this portably.
+(defun unhandled-trap-error (context-sap)
+  (declare (type system-area-pointer context-sap))
+  (infinite-error-protect
+   (let ((context (sap-alien context-sap (* os-context-t))))
+     (error "Unhandled breakpoint/trap at #x~X."
+            (sap-int (sb!vm:context-pc context))))))
diff --git a/src/code/target-signal.lisp b/src/code/target-signal.lisp
index ceffe0b..c3b6644 100644
--- a/src/code/target-signal.lisp
+++ b/src/code/target-signal.lisp
@@ -118,7 +118,6 @@
 
 (define-signal-handler sigint-handler "interrupted" sigint-%break)
 (define-signal-handler sigill-handler "illegal instruction")
-(define-signal-handler sigtrap-handler "breakpoint/trap")
 #!-linux
 (define-signal-handler sigemt-handler "SIGEMT")
 (define-signal-handler sigbus-handler "bus error")
@@ -147,7 +146,6 @@
   (enable-interrupt sigint #'sigint-handler)
   (enable-interrupt sigterm #'sigterm-handler)
   (enable-interrupt sigill #'sigill-handler)
-  (enable-interrupt sigtrap #'sigtrap-handler)
   (enable-interrupt sigiot #'sigiot-handler)
   #!-linux
   (enable-interrupt sigemt #'sigemt-handler)
diff --git a/src/compiler/generic/parms.lisp b/src/compiler/generic/parms.lisp
index 275b259..95fae73 100644
--- a/src/compiler/generic/parms.lisp
+++ b/src/compiler/generic/parms.lisp
@@ -20,6 +20,7 @@
     sb!kernel::undefined-alien-variable-error
     sb!kernel::undefined-alien-function-error
     sb!kernel::memory-fault-error
+    sb!kernel::unhandled-trap-error
     sb!di::handle-breakpoint
     sb!di::handle-single-step-trap
     fdefinition-object
diff --git a/src/runtime/alpha-arch.c b/src/runtime/alpha-arch.c
index 827080f..26fa572 100644
--- a/src/runtime/alpha-arch.c
+++ b/src/runtime/alpha-arch.c
@@ -328,8 +328,7 @@ sigtrap_handler(int signal, siginfo_t *siginfo, os_context_t *context)
     } else
         /* a "system service" */
     code=*((u32 *)(*os_context_pc_addr(context)));
-    if (!maybe_handle_trap(context, code))
-        interrupt_handle_now(signal, siginfo, context);
+    handle_trap(context, code);
 }
 
 unsigned long
diff --git a/src/runtime/hppa-arch.c b/src/runtime/hppa-arch.c
index 0624da6..a2a758b 100644
--- a/src/runtime/hppa-arch.c
+++ b/src/runtime/hppa-arch.c
@@ -215,8 +215,7 @@ sigtrap_handler(int signal, siginfo_t *siginfo, void *void_context)
         interrupt_handle_now(signal, siginfo, context);
     else {
         int im5 = bad_inst & 0x1f;
-        if (!maybe_handle_trap(context, trap))
-            interrupt_handle_now(signal, sigingo, context);
+        handle_trap(context, trap);
     }
 }
 
diff --git a/src/runtime/interrupt.c b/src/runtime/interrupt.c
index 8cdb01f..9ee4193 100644
--- a/src/runtime/interrupt.c
+++ b/src/runtime/interrupt.c
@@ -406,14 +406,18 @@ interrupt_internal_error(os_context_t *context, boolean continuable)
 void
 interrupt_handle_pending(os_context_t *context)
 {
-    struct thread *thread;
-    struct interrupt_data *data;
+#ifdef LISP_FEATURE_WIN32
+    /* Actually this part looks almost good to go for
+     * Windows too, but currently we never save anything
+     * on Windows, so ending up here would be pretty bad. */
+    lose("Pending interrupts unimplemented on Windows.");
+#else
+    struct thread *thread = arch_os_get_current_thread();
+    struct interrupt_data *data = thread->interrupt_data;
 
     FSHOW_SIGNAL((stderr, "/entering interrupt_handle_pending\n"));
 
     check_blockables_blocked_or_lose();
-    thread=arch_os_get_current_thread();
-    data=thread->interrupt_data;
 
     /* If pseudo_atomic_interrupted is set then the interrupt is going
      * to be handled now, ergo it's safe to clear it. */
@@ -461,7 +465,6 @@ interrupt_handle_pending(os_context_t *context)
              * PSEUDO_ATOMIC_INTERRUPTED only if interrupts are enabled.*/
             SetSymbolValue(INTERRUPT_PENDING, NIL,thread);
 
-#ifndef LISP_FEATURE_WIN32
             /* restore the saved signal mask from the original signal (the
              * one that interrupted us during the critical section) into the
              * os_context for the signal we're currently in the handler for.
@@ -470,12 +473,12 @@ interrupt_handle_pending(os_context_t *context)
             sigcopyset(os_context_sigmask_addr(context), &data->pending_mask);
 
             sigemptyset(&data->pending_mask);
-#endif
             /* This will break on sparc linux: the deferred handler really wants
              * to be called with a void_context */
             run_deferred_handler(data,(void *)context);
         }
     }
+#endif
 }
 
 /*
@@ -1315,11 +1318,24 @@ lisp_memory_fault_error(os_context_t *context, os_vm_address_t addr)
 }
 #endif
 
+void
+unhandled_trap_error(os_context_t *context)
+{
+    lispobj context_sap;
+    fake_foreign_function_call(context);
+    context_sap = alloc_sap(context);
+#ifndef LISP_FEATURE_WIN32
+    thread_sigmask(SIG_SETMASK, os_context_sigmask_addr(context), 0);
+#endif
+    funcall1(SymbolFunction(UNHANDLED_TRAP_ERROR), context_sap);
+    lose("UNHANDLED-TRAP-ERROR fell through");
+}
+
 /* Common logic far trapping instructions. How we actually handle each
  * case is highly architecture dependant, but the overall shape is
  * this. */
-boolean
-maybe_handle_trap(os_context_t *context, int trap)
+void
+handle_trap(os_context_t *context, int trap)
 {
     switch(trap) {
     case trap_PendingInterrupt:
@@ -1353,11 +1369,7 @@ maybe_handle_trap(os_context_t *context, int trap)
         fake_foreign_function_call(context);
         lose("%%PRIMITIVE HALT called; the party is over.\n");
     default:
-        FSHOW((stderr,"/[C--trap default %d %d %x]\n",
-               signal, trap, context));
-        /* Not our trap! */
-        return 0;
+        unhandled_trap_error(context);
     }
-    return 1;
 }
 
diff --git a/src/runtime/interrupt.h b/src/runtime/interrupt.h
index e8a23b6..a373662 100644
--- a/src/runtime/interrupt.h
+++ b/src/runtime/interrupt.h
@@ -104,7 +104,7 @@ extern void block_blockable_signals();
  * "cleanly" with union types is in fact a mess. */
 #define ARE_SAME_HANDLER(x, y) ((void*)(x) == (void*)(y))
 
-extern boolean maybe_handle_trap(os_context_t *context, int trap);
+extern void handle_trap(os_context_t *context, int trap);
 
 #ifndef LISP_FEATURE_WIN32
 extern void lisp_memory_fault_error(os_context_t *context, os_vm_address_t addr);
diff --git a/src/runtime/mips-arch.c b/src/runtime/mips-arch.c
index d53ea91..590f9b1 100644
--- a/src/runtime/mips-arch.c
+++ b/src/runtime/mips-arch.c
@@ -392,13 +392,13 @@ sigtrap_handler(int signal, siginfo_t *info, void *void_context)
     os_context_t *context = arch_os_get_context(&void_context);
     unsigned int code = (os_context_insn(context) >> 6) & 0xfffff;
     /* FIXME: WTF is this magic number? Needs to become a #define
-     * and go into maybe_handle_trap. */
+     * and go into handle_trap. */
     if (code==0x10) {
         arch_clear_pseudo_atomic_interrupted(context);
         arch_skip_instruction(context);
         interrupt_handle_pending(context);
-    } else if (!maybe_handle_trap(context,code))
-        interrupt_handle_now(signal, info, void_context);
+    } else
+        handle_trap(context,code);
 }
 
 #define FIXNUM_VALUE(lispobj) (((int)lispobj) >> N_FIXNUM_TAG_BITS)
diff --git a/src/runtime/ppc-arch.c b/src/runtime/ppc-arch.c
index 55bc375..ad4450a 100644
--- a/src/runtime/ppc-arch.c
+++ b/src/runtime/ppc-arch.c
@@ -443,9 +443,7 @@ sigtrap_handler(int signal, siginfo_t *siginfo, os_context_t *context)
     if ((code >> 16) == ((3 << 10) | (6 << 5))) {
         /* twllei reg_ZERO,N will always trap if reg_ZERO = 0 */
         int trap = code & 0x1f;
-
-        if (!maybe_handle_trap(context,trap))
-            interrupt_handle_now(signal, siginfo, context);
+        handle_trap(context,trap);
 
 #ifdef LISP_FEATURE_DARWIN
         DARWIN_FIX_CONTEXT(context);
diff --git a/src/runtime/sparc-arch.c b/src/runtime/sparc-arch.c
index d258e7d..c2f7d2d 100644
--- a/src/runtime/sparc-arch.c
+++ b/src/runtime/sparc-arch.c
@@ -250,8 +250,7 @@ static void sigill_handler(int signal, siginfo_t *siginfo, void *void_context)
 
         inst = *pc;
         trap = inst & 0x3fffff;
-        if (!maybe_handle_trap(context,trap))
-            interrupt_handle_now(signal, siginfo, context);
+        handle_trap(context,trap);
     }
     else if ((siginfo->si_code) == ILL_ILLTRP
 #ifdef LISP_FEATURE_LINUX
diff --git a/src/runtime/win32-os.c b/src/runtime/win32-os.c
index e4620f8..d2acd50 100644
--- a/src/runtime/win32-os.c
+++ b/src/runtime/win32-os.c
@@ -305,10 +305,6 @@ is_valid_lisp_addr(os_vm_address_t addr)
     return 0;
 }
 
-/*
- * any OS-dependent special low-level handling for signals
- */
-
 /* A tiny bit of interrupt.c state we want our paws on. */
 extern boolean internal_errors_enabled;
 
@@ -345,17 +341,18 @@ handle_exception(EXCEPTION_RECORD *exception_record,
     }
 
     if (exception_record->ExceptionCode == EXCEPTION_BREAKPOINT) {
-        /* Pick off sigtrap case first. */
-
-        extern void sigtrap_handler(int signal, siginfo_t *info, void *context);
-        /*
-         * Unlike some other operating systems, Win32 leaves EIP
-         * pointing to the breakpoint instruction.
-         */
+        /* This is just for info in case the monitor wants to print an
+         * approximation. */
+        current_control_stack_pointer =
+            (lispobj *)*os_context_sp_addr(context);
+        /* Unlike some other operating systems, Win32 leaves EIP
+         * pointing to the breakpoint instruction. */
         context->Eip++;
-
-        sigtrap_handler(0, NULL, context);
-
+        /* Now EIP points just after the INT3 byte and aims at the
+         * 'kind' value (eg trap_Cerror). */
+        trap = *(unsigned char *)(*os_context_pc_addr(context));
+        handle_trap(context, trap);
+        /* Done, we're good to go! */
         return ExceptionContinueExecution;
     }
     else if (exception_record->ExceptionCode == EXCEPTION_ACCESS_VIOLATION &&
@@ -457,7 +454,7 @@ handle_exception(EXCEPTION_RECORD *exception_record,
     fflush(stderr);
 
     fake_foreign_function_call(context);
-    lose("fake_foreign_function_call fell through");
+    lose("Exception too early in cold init, cannot continue.");
 
     /* FIXME: WTF? How are we supposed to end up here? */
     return ExceptionContinueSearch;
diff --git a/src/runtime/x86-64-arch.c b/src/runtime/x86-64-arch.c
index 657381f..0a4769a 100644
--- a/src/runtime/x86-64-arch.c
+++ b/src/runtime/x86-64-arch.c
@@ -279,8 +279,7 @@ sigtrap_handler(int signal, siginfo_t *info, void *void_context)
      * arguments to follow. */
     trap = *(unsigned char *)(*os_context_pc_addr(context));
 
-    if (!maybe_handle_trap(context, trap))
-        interrupt_handle_now(signal, info, context);
+    handle_trap(context, trap);
 }
 
 void
@@ -298,7 +297,7 @@ sigill_handler(int signal, siginfo_t *siginfo, void *void_context) {
 #endif
 
     fake_foreign_function_call(context);
-    lose("fake_foreign_function_call fell through");
+    lose("Unhandled SIGILL.");
 }
 
 #ifdef X86_64_SIGFPE_FIXUP
diff --git a/src/runtime/x86-arch.c b/src/runtime/x86-arch.c
index 3ef7fc8..0a8bdd5 100644
--- a/src/runtime/x86-arch.c
+++ b/src/runtime/x86-arch.c
@@ -256,19 +256,17 @@ arch_handle_single_step_trap(os_context_t *context, int trap)
     handle_single_step_trap(context, trap, 0);
 }
 
+#ifndef LISP_FEATURE_WIN32
 void
 sigtrap_handler(int signal, siginfo_t *info, void *void_context)
 {
     os_context_t *context = (os_context_t*)void_context;
     unsigned int trap;
 
-#ifndef LISP_FEATURE_WIN32
-    /* On Windows this is done in the SE handler. */
     if (single_stepping && (signal==SIGTRAP)) {
         restore_breakpoint_from_single_step(context);
         return;
     }
-#endif
 
     /* This is just for info in case the monitor wants to print an
      * approximation. */
@@ -285,7 +283,6 @@ sigtrap_handler(int signal, siginfo_t *info, void *void_context)
     os_restore_fp_control(context);
 #endif
 
-
 #ifdef LISP_FEATURE_SUNOS
     /* For some reason the breakpoints that :ENCAPSULATE NIL tracing sets up
      * cause a trace trap (i.e. processor single-stepping trap) on the following
@@ -302,8 +299,7 @@ sigtrap_handler(int signal, siginfo_t *info, void *void_context)
      * number of bytes will follow, the first is the length of the byte
      * arguments to follow. */
     trap = *(unsigned char *)(*os_context_pc_addr(context));
-    if (!maybe_handle_trap(context, trap))
-        interrupt_handle_now(signal, info, context);
+    handle_trap(context, trap);
 }
 
 void
@@ -319,10 +315,10 @@ sigill_handler(int signal, siginfo_t *siginfo, void *void_context) {
         return sigtrap_handler(signal, siginfo, void_context);
     }
 #endif
-
     fake_foreign_function_call(context);
-    lose("fake_foreign_call fell through");
+    lose("Unhandled SIGILL");
 }
+#endif /* not LISP_FEATURE_WIN32 */
 
 void
 arch_install_interrupt_handlers()
@@ -343,7 +339,6 @@ arch_install_interrupt_handlers()
     undoably_install_low_level_interrupt_handler(SIGILL , sigill_handler);
     undoably_install_low_level_interrupt_handler(SIGTRAP, sigtrap_handler);
 #endif
-
     SHOW("returning from arch_install_interrupt_handlers()");
 }
 
diff --git a/version.lisp-expr b/version.lisp-expr
index 9fe3ba6..2e3da09 100644
--- a/version.lisp-expr
+++ b/version.lisp-expr
@@ -17,4 +17,4 @@
 ;;; checkins which aren't released. (And occasionally for internal
 ;;; versions, especially for internal versions off the main CVS
 ;;; branch, it gets hairier, e.g. "0.pre7.14.flaky4.13".)
-"1.0.4.17"
+"1.0.4.18"