[gs-cvs] rev 7666 - trunk/gs/src

leonardo at ghostscript.com leonardo at ghostscript.com
Thu Feb 1 09:38:00 PST 2007


Author: leonardo
Date: 2007-02-01 09:37:59 -0800 (Thu, 01 Feb 2007)
New Revision: 7666

Modified:
   trunk/gs/src/isave.c
   trunk/gs/src/isave.h
   trunk/gs/src/zvmem.c
Log:
Fix (memory management) : Propagate error codes from mark_allocated.

DETAILS :

This change is algorithmically equivalent,
except it returns error code rather than crash
when mark_allocated detects a heap damage.

The old code implemented "assert" with emitting a segfault.
However it's not fully portable,
because some platforms may have no memory protection at all.

The core of the change is in mark_allocated.
Others just an interface changes for passing error codes.

EXPECTED DIFFERENCES :

None.


Modified: trunk/gs/src/isave.c
===================================================================
--- trunk/gs/src/isave.c	2007-02-01 03:06:58 UTC (rev 7665)
+++ trunk/gs/src/isave.c	2007-02-01 17:37:59 UTC (rev 7666)
@@ -290,8 +290,8 @@
 /* Forward references */
 private int  restore_resources(alloc_save_t *, gs_ref_memory_t *);
 private void restore_free(gs_ref_memory_t *);
-private long save_set_new(gs_ref_memory_t *, bool, bool);
-private void save_set_new_changes(gs_ref_memory_t *, bool, bool);
+private int  save_set_new(gs_ref_memory_t * mem, bool to_new, bool set_limit, ulong *pscanned);
+private int  save_set_new_changes(gs_ref_memory_t *, bool, bool);
 #if NO_INVISIBLE_LEVELS
 private bool check_l_mark(void *obj);
 #endif
@@ -345,8 +345,8 @@
     /* Free any inner chunk structures.  This is the easiest way to do it. */
     restore_free(mem);
 }
-ulong
-alloc_save_state(gs_dual_memory_t * dmem, void *cdata)
+int
+alloc_save_state(gs_dual_memory_t * dmem, void *cdata, ulong *psid)
 {
     gs_ref_memory_t *lmem = dmem->space_local;
     gs_ref_memory_t *gmem = dmem->space_global;
@@ -379,8 +379,11 @@
     /* can have the attribute set are the ones on the changes chain, */
     /* and ones in objects allocated since the last save. */
     if (lmem->save_level > 1) {
-	long scanned = save_set_new(&lsave->state, false, true);
+	ulong scanned;
+	int code = save_set_new(&lsave->state, false, true, &scanned);
 
+	if (code < 0)
+	    return code;
 #if !NO_INVISIBLE_LEVELS
 	if ((lsave->state.total_scanned += scanned) > max_repeated_scan) {
 	    /* Do a second, invisible save. */
@@ -414,7 +417,8 @@
 #endif
     }
     alloc_set_in_save(dmem);
-    return sid;
+    *psid = sid;
+    return 0;
 }
 /* Save the state of one space (global or local). */
 private alloc_save_t *
@@ -820,7 +824,11 @@
 	}
 	alloc_set_not_in_save(dmem);
     } else {			/* Set the l_new attribute in all slots that are now new. */
-	save_set_new(mem, true, false);
+	ulong scanned;
+
+	code = save_set_new(mem, true, false, &scanned);
+	if (code < 0)
+	    return code;
     }
 
     return sprev == save;
@@ -1006,11 +1014,13 @@
 private void file_forget_save(gs_ref_memory_t *);
 private void combine_space(gs_ref_memory_t *);
 private void forget_changes(gs_ref_memory_t *);
-void
+int
 alloc_forget_save_in(gs_dual_memory_t *dmem, alloc_save_t * save)
 {
     gs_ref_memory_t *mem = save->space_local;
     alloc_save_t *sprev;
+    ulong scanned;
+    int code;
 
     print_save("forget_save", mem->space, save);
 
@@ -1022,7 +1032,9 @@
 	if (mem->save_level != 0) {
 	    alloc_change_t *chp = mem->changes;
 
-	    save_set_new(&sprev->state, true, false);
+	    code = save_set_new(&sprev->state, true, false, &scanned);
+	    if (code < 0)
+		return code;
 	    /* Concatenate the changes chains. */
 	    if (chp == 0)
 		mem->changes = sprev->state.changes;
@@ -1035,7 +1047,9 @@
 	    combine_space(mem);	/* combine memory */
 	} else {
 	    forget_changes(mem);
-	    save_set_new(mem, false, false);
+	    code = save_set_new(mem, false, false, &scanned);
+	    if (code < 0)
+		return code;
 	    file_forget_save(mem);
 	    combine_space(mem);	/* combine memory */
 	    /* This is the outermost save, which might also */
@@ -1043,7 +1057,9 @@
 	    mem = save->space_global;
 	    if (mem != save->space_local && mem->saved != 0) {
 		forget_changes(mem);
-		save_set_new(mem, false, false);
+		code = save_set_new(mem, false, false, &scanned);
+		if (code < 0)
+		    return code;
 		file_forget_save(mem);
 		combine_space(mem);
 	    }
@@ -1052,6 +1068,7 @@
 	}
     }
     while (sprev != save);
+    return 0;
 }
 /* Combine the chunks of the next outer level with those of the current one, */
 /* and free the bookkeeping structures. */
@@ -1179,8 +1196,8 @@
     }
 }
 
-private inline uint
-mark_allocated(void *obj, bool to_new)
+private inline int
+mark_allocated(void *obj, bool to_new, uint *psize)
 {   
     obj_header_t *pre = (obj_header_t *)obj - 1;
     uint size = pre_obj_contents_size(pre);
@@ -1194,11 +1211,9 @@
 #endif
 
     if (pre->o_type != &st_refs) {
-	/* Must not happen. Can't continue. Emit a crash. */
-	void *p = *(void **)0;
-
-	mark_allocated(p, false); /* a non-trivial use of p to defeat
-				     code optimization. */
+	/* Must not happen. */
+	if_debug0('u', "Wrong object type when expected a ref.\n");
+	return_error(e_Fatal);
     }
     /* We know that every block of refs ends with */
     /* a full-size ref, so we only need the end check */
@@ -1225,7 +1240,8 @@
 	    }
 	}
 #undef RP_REF
-    return size;
+    *psize = size;
+    return 0;
 }
 
 #if NO_INVISIBLE_LEVELS
@@ -1268,13 +1284,16 @@
 /* This includes every slot on the current change chain, */
 /* and every (ref) slot allocated at this save level. */
 /* Return the number of bytes of data scanned. */
-private long
-save_set_new(gs_ref_memory_t * mem, bool to_new, bool set_limit)
+private int
+save_set_new(gs_ref_memory_t * mem, bool to_new, bool set_limit, ulong *pscanned)
 {
-    long scanned = 0;
+    ulong scanned = 0;
+    int code;
 
     /* Handle the change chain. */
-    save_set_new_changes(mem, to_new, set_limit);
+    code = save_set_new_changes(mem, to_new, set_limit);
+    if (code < 0)
+	return code;
 
 #if !NO_INVISIBLE_LEVELS
     /* Handle newly allocated ref objects. */
@@ -1289,8 +1308,12 @@
 	    if (pre->o_type == &st_refs) {
 		/* These are refs, scan them. */
 		ref_packed *prp = (ref_packed *) (pre + 1);
-
-		scanned += mark_allocated(prp, to_new);
+		uint size;
+		
+		code = mark_allocated(prp, to_new, &size);
+		if (code < 0)
+		    return code;
+		scanned += size;
 	    } else
 		scanned += sizeof(obj_header_t);
 	    END_OBJECTS_SCAN
@@ -1301,24 +1324,31 @@
 	if_debug2('u', "[u]set_new (%s) scanned %ld\n",
 		  (to_new ? "restore" : "save"), scanned);
 #endif
-    return scanned;
+    *pscanned = scanned;
+    return 0;
 }
 
 /* Set or reset the l_new attribute on the changes chain. */
-private void
+private int
 save_set_new_changes(gs_ref_memory_t * mem, bool to_new, bool set_limit)
 {
     register alloc_change_t *chp = mem->changes;
     register uint new = (to_new ? l_new : 0);
 #if NO_INVISIBLE_LEVELS
-    long scanned = mem->total_scanned;
+    ulong scanned = mem->total_scanned;
 #endif
 
     for (; chp; chp = chp->next) {
 #if NO_INVISIBLE_LEVELS
 	if (chp->offset == AC_OFFSET_ALLOCATED) {
-	    if (chp->where != 0)
-		scanned += mark_allocated((void *)chp->where, to_new);
+	    if (chp->where != 0) {
+		uint size;
+		int code = mark_allocated((void *)chp->where, to_new, &size);
+
+		if (code < 0)
+		    return code;
+		scanned += size;
+	    }
 	} else
 #endif
 	{
@@ -1347,4 +1377,5 @@
 	    mem->total_scanned = scanned;
     }
 #endif
+    return 0;
 }

Modified: trunk/gs/src/isave.h
===================================================================
--- trunk/gs/src/isave.h	2007-02-01 03:06:58 UTC (rev 7665)
+++ trunk/gs/src/isave.h	2007-02-01 17:37:59 UTC (rev 7666)
@@ -52,7 +52,7 @@
  * otherwise return the save ID.  The second argument is a client data
  * pointer, assumed to point to an object.
  */
-ulong alloc_save_state(gs_dual_memory_t *, void *);
+int alloc_save_state(gs_dual_memory_t * dmem, void *cdata, ulong *psid);
 
 /* Get the client pointer passed to alloc_saved_state. */
 void *alloc_save_client_data(const alloc_save_t *);
@@ -90,7 +90,7 @@
  * by calling alloc_find_save.  Note that forgetting a save does not
  * require checking pointers for recency.
  */
-void alloc_forget_save_in(gs_dual_memory_t *, alloc_save_t *);
+int alloc_forget_save_in(gs_dual_memory_t *, alloc_save_t *);
 /* Backward compatibility */
 #define alloc_forget_save(save) alloc_forget_save_in(idmemory, save)
 

Modified: trunk/gs/src/zvmem.c
===================================================================
--- trunk/gs/src/zvmem.c	2007-02-01 03:06:58 UTC (rev 7665)
+++ trunk/gs/src/zvmem.c	2007-02-01 17:37:59 UTC (rev 7666)
@@ -74,7 +74,9 @@
     ialloc_set_space(idmemory, space);
     if (vmsave == 0)
 	return_error(e_VMerror);
-    sid = alloc_save_state(idmemory, vmsave);
+    code = alloc_save_state(idmemory, vmsave, &sid);
+    if (code < 0)
+	return code;
     if (sid == 0) {
 	ifree_object(vmsave, "zsave");
 	return_error(e_VMerror);
@@ -405,7 +407,9 @@
 	gs_grestore(last);
     }
     /* Forget the save in the memory manager. */
-    alloc_forget_save(asave);
+    code = alloc_forget_save(asave);
+    if (code < 0)
+	return code;
     {
 	uint space = icurrent_space;
 



More information about the gs-cvs mailing list