Expanding macros in igc.c and a possible fix for 690176.

View: New views

1 Messages — Rating Filter: Alert me

Expanding macros in igc.c and a possible fix for 690176.

by Alex Cherepanov :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

This submission has 2 parts. The first part expands some of macro
definitions in igc.c to make it easier to read. This change is
expected to be fully equivalent to the original version. The second
part is a small patch on top of the 1st one that fixes bug 690176.

After macro expansion it clear that local garbage collection
(i.e. global flag is off) clears marks only for the objects
allocated by local allocators but marks objects starting from
global allocators.

Clearing marks in all objects prevents access to uninitialized
marks and associated warnings. I don't yet know whether uninitialized
marks in allocators can cause any heap corruption. This patch
should be carefully reviewed because it makes GC to to more work.

--- igc.c.expanded 2009-08-10 00:51:07.000000000 -0400
+++ igc.c 2009-08-10 00:53:46.000000000 -0400
@@ -230,8 +230,8 @@
goto no_collect;
}

- /* Clear marks in spaces to be collected. */
- for (ispace = min_collect; ispace <= max_trace; ++ispace) {
+ /* Clear marks in all spaces. */
+ for (ispace = 1; ispace <= max_trace; ++ispace) {
for (mem = space_memories[ispace]; mem != 0; mem = &mem->saved->state) {
for (cp = mem->cfirst; cp != 0; cp = cp->cnext) {
gc_objects_clear_marks((const gs_memory_t *)mem, cp);

Index: gs/psi/igc.c
===================================================================
--- gs/psi/igc.c (revision 9930)
+++ gs/psi/igc.c (working copy)
@@ -198,24 +198,6 @@
if (global)
min_collect = min_collect_vm_space = 1;

-#define for_spaces(i, n)\
- for (i = 1; i <= n; ++i)
-#define for_collected_spaces(i)\
- for (i = min_collect; i <= max_trace; ++i)
-#define for_space_mems(i, mem)\
- for (mem = space_memories[i]; mem != 0; mem = &mem->saved->state)
-#define for_mem_chunks(mem, cp)\
- for (cp = (mem)->cfirst; cp != 0; cp = cp->cnext)
-#define for_space_chunks(i, mem, cp)\
- for_space_mems(i, mem) for_mem_chunks(mem, cp)
-#define for_chunks(n, mem, cp)\
- for_spaces(ispace, n) for_space_chunks(ispace, mem, cp)
-#define for_collected_chunks(mem, cp)\
- for_collected_spaces(ispace) for_space_chunks(ispace, mem, cp)
-#define for_roots(n, mem, rp)\
- for_spaces(ispace, n)\
- for (mem = space_memories[ispace], rp = mem->roots; rp != 0; rp = rp->next)
-
/* Initialize the state. */

state.procs = &igc_procs;
@@ -231,22 +213,17 @@
/* Register the allocators themselves as roots, */
/* so we mark and relocate the change and save lists properly. */

- for_spaces(ispace, max_trace)
+ for (ispace = 1; ispace <= max_trace; ++ispace) {
gs_register_struct_root((gs_memory_t *)space_memories[ispace],
- &space_roots[ispace],
- (void **)&space_memories[ispace],
- "gc_top_level");
-
+ &space_roots[ispace],
+ (void **)&space_memories[ispace], "gc_top_level");
+ }
end_phase("register space roots");

#ifdef DEBUG
-
/* Pre-validate the state. This shouldn't be necessary.... */
-
gc_validate_spaces(space_memories, max_trace, &state);
-
end_phase("pre-validate pointers");
-
#endif

if (I_BYPASS_GC) { /* Don't collect at all. */
@@ -254,26 +231,28 @@
}

/* Clear marks in spaces to be collected. */
-
- for_collected_spaces(ispace)
- for_space_chunks(ispace, mem, cp) {
- gc_objects_clear_marks((const gs_memory_t *)mem, cp);
- gc_strings_set_marks(cp, false);
- }
-
+ for (ispace = min_collect; ispace <= max_trace; ++ispace) {
+ for (mem = space_memories[ispace]; mem != 0; mem = &mem->saved->state) {
+ for (cp = mem->cfirst; cp != 0; cp = cp->cnext) {
+ gc_objects_clear_marks((const gs_memory_t *)mem, cp);
+ gc_strings_set_marks(cp, false);
+ }
+ }
+ }
end_phase("clear chunk marks");

/* Clear the marks of roots. We must do this explicitly, */
/* since some roots are not in any chunk. */

- for_roots(max_trace, mem, rp) {
- enum_ptr_t eptr;
+ for (ispace = 1; ispace <= max_trace; ++ispace) {
+ for (mem = space_memories[ispace], rp = mem->roots; rp != 0; rp = rp->next) {
+ enum_ptr_t eptr;

- eptr.ptr = *rp->p;
- if_debug_root('6', "[6]unmarking root", rp);
- (*rp->ptype->unmark)(&eptr, &state);
+ eptr.ptr = *rp->p;
+ if_debug_root('6', "[6]unmarking root", rp);
+ (*rp->ptype->unmark)(&eptr, &state);
+ }
}
-
end_phase("clear root marks");

if (global)
@@ -287,65 +266,73 @@

/* Add all large-enough free blocks to the mark stack. */
/* Also initialize the rescan pointers. */
+ { gc_mark_stack *end = mark_stack;

- {
- gc_mark_stack *end = mark_stack;
+ for (ispace = 1; ispace <= max_trace; ++ispace) {
+ for (mem = space_memories[ispace]; mem != 0; mem = &mem->saved->state) {
+ for (cp = mem->cfirst; cp != 0; cp = cp->cnext) {
+ uint avail = cp->ctop - cp->cbot;

- for_chunks(max_trace, mem, cp) {
- uint avail = cp->ctop - cp->cbot;
+ if (avail >= sizeof(gc_mark_stack) + sizeof(ms_entry)*ms_size_min &&
+ !cp->inner_count
+ ) {
+ gc_mark_stack *pms = (gc_mark_stack *) cp->cbot;

- if (avail >= sizeof(gc_mark_stack) + sizeof(ms_entry) *
- ms_size_min &&
- !cp->inner_count
- ) {
- gc_mark_stack *pms = (gc_mark_stack *) cp->cbot;
-
- gc_init_mark_stack(pms, (avail - sizeof(gc_mark_stack)) /
- sizeof(ms_entry));
- end->next = pms;
- pms->prev = end;
- pms->on_heap = false;
- if_debug2('6', "[6]adding free 0x%lx(%u) to mark stack\n",
- (ulong) pms, pms->count);
+ gc_init_mark_stack(pms, (avail - sizeof(gc_mark_stack)) /
+ sizeof(ms_entry));
+ end->next = pms;
+ pms->prev = end;
+ pms->on_heap = false;
+ if_debug2('6', "[6]adding free 0x%lx(%u) to mark stack\n",
+ (ulong) pms, pms->count);
+ }
+ cp->rescan_bot = cp->cend;
+ cp->rescan_top = cp->cbase;
+ }
}
- cp->rescan_bot = cp->cend;
- cp->rescan_top = cp->cbase;
- }
+ }
}

/* Mark reachable objects. */
-
{
int more = 0;

/* Mark from roots. */
-
- for_roots(max_trace, mem, rp) {
- if_debug_root('6', "[6]marking root", rp);
- more |= gc_trace(rp, &state, mark_stack);
- }
-
+ for (ispace = 1; ispace <= max_trace; ++ispace) {
+ for (mem = space_memories[ispace], rp = mem->roots; rp != 0; rp = rp->next) {
+ if_debug_root('6', "[6]marking root", rp);
+ more |= gc_trace(rp, &state, mark_stack);
+ }
+ }
end_phase("mark");

/* If this is a local GC, mark from non-local chunks. */
+ if (!global) {
+ for (ispace = 1; ispace <= min_collect - 1; ++ispace) {
+ for (mem = space_memories[ispace]; mem != 0; mem = &mem->saved->state) {
+ for (cp = mem->cfirst; cp != 0; cp = cp->cnext) {
+ more |= gc_trace_chunk((const gs_memory_t *)mem, cp, &state, mark_stack);
+ }
+ }
+ }
+ }

- if (!global)
- for_chunks(min_collect - 1, mem, cp)
- more |= gc_trace_chunk((const gs_memory_t *)mem, cp, &state, mark_stack);
-
/* Handle mark stack overflow. */
-
while (more < 0) { /* stack overflowed */
more = 0;
- for_chunks(max_trace, mem, cp)
- more |= gc_rescan_chunk(cp, &state, mark_stack);
+ for (ispace = 1; ispace <= max_trace; ++ispace) {
+ for (mem = space_memories[ispace]; mem != 0; mem = &mem->saved->state) {
+ for (cp = mem->cfirst; cp != 0; cp = cp->cnext) {
+ more |= gc_rescan_chunk(cp, &state, mark_stack);
+ }
+ }
+ }
}

end_phase("mark overflow");
}

/* Free the mark stack. */
-
{
gc_mark_stack *pms = mark_stack;

@@ -372,29 +359,34 @@
end_phase("finish trace");
}

- /* Filter save change lists with removing elements,
- which point to unmarked blocks of refs. */
- {
- int i;
+ /* Filter save change lists with removing elements, */
+ /* which point to unmarked blocks of refs. */
+ for (ispace = min_collect; ispace <= max_trace; ++ispace) {
+ gs_ref_memory_t *mem = space_memories[ispace];

- for_collected_spaces(i) {
- gs_ref_memory_t *mem = space_memories[i];
-
- alloc_save__filter_changes(mem);
- }
+ alloc_save__filter_changes(mem);
}
+
/* Clear marks and relocation in spaces that are only being traced. */
/* We have to clear the marks first, because we want the */
/* relocation to wind up as o_untraced, not o_unmarked. */

- for_chunks(min_collect - 1, mem, cp)
- gc_objects_clear_marks((const gs_memory_t *)mem, cp);
-
+ for (ispace = 1; ispace <= min_collect - 1; ++ispace) {
+ for (mem = space_memories[ispace]; mem != 0; mem = &mem->saved->state) {
+ for (cp = mem->cfirst; cp != 0; cp = cp->cnext) {
+ gc_objects_clear_marks((const gs_memory_t *)mem, cp);
+ }
+ }
+ }
end_phase("post-clear marks");

- for_chunks(min_collect - 1, mem, cp)
- gc_clear_reloc(cp);
-
+ for (ispace = 1; ispace <= min_collect - 1; ++ispace) {
+ for (mem = space_memories[ispace]; mem != 0; mem = &mem->saved->state) {
+ for (cp = mem->cfirst; cp != 0; cp = cp->cnext) {
+ gc_clear_reloc(cp);
+ }
+ }
+ }
end_phase("clear reloc");

/* Set the relocation of roots outside any chunk to o_untraced, */
@@ -403,43 +395,49 @@

/* Disable freeing in the allocators of the spaces we are */
/* collecting, so finalization procedures won't cause problems. */
- {
- int i;
+ for (ispace = min_collect; ispace <= max_trace; ++ispace)
+ gs_enable_free((gs_memory_t *)space_memories[ispace], false);

- for_collected_spaces(i)
- gs_enable_free((gs_memory_t *)space_memories[i], false);
- }
-
/* Compute relocation based on marks, in the spaces */
/* we are going to compact. Also finalize freed objects. */
-
- for_collected_chunks(mem, cp) {
- gc_objects_set_reloc(&state, cp);
- gc_strings_set_reloc(cp);
+ for (ispace = min_collect; ispace <= max_trace; ++ispace) {
+ for (mem = space_memories[ispace]; mem != 0; mem = &mem->saved->state) {
+ for (cp = mem->cfirst; cp != 0; cp = cp->cnext) {
+ gc_objects_set_reloc(&state, cp);
+ gc_strings_set_reloc(cp);
+ }
+ }
}

/* Re-enable freeing. */
- {
- int i;
+ for (ispace = min_collect; ispace <= max_trace; ++ispace)
+ gs_enable_free((gs_memory_t *)space_memories[ispace], true);

- for_collected_spaces(i)
- gs_enable_free((gs_memory_t *)space_memories[i], true);
- }
-
end_phase("set reloc");

/* Relocate pointers. */

state.relocating_untraced = true;
- for_chunks(min_collect - 1, mem, cp)
- gc_do_reloc(cp, mem, &state);
- state.relocating_untraced = false;
- for_collected_chunks(mem, cp)
- gc_do_reloc(cp, mem, &state);
+ for (ispace = 1; ispace <= min_collect - 1; ++ispace) {
+ for (mem = space_memories[ispace]; mem != 0; mem = &mem->saved->state) {
+ for (cp = mem->cfirst; cp != 0; cp = cp->cnext) {
+ gc_do_reloc(cp, mem, &state);
+ }
+ }
+ }

+ state.relocating_untraced = false;
+ for (ispace = min_collect; ispace <= max_trace; ++ispace) {
+ for (mem = space_memories[ispace]; mem != 0; mem = &mem->saved->state) {
+ for (cp = mem->cfirst; cp != 0; cp = cp->cnext) {
+ gc_do_reloc(cp, mem, &state);
+ }
+ }
+ }
end_phase("relocate chunks");

- for_roots(max_trace, mem, rp) {
+ for (ispace = 1; ispace <= max_trace; ++ispace)
+ for (mem = space_memories[ispace], rp = mem->roots; rp != 0; rp = rp->next) {
if_debug3('6', "[6]relocating root 0x%lx: 0x%lx -> 0x%lx\n",
(ulong) rp, (ulong) rp->p, (ulong) * rp->p);
if (rp->ptype == ptr_ref_type) {
@@ -458,9 +456,9 @@

/* Compact data. We only do this for spaces we are collecting. */

- for_collected_spaces(ispace) {
- for_space_mems(ispace, mem) {
- for_mem_chunks(mem, cp) {
+ for (ispace = min_collect; ispace <= max_trace; ++ispace) {
+ for (mem = space_memories[ispace]; mem != 0; mem = &mem->saved->state) {
+ for (cp = mem->cfirst; cp != 0; cp = cp->cnext) {
if_debug_chunk('6', "[6]compacting chunk", cp);
gc_objects_compact(cp, &state);
gc_strings_compact(cp);
@@ -477,8 +475,8 @@

/* Free empty chunks. */

- for_collected_spaces(ispace) {
- for_space_mems(ispace, mem) {
+ for (ispace = min_collect; ispace <= max_trace; ++ispace) {
+ for (mem = space_memories[ispace]; mem != 0; mem = &mem->saved->state) {
gc_free_empty_chunks(mem);
}
}
@@ -494,7 +492,7 @@
* information local to the individual save level.)
*/

- for_collected_spaces(ispace) { /* Reverse the pointers. */
+ for (ispace = min_collect; ispace <= max_trace; ++ispace) { /* Reverse the pointers. */
alloc_save_t *curr;
alloc_save_t *prev = 0;
alloc_save_t *next;
@@ -534,7 +532,7 @@

/* Unregister the allocator roots. */

- for_spaces(ispace, max_trace)
+ for (ispace = 1; ispace <= max_trace; ++ispace)
gs_unregister_root((gs_memory_t *)space_memories[ispace],
&space_roots[ispace], "gc_top_level");

_______________________________________________
gs-code-review mailing list
gs-code-review@...
http://www.ghostscript.com/mailman/listinfo/gs-code-review