[m-rev.] for review: fix memory management bug
Fergus Henderson
fjh at cs.mu.OZ.AU
Tue Feb 10 02:39:39 AEDT 2004
Estimated hours taken: 4
Branches: main
Fix a nasty memory management problem, where we were allocating memory
with GC_malloc() and storing pointers to that memory in space allocated
with malloc(), which isn't traced by the conservative collector.
As a result the memory allocated by GC_malloc() got prematurely reclaimed.
runtime/mercury_array_macros.h:
Add MR_GC_ensure_room_for_next(), which is just like
MR_ensure_room_for_next() except that it allocates the
memory using MR_GC_malloc() rather than MR_malloc().
Add some comments warning about the memory management constraints.
runtime/mercury_tabling.c:
trace/mercury_trace.c:
trace/mercury_trace_tables.c:
Use the _GC version of MR_ensure_room_for_next().
This is required because the tables in question may
contain pointers to memory on the GC heap.
trace/mercury_trace_vars.c:
Use MR_free() rather than free() to deallocate memory allocated
with MR_copy_string().
Workspace: /home/jupiter/fjh/ws-jupiter/mercury
Index: runtime/mercury_array_macros.h
===================================================================
RCS file: /home/mercury1/repository/mercury/runtime/mercury_array_macros.h,v
retrieving revision 1.12
diff -u -d -r1.12 mercury_array_macros.h
--- runtime/mercury_array_macros.h 22 Nov 2002 08:50:42 -0000 1.12
+++ runtime/mercury_array_macros.h 9 Feb 2004 14:55:45 -0000
@@ -31,6 +31,23 @@
** If not, doubles the size of the existing widgets array, or
** allocates an array of INIT_SIZE items if the widgets array
** has not been initialized before.
+**
+** BEWARE: YOU NEED TO BE VERY CAREFUL OF POINTERS TO OBJECTS ON THE GC HEAP:
+**
+** - For MR_ensure_room_for_next(), the memory is allocated with MR_malloc(),
+** which means that if conservative GC is enabled, the array elements
+** _MUST NOT_ point to the GC heap (or at least must not be the _only_
+** pointer to any object on the GC heap); but the array address can be
+** stored anywhere.
+**
+** - For MR_GC_ensure_room_for_next(), the memory is allocated on the GC heap,
+** which means that the array elements can point to anything;
+** but the array address _MUST NOT_ be stored in memory allocated with
+** malloc() or MR_malloc().
+**
+** It is the caller's responsibility to deallocate the memory for the array
+** if/when it is no longer needed, using MR_free() or MR_GC_free()
+** respectively.
*/
#define MR_ensure_room_for_next(base, type, init) \
@@ -46,6 +63,19 @@
} \
} \
} while(0)
+#define MR_GC_ensure_room_for_next(base, type, init) \
+ do { \
+ if (base##_next >= base##_max) { \
+ if (base##_max == 0) { \
+ base##_max = (init); \
+ base##s = MR_GC_NEW_ARRAY(type, base##_max); \
+ } else { \
+ base##_max *= 2; \
+ base##s = MR_GC_RESIZE_ARRAY(base##s, type, \
+ base##_max); \
+ } \
+ } \
+ } while(0)
/*
** MR_ensure_big_enough makes the same assumptions as MR_ensure_room_for_next,
@@ -54,6 +84,9 @@
** is big enough to contain the element at index `slot'. Since with this regime
** there is no notion of the "next" slot, this macro does not access, nor does
** it require the existence of, base##_next.
+**
+** BEWARE: YOU NEED TO BE VERY CAREFUL OF POINTERS TO OBJECTS ON THE GC HEAP.
+** See the comment for MR_ensure_room_for_next().
*/
#define MR_ensure_big_enough(slot, base, type, init) \
@@ -76,6 +109,9 @@
** it resizes two arrays at once. These two arrays are named base##s1 and
** base##s2, and since they are always the same size, they share the
** base##_max variable.
+**
+** BEWARE: YOU NEED TO BE VERY CAREFUL OF POINTERS TO OBJECTS ON THE GC HEAP.
+** See the comment for MR_ensure_room_for_next().
*/
#define MR_ensure_big_enough2(slot, base, s1, s2, type, init) \
@@ -83,8 +119,8 @@
if ((slot) >= base##_max) { \
if (base##_max == 0) { \
base##_max = MR_max((init), (slot) + 1); \
- base##s1 = MR_NEW_ARRAY(type, base##_max); \
- base##s2 = MR_NEW_ARRAY(type, base##_max); \
+ base##s1 = MR_NEW_ARRAY(type, base##_max); \
+ base##s2 = MR_NEW_ARRAY(type, base##_max); \
} else { \
base##_max = MR_max(base##_max * 2, \
(slot) + 1); \
Index: runtime/mercury_tabling.c
===================================================================
RCS file: /home/mercury1/repository/mercury/runtime/mercury_tabling.c,v
retrieving revision 1.60
diff -u -d -r1.60 mercury_tabling.c
--- runtime/mercury_tabling.c 21 Jan 2004 02:52:37 -0000 1.60
+++ runtime/mercury_tabling.c 9 Feb 2004 15:04:28 -0000
@@ -641,7 +641,7 @@
for (bucket = 0; bucket < table->size; bucket++) { \
slot = table->hash_table[bucket].table_field; \
while (slot != NULL) { \
- MR_ensure_room_for_next(value, type_name, \
+ MR_GC_ensure_room_for_next(value, type_name, \
MR_INIT_HASH_CONTENTS_ARRAY_SIZE); \
values[value_next] = slot->key; \
value_next++; \
Index: trace/mercury_trace.c
===================================================================
RCS file: /home/mercury1/repository/mercury/trace/mercury_trace.c,v
retrieving revision 1.64
diff -u -d -r1.64 mercury_trace.c
--- trace/mercury_trace.c 30 Sep 2003 09:07:47 -0000 1.64
+++ trace/mercury_trace.c 9 Feb 2004 15:29:30 -0000
@@ -1376,7 +1376,7 @@
}
if (call_table != NULL) {
- MR_ensure_room_for_next(MR_call_table_ptr, MR_TrieNode,
+ MR_GC_ensure_room_for_next(MR_call_table_ptr, MR_TrieNode,
INIT_CALL_TABLE_ARRAY_SIZE);
MR_call_table_ptrs[MR_call_table_ptr_next] = call_table;
@@ -1435,7 +1435,7 @@
MR_abandon_call_table_array(void)
{
if (MR_call_table_ptrs != NULL) {
- MR_free(MR_call_table_ptrs);
+ MR_GC_free(MR_call_table_ptrs);
MR_call_table_ptrs = NULL;
}
}
Index: trace/mercury_trace_tables.c
===================================================================
RCS file: /home/mercury1/repository/mercury/trace/mercury_trace_tables.c,v
retrieving revision 1.25
diff -u -d -r1.25 mercury_trace_tables.c
--- trace/mercury_trace_tables.c 21 Jan 2004 04:55:50 -0000 1.25
+++ trace/mercury_trace_tables.c 9 Feb 2004 15:18:01 -0000
@@ -191,7 +191,7 @@
MR_bool found;
const char *nickname;
- MR_ensure_room_for_next(MR_module_info, const MR_Module_Layout *,
+ MR_GC_ensure_room_for_next(MR_module_info, const MR_Module_Layout *,
INIT_MODULE_TABLE_SIZE);
MR_prepare_insert_into_sorted(MR_module_infos, MR_module_info_next,
slot,
@@ -211,7 +211,7 @@
MR_module_nicks[slot].MR_nick_layouts,
module);
} else {
- MR_ensure_room_for_next(MR_module_nick,
+ MR_GC_ensure_room_for_next(MR_module_nick,
MR_Module_Nick, INIT_MODULE_TABLE_SIZE);
MR_prepare_insert_into_sorted(MR_module_nicks,
MR_module_nick_next,
Index: trace/mercury_trace_vars.c
===================================================================
RCS file: /home/mercury1/repository/mercury/trace/mercury_trace_vars.c,v
retrieving revision 1.55
diff -u -d -r1.55 mercury_trace_vars.c
--- trace/mercury_trace_vars.c 20 Oct 2003 07:29:55 -0000 1.55
+++ trace/mercury_trace_vars.c 9 Feb 2004 15:24:03 -0000
@@ -400,8 +400,8 @@
for (slot = 0; slot < MR_point.MR_point_var_count; slot++) {
/* free the memory allocated by previous MR_copy_string */
- free(MR_point.MR_point_vars[slot].MR_var_fullname);
- free(MR_point.MR_point_vars[slot].MR_var_basename);
+ MR_free(MR_point.MR_point_vars[slot].MR_var_fullname);
+ MR_free(MR_point.MR_point_vars[slot].MR_var_basename);
}
string_table = entry->MR_sle_module_layout->MR_ml_string_table;
--
Fergus Henderson <fjh at cs.mu.oz.au> | "I have always known that the pursuit
The University of Melbourne | of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh> | -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-reviews mailing list
post: mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------
More information about the reviews
mailing list