[Sbcl-devel] gencgc patch

[Cyrus H. <ch...@bo...>]

I'm hoping our resident GC experts can look at this patch and let me  
know if this sounds reasonable. It seems to be the right thing, but  
I'm not quite sure.

Thanks,

Cyrus

--- gencgc.c        23 May 2006 06:36:40 -0700        1.99.2.3
+++ gencgc.c        23 May 2006 06:51:40 -0700
@@ -4090,36 +4090,44 @@
              void **esp=(void **)-1;
  #ifdef LISP_FEATURE_SB_THREAD
              long i,free;
+            void **esp1;
+
              if(th==arch_os_get_current_thread()) {
                  /* Somebody is going to burn in hell for this, but  
casting
                   * it in two steps shuts gcc up about strict  
aliasing. */
                  esp = (void **)((void *)&raise);
-            } else {
-                void **esp1;
-                free=fixnum_value(SymbolValue 
(FREE_INTERRUPT_CONTEXT_INDEX,th));
-                for(i=free-1;i>=0;i--) {
-                    os_context_t *c=th->interrupt_contexts[i];
-                    esp1 = (void **) *os_context_register_addr 
(c,reg_SP);
-                    if (esp1>=(void **)th->control_stack_start &&
-                        esp1<(void **)th->control_stack_end) {
-                        if(esp1<esp) esp=esp1;
-                        /* Ok, so the problem here is that on linux
-                           the registers are on the stack. My hunch is
-                           that on Darwin, while the context is on the
-                           stack, the registers are actually elsewhere
-                           in the ss struct. */
+            }
+
+            free=fixnum_value(SymbolValue 
(FREE_INTERRUPT_CONTEXT_INDEX,th));
+            for(i=free-1;i>=0;i--) {
+                os_context_t *c=th->interrupt_contexts[i];
+                esp1 = (void **) *os_context_register_addr(c,reg_SP);
+                if (esp1>=(void **)th->control_stack_start &&
+                    esp1<(void **)th->control_stack_end) {
+                    if(esp1<esp) esp=esp1;
+                    /* On Darwin, the context contains a pointer to  
another
+                     * struct elsewhere on the stack that contains  
the actual
+                     * register values. Therefore, we need to not  
just preserve
+                     * the context, but also the register values in the
+                     * i386_thread_state pointed to by the context.  
Fortunately,
+                     * we can just use os_context_reg_addr to get  
the address of
+                     * these.
+                     * FIXME: Do we need to preserver the context as  
well?
+                     *        Since we get the register values, do  
we need to
+                     *        worry about the preserving the values  
in the
+                     *        context itself?
+                     */
  #if defined(LISP_FEATURE_DARWIN)
-                        preserve_pointer((void*) 
*os_context_register_addr(c,reg_EAX));
-                        preserve_pointer((void*) 
*os_context_register_addr(c,reg_ECX));
-                        preserve_pointer((void*) 
*os_context_register_addr(c,reg_EDX));
-                        preserve_pointer((void*) 
*os_context_register_addr(c,reg_EBX));
-                        preserve_pointer((void*) 
*os_context_register_addr(c,reg_ESI));
-                        preserve_pointer((void*) 
*os_context_register_addr(c,reg_EDI));
-                        preserve_pointer((void*)*os_context_pc_addr 
(c));
-#endif
-                        for(ptr = (void **)(c+1); ptr>=(void **)c;  
ptr--) {
-                            preserve_pointer(*ptr);
-                        }
+                    preserve_pointer((void*)*os_context_register_addr 
(c,reg_EAX));
+                    preserve_pointer((void*)*os_context_register_addr 
(c,reg_ECX));
+                    preserve_pointer((void*)*os_context_register_addr 
(c,reg_EDX));
+                    preserve_pointer((void*)*os_context_register_addr 
(c,reg_EBX));
+                    preserve_pointer((void*)*os_context_register_addr 
(c,reg_ESI));
+                    preserve_pointer((void*)*os_context_register_addr 
(c,reg_EDI));
+                    preserve_pointer((void*)*os_context_pc_addr(c));
+#endif
+                    for(ptr = (void **)(c+1); ptr>=(void **)c; ptr--) {
+                        preserve_pointer(*ptr);
                      }
                  }
              }