[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Fixes to AKCL



William Shelter,

I have identified some problems with the AKCL Lisp package at our site.
The changes I propose have been incorporated into our package and are 
working fine.  There are two areas where changes have been made, 1)
the Lisp reader when patching the self references and 2) the garbage
collector for the handling of displaced arrays.  Both problems were 
causing segmentation errors.  I hope the information I have provided here
will help you in correcting these problems for future releases.


                  Bruce Westergren  
                  AT&T Bell Labs
                  il/ihl/ihlpf!wester



AKCL Version : 1.189

Starts dribbling to akcl.out (1989/9/21, 9:44:55).
NIL

>
"This will demonstrate the garbage collector errors for displaced arrays"
"This will demonstrate the garbage collector errors for displaced arrays"

>
(setf a (make-array 10))
#(NIL NIL NIL NIL NIL NIL NIL NIL NIL NIL)

>
(make-array 5 :displaced-to a)
#(NIL NIL NIL NIL NIL)

>
(gbc 2)
2

>
(gbc 2)

The second gbc causes a segmentation error and a core dump.

-----------------------------------------------------------------------

This is the code segment from array.c which shows the checking
for the FROM array and the TO array CONS (which points to FROM)
being FREE.  This is done to increase efficiency by avoiding
the undisplacement process, however the TO array CONS pointers
need to be readjusted if there are other arrays displaced.  If this
is not done segmentation errors will occur.  Since we are already
here it is efficient to just mark the FROM array CONS and TO array
CONS as FREE.  This will eliminate some steps in the sweep phase.

undisplace(from)
object from;
{
	object *p;
	object to;
	
	  /* if the cons is free, neither the FROM nor the TO array will
	     survive the gc (or we would have marked this), and we can
	     skip undisplacing */
	
/*	if (from->a.a_displaced->d.m == FREE) return;    */
	to= from->a.a_displaced->c.c_car;
	
	if (to == Cnil)
		return;
	from->a.a_displaced->c.c_car = Cnil;
        from->a.a_displaced->c.m = FREE;
	for (p = &(to->a.a_displaced->c.c_cdr);;  p = &((*p)->c.c_cdr)){

/*	  if ((*p)->d.m == FREE) return;         */

	  if ((*p)->c.c_car == from) {
            (*p)->c.m = FREE;
	    *p = (*p)->c.c_cdr;
	    return;
          }
        }
}


This is the macro from gbc.c which marks the arrays. Below are the 
the original and the corrected routines.  The first problem is a
syntax error when attempting to mark the first cons.  The == should
be replaced with a single =.  The second problem is the marking of
all FROM arrays if the TO array is being marked.  The FROM array
is not necessarily a valid object and should be marked only if
it is called from the top level.  Marking the from arrays
will keep them around when they could be swept.

******************  ORIGINAL MACRO  ***************************

#define mark_displaced_field(ar) \
do{register object y=ar->a.a_displaced; \
   /* mark the array pointed to by ar */  \
   if (y->c.c_car != Cnil) mark_object(y->c.c_car); \
   y->d.m=TRUE; \
   while ((y=y->c.c_cdr) != Cnil) \
     { y->d.m==TRUE ; \
       /* mark the first cons (which holds the pointer to ar) \
	  of each array pointing to ar    \
	  so that undisplace will be able to work */ \
       y->c.c_car->a.a_displaced->d.m=TRUE; \
     }} while (0)

******************  CORRECTED MACRO  **************************

#define mark_displaced_field(ar)           \
do { register object y=ar->a.a_displaced;  \
     y->d.m = TRUE;                        \
     if (y->c.c_car != Cnil)               \
       mark_object(y->c.c_car);            \
     while ((y=y->c.c_cdr) != Cnil)        \
       y->c.m = TRUE;                      \
   } while(0)

----------------------------------------------------------------------

Starts dribbling to sharp.out (1989/9/21, 10:37:58).
NIL

>
"This will illustrate the self reference problem in arrays and vectors
with element-types other than object."
"This will illustrate the self reference problem in arrays and vectors
with element-types other than object."

>
(setf b (make-array '(5 5) :element-type 'bit))
#2A((0 0 0 0 0) (0 0 0 0 0) (0 0 0 0 0) (0 0 0 0 0) (0 0 0 0 0))

>
#,b
#2A((0 0 0 0 0) (0 0 0 0 0) (0 0 0 0 0) (0 0 0 0 0) (0 0 0 0 0))

>#0=#,b
A segmentation violation occurs at this point.

The reader should not be allowed to parse self referencing structures
in arrays and vectors with elements types of bit, string, fixnum,
short-float and long-float.  This will cause segmentation type errors
and core dumps.  A simple check in the patch sharp function in the
cases for t_array and t_vector for a valid object type is required.
The code below illustrates the correction.

object
patch_sharp(x)
object x;
{
	cs_check(x);

	switch (type_of(x)) {
	case t_spice:
	{
		int i;

		for (i = 0;  i < sharp_eq_context_max;  i++)
			if (sharp_eq_context[i].sharp_sharp == x)
				return(sharp_eq_context[i].sharp_eq);
		break;
	}
	case t_cons:
	/*
		x->c.c_car = patch_sharp(x->c.c_car);
		x->c.c_cdr = patch_sharp(x->c.c_cdr);
	*/
		patch_sharp_cons(x);
		break;

	case t_vector:
	{
		int i;

		if ((enum aelttype)x->v.v_elttype != aet_object)
                  break;

		for (i = 0;  i < x->v.v_fillp;  i++)
			x->v.v_self[i] = patch_sharp(x->v.v_self[i]);
		break;
	}
	case t_array:
	{
		int i, j;

		if ((enum aelttype)x->a.a_elttype != aet_object)
                  break;

		for (i = 0, j = 1;  i < x->a.a_rank;  i++)
			j *= x->a.a_dims[i];
		for (i = 0;  i < j;  i++)
			x->a.a_self[i] = patch_sharp(x->a.a_self[i]);
		break;
	}
        case t_structure:
        {
                struct s_data *def=S_DATA(x->str.str_def);
                int i, max_slots;

                max_slots = def->length;

                for ( i = 0 ; i < max_slots ; i++ )
                   structure_set(x, 
                                 x->str.str_def,
                                 i,
                                 patch_sharp(structure_ref(x,
                                                           x->str.str_def,
                                                           i)));
                break;
        }
	}
	return(x);
}

It is also desirable for the defstruct to return the structure name
as opposed to the constructor.  This is easily changed by adding the
`,name end of the definition.

There are currently two places where the sharp-s-reader is defined, 
iolib.lsp and defstruct.lsp.  It would be nice to eliminate the
definition from defstruct.lsp to avoid any confusion.



~c iexist!lgm iexist!clarisse ihlpf!ian