bug#6301: 23.2.50; GC may lose Lisp objects in the image cache

bug#6301: 23.2.50; GC may lose Lisp objects in the image cache

[YAMAMOTO Mitsuharu]

Currently, Fgarbage_collect calls mark_terminals after marking
staticpro'ed objects.  Terminal objects need a special marking
strategy with respect to image cache, but if a terminal object is
reachable from some staticpro'ed object, then it is marked normally
(i.e., without considering image cache).  As a result, Lisp objects in
the image cache might be collected though they should have been
marked.

The simplest way to fix this would be to call mark_terminals earlier.
Actually, the latest release of the Mac port includes the following
patch.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

=== modified file 'src/alloc.c'
*** src/alloc.c	2010-01-22 09:10:04 +0000
--- src/alloc.c	2010-05-29 07:58:30 +0000
*************** returns nil, because real GC can't be do
*** 5088,5093 ****
--- 5088,5097 ----
  
    /* Mark all the special slots that serve as the roots of accessibility.  */
  
+   /* Terminals need to be marked in a special way.  But they can be
+      reachable from other roots and might be marked normally if
+      mark_terminals is called later.  */
+   mark_terminals ();
    for (i = 0; i < staticidx; i++)
      mark_object (*staticvec[i]);
  
*************** returns nil, because real GC can't be do
*** 5096,5102 ****
        mark_object (bind->symbol);
        mark_object (bind->old_value);
      }
-   mark_terminals ();
    mark_kboards ();
    mark_ttys ();
  
--- 5100,5105 ----

bug#6301: 23.2.50; GC may lose Lisp objects in the image cache

[YAMAMOTO Mitsuharu]

>>>>> On Sat, 29 May 2010 17:37:43 +0900, YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> said:

> Currently, Fgarbage_collect calls mark_terminals after marking
> staticpro'ed objects.  Terminal objects need a special marking
> strategy with respect to image cache, but if a terminal object is
> reachable from some staticpro'ed object, then it is marked normally
> (i.e., without considering image cache).  As a result, Lisp objects in
> the image cache might be collected though they should have been
> marked.

> The simplest way to fix this would be to call mark_terminals earlier.

On second thought, the following change would be more natural.

=== modified file 'src/alloc.c'
*** src/alloc.c	2010-01-22 09:10:04 +0000
--- src/alloc.c	2010-10-10 01:56:51 +0000
*************** mark_terminals (void)
*** 5771,5783 ****
    for (t = terminal_list; t; t = t->next_terminal)
      {
        eassert (t->name != NULL);
-       if (!VECTOR_MARKED_P (t))
- 	{
  #ifdef HAVE_WINDOW_SYSTEM
! 	  mark_image_cache (t->image_cache);
  #endif /* HAVE_WINDOW_SYSTEM */
! 	  mark_vectorlike ((struct Lisp_Vector *)t);
! 	}
      }
  }
  
--- 5779,5789 ----
    for (t = terminal_list; t; t = t->next_terminal)
      {
        eassert (t->name != NULL);
  #ifdef HAVE_WINDOW_SYSTEM
!       mark_image_cache (t->image_cache);
  #endif /* HAVE_WINDOW_SYSTEM */
!       if (!VECTOR_MARKED_P (t))
! 	mark_vectorlike ((struct Lisp_Vector *)t);
      }
  }


Also, as for stack marking, I noticed that about half of the stack
elements have Lisp_Int* tags at least on Mac OS X.  Such an element
might be either a real Lisp integer or an aligned pointer value.  We
can omit mem_find calls for them by checking the tag value early.

=== modified file 'src/alloc.c'
*** src/alloc.c	2010-01-22 09:10:04 +0000
--- src/alloc.c	2010-10-10 01:56:51 +0000
*************** static INLINE void
*** 4109,4117 ****
  mark_maybe_object (obj)
       Lisp_Object obj;
  {
!   void *po = (void *) XPNTR (obj);
!   struct mem_node *m = mem_find (po);
  
    if (m != MEM_NIL)
      {
        int mark_p = 0;
--- 4109,4125 ----
  mark_maybe_object (obj)
       Lisp_Object obj;
  {
!   void *po;
!   struct mem_node *m;
! 
!   switch (XTYPE (obj))
!     {
!     case_Lisp_Int:
!       return;
!     }
  
+   po = (void *) XPNTR (obj);
+   m = mem_find (po);
    if (m != MEM_NIL)
      {
        int mark_p = 0;

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

bug#6301: 23.2.50; GC may lose Lisp objects in the image cache

[Andreas Schwab]

YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:

> --- 4109,4125 ----
>   mark_maybe_object (obj)
>        Lisp_Object obj;
>   {
> !   void *po;
> !   struct mem_node *m;
> ! 
> !   switch (XTYPE (obj))
> !     {
> !     case_Lisp_Int:
> !       return;
> !     }

aka. if (INTEGERP (obj)) return;

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#6301: 23.2.50; GC may lose Lisp objects in the image cache

[YAMAMOTO Mitsuharu]

>>>>> On Mon, 11 Oct 2010 19:21:25 +0200, Andreas Schwab <schwab@linux-m68k.org> said:

> YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:
>> --- 4109,4125 ----
>> mark_maybe_object (obj)
>> Lisp_Object obj;
>> {
>> !   void *po;
>> !   struct mem_node *m;
>> ! 
>> !   switch (XTYPE (obj))
>> !     {
>> !     case_Lisp_Int:
>> !       return;
>> !     }

> aka. if (INTEGERP (obj)) return;

Indeed.  And it actually produces a better code on gcc 4.2.1 with -O2.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

bug#6301: 23.2.50; GC may lose Lisp objects in the image cache

[Chong Yidong]

> Currently, Fgarbage_collect calls mark_terminals after marking
> staticpro'ed objects.  Terminal objects need a special marking
> strategy with respect to image cache, but if a terminal object is
> reachable from some staticpro'ed object, then it is marked normally
> (i.e., without considering image cache).  As a result, Lisp objects in
> the image cache might be collected though they should have been
> marked.
>
> Also, as for stack marking, I noticed that about half of the stack
> elements have Lisp_Int* tags at least on Mac OS X.  Such an element
> might be either a real Lisp integer or an aligned pointer value.  We
> can omit mem_find calls for them by checking the tag value early.

Thanks.  I've committed your patches (the former to emacs-23, the latter
to the trunk).  Sorry for the long delay.