diff: fix debugger hp allocation bug

Fergus Henderson fjh at cs.mu.OZ.AU
Fri Nov 6 21:35:13 AEDT 1998


I haven't tested this very well yet, but I plan to commit it anyway,
so that the nightly tests will test it in a variety of grades
on different architectures.  Even if this change doesn't fix
all the problems, it at least does a good job of abstracting
the code, so that there should be much fewer places we need
to modify if this doesn't work or if things change.

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

Estimated hours taken: 8

Fix a bug in mdb (for non-conservative-gc grades) where manual memory
allocations in the tracer and memory allocations in the Mercury code
that the tracer calls were overwriting each other, due to confusion
about whether the heap pointer was in its real reg or in its fake_reg
slot.

trace/README:
	Document some conventions for dealing with registers,

trace/mercury_trace_util.h:
	Define macros to allow easy adherence to those conventions.
	
trace/mercury_trace_external.c:
trace/mercury_trace_help.c:
trace/mercury_trace_internal.c:
	Use the new macros to enforce adherence to the conventions.

Index: trace/README
===================================================================
RCS file: README
diff -N README
--- /dev/null	Fri Nov  6 21:21:31 1998
+++ README	Fri Nov  6 21:18:06 1998
@@ -0,0 +1,30 @@
+This directory holds the trace subsystem,
+i.e. the part of the Mercury debugger that is written in C code.
+
+
+Notes on interfacing with other subsystems
+------------------------------------------
+
+If tracing is enabled, the compiler includes calls to MR_trace() in the
+generated C code.  The trace subsystem in this directory is therefore
+called directly from Mercury code, via MR_trace() in
+runtime/mercury_trace_base.c.
+
+For all code in this directory, the usual convention is that the
+normal Mercury registers are in their normal locations, not in the
+fake_reg copies, while the transient (register window) registers,
+if any, are in the fake_reg copies.
+Any code which uses macros such as incr_hp(), list_cons(),
+make_aligned_string(), etc. that modify the heap pointer must call
+restore_transient_hp() beforehand and must call save_transient_hp()
+afterwards.  The simplest way to do this is to use the macro
+MR_USE_HP() in trace/mercury_trace_util.h.
+
+The tracer may invoke Mercury code defined in the browser or library
+directories if that code is exported to C using `pragma export'.
+But any calls from functions here to code defined in Mercury
+and exported using `pragma export', i.e. functions starting with `ML_'
+prefixes, must be preceded by a call to save_registers() and
+followed by a call to restore_registers().
+The simplest way to do this is to use the macro
+MR_CALL_MERCURY() in trace/mercury_trace_util.h.
Index: trace/mercury_trace_external.c
===================================================================
RCS file: /home/mercury1/repository/mercury/trace/mercury_trace_external.c,v
retrieving revision 1.3
diff -u -r1.3 mercury_trace_external.c
--- mercury_trace_external.c	1998/10/28 05:24:19	1.3
+++ mercury_trace_external.c	1998/11/06 10:21:18
@@ -26,6 +26,7 @@
 
 #include "mercury_trace.h"
 #include "mercury_trace_external.h"
+#include "mercury_trace_util.h"
 #include "mercury_layout_util.h"
 #include <stdio.h>
 #include <errno.h>
@@ -432,7 +433,8 @@
 	** compiler-generated procedures.
 	*/
 
-	MR_DI_output_current_slots(
+    MR_TRACE_CALL_MERCURY(
+	ML_DI_output_current_slots(
 		MR_trace_event_number,
 		seqno,
 		depth,
@@ -444,32 +446,39 @@
 		layout->MR_sll_entry->MR_sle_detism,
 		(String) (Word) path,
 		(Word) &MR_debugger_socket_out);
+    );
 }
 
 static void
 MR_output_current_vars(Word var_list, Word string_list)
 {
-	MR_DI_output_current_vars(
+    MR_TRACE_CALL_MERCURY(
+	ML_DI_output_current_vars(
 		var_list,
 		string_list,
 		(Word) &MR_debugger_socket_out);
+    );
 }
 
 static void
 MR_output_current_nth_var(Word var)
 {
-	MR_DI_output_current_nth_var(
+    MR_TRACE_CALL_MERCURY(
+	ML_DI_output_current_nth_var(
 		var,
 		(Word) &MR_debugger_socket_out);
+    ):
 }
 
 static void
 MR_output_current_live_var_names(Word var_names_list, Word type_list)
 {
-	MR_DI_output_current_live_var_names(
+    MR_TRACE_CALL_MERCURY(
+	ML_DI_output_current_live_var_names(
 		var_names_list,
 		type_list,
 		(Word) &MR_debugger_socket_out);
+    );
 }
 
 static void
@@ -478,10 +487,13 @@
 			Integer *debugger_request_type_ptr)
 {		
 	fflush(MR_debugger_socket_in.file);
-	MR_DI_read_request_from_socket(
+
+    MR_TRACE_CALL_MERCURY(
+	ML_DI_read_request_from_socket(
 		(Word) &MR_debugger_socket_in, 
 		debugger_request_ptr, 
 		debugger_request_type_ptr);
+    );
 }
  
 
@@ -503,7 +515,9 @@
 
 	/* XXX get live vars from registers */
 	Word arguments = /* XXX FIXME!!! */ 0;
-	result = MR_DI_found_match(
+
+    MR_TRACE_CALL_MERCURY(
+	result = ML_DI_found_match(
 		MR_trace_event_number,
 		seqno,
 		depth,
@@ -516,6 +530,8 @@
 		arguments,
 		(String) (Word) path,
 		search_data);
+    );
+
 	return result;
 }
 
@@ -531,6 +547,7 @@
 /*
 ** This function returns the list of the internal names of currently live
 ** variables.
+** The memory needed will be allocated on the Mercury heap.
 */
 
 static Word
@@ -546,16 +563,13 @@
 	var_count = layout->MR_sll_var_count;
 	vars = &layout->MR_sll_var_info;
 
-	restore_transient_registers();
+    MR_TRACE_USE_HP(
 	var_names_list = list_empty();
-	save_transient_registers();
 	for (i = var_count - 1; i >= 0; i--) {
-
 		name = MR_name_if_present(vars, i);
-		restore_transient_registers();
 		var_names_list = list_cons(name, var_names_list);
-		save_transient_registers();
 	}
+    );
 
 	return var_names_list;
 }
@@ -563,6 +577,7 @@
 
 /*
 ** This function returns the list of types of currently live variables.
+** The memory needed will be allocated on the Mercury heap.
 */
 
 static Word
@@ -581,9 +596,9 @@
 	var_count = layout->MR_sll_var_count;
 	vars = &layout->MR_sll_var_info;
 
-	restore_transient_registers();
-	type_list = list_empty();
-	save_transient_registers();
+        MR_TRACE_USE_HP(
+		type_list = list_empty();
+        );
 	for (i = var_count - 1; i >= 0; i--) {
 
 		name = MR_name_if_present(vars, i);
@@ -594,10 +609,12 @@
 			continue;
 		}
 
-		restore_transient_registers();
-		type_info_string = MR_type_name(type_info);
-		type_list = list_cons(type_info_string, type_list);
-		save_transient_registers();
+		MR_TRACE_CALL_MERCURY(
+			type_info_string = MR_type_name(type_info);
+		);
+	        MR_TRACE_USE_HP(
+			type_list = list_cons(type_info_string, type_list);
+	        );
 	}
 
 	return type_list;
@@ -605,7 +622,8 @@
 
 
 /*
-** This function returns the requested live variable.
+** This function returns the requested live variable, as a univ.
+** Any memory needed will be allocated on the Mercury heap.
 */
 
 static Word
@@ -629,9 +647,9 @@
 	name = MR_name_if_present(vars, var_number);
 	var = &vars->MR_slvs_pairs[var_number];
 
-	restore_transient_registers();
-	incr_hp(univ, 2);
-
+	MR_TRACE_USE_HP(
+		incr_hp(univ, 2);
+	);
 
 	if (MR_get_type_and_value_filtered(var, saved_regs, name,
 			&type_info, &value))
@@ -646,8 +664,6 @@
 		fatal_error("try to retrieve a non-live variable");
 	}
 
-	save_transient_registers();
-
 	return univ;
 }
 
@@ -660,6 +676,10 @@
 static int
 MR_get_var_number(Word debugger_request)
 {
-	return MR_DI_get_var_number(debugger_request);
+	int num;
+	MR_TRACE_CALL_MERCURY(
+		num = ML_DI_get_var_number(debugger_request);
+	);
+	return num;
 }
 #endif /* MR_USE_EXTERNAL_DEBUGGER */
Index: trace/mercury_trace_help.c
===================================================================
RCS file: /home/mercury1/repository/mercury/trace/mercury_trace_help.c,v
retrieving revision 1.4
diff -u -r1.4 mercury_trace_help.c
--- mercury_trace_help.c	1998/11/06 07:25:36	1.4
+++ mercury_trace_help.c	1998/11/06 10:22:50
@@ -27,6 +27,7 @@
 #include "mercury_layout_util.h"
 #include "mercury_array_macros.h"
 #include "mercury_deep_copy.h"
+#include "mercury_trace_util.h"
 #include "std_util.h"
 #include "help.h"
 #include "io.h"
@@ -46,9 +47,9 @@
 	Word	path;
 
 	MR_trace_help_ensure_init();
-	restore_transient_registers();
-	path = list_empty();
-	save_transient_registers();
+	MR_TRACE_USE_HP(
+		path = list_empty();
+	);
 	return MR_trace_help_add_node(path, category, slot, text);
 }
 
@@ -58,13 +59,16 @@
 {
 	Word	path;
 	char	*category_on_heap;
+	const char *result;
 
 	MR_trace_help_ensure_init();
-	restore_transient_registers();
-	make_aligned_string_copy(category_on_heap, category);
-	path = list_empty();
-	path = list_cons(category_on_heap, path);
-	save_transient_registers();
+
+	MR_TRACE_USE_HP(
+		make_aligned_string_copy(category_on_heap, category);
+		path = list_empty();
+		path = list_cons(category_on_heap, path);
+	);
+
 	return MR_trace_help_add_node(path, item, slot, text);
 }
 
@@ -75,30 +79,34 @@
 	char	*msg;
 	char	*name_on_heap;
 	char	*text_on_heap;
+	bool	error;
 
-	restore_transient_registers();
-	make_aligned_string_copy(name_on_heap, name);
-	make_aligned_string_copy(text_on_heap, text);
-	save_transient_registers();
-
-	ML_HELP_add_help_node(MR_trace_help_system, path, slot,
-		name_on_heap, text_on_heap, &result, &MR_trace_help_system);
+	MR_TRACE_USE_HP(
+		make_aligned_string_copy(name_on_heap, name);
+		make_aligned_string_copy(text_on_heap, text);
+	);
+
+	MR_TRACE_CALL_MERCURY(
+		ML_HELP_add_help_node(MR_trace_help_system, path, slot,
+			name_on_heap, text_on_heap, &result,
+			&MR_trace_help_system);
+		error = ML_HELP_result_is_error(result, &msg);
+	);
 
 	MR_trace_help_system = MR_make_permanent(MR_trace_help_system,
 				(Word *) MR_trace_help_system_type);
 
-	if (ML_HELP_result_is_error(result, &msg)) {
-		return msg;
-	} else {
-		return NULL;
-	}
+	return (error ? msg : NULL);
 }
 
 void
 MR_trace_help(void)
 {
 	MR_trace_help_ensure_init();
-	ML_HELP_help(MR_trace_help_system, MR_trace_help_stdout);
+
+	MR_TRACE_CALL_MERCURY(
+		ML_HELP_help(MR_trace_help_system, MR_trace_help_stdout);
+	);
 }
 
 void
@@ -108,12 +116,14 @@
 
 	MR_trace_help_ensure_init();
 
-	restore_transient_registers();
-	make_aligned_string_copy(word_on_heap, word);
-	save_transient_registers();
-
-	ML_HELP_name(MR_trace_help_system, word_on_heap,
-		MR_trace_help_stdout);
+	MR_TRACE_USE_HP(
+		make_aligned_string_copy(word_on_heap, word);
+	);
+
+	MR_TRACE_CALL_MERCURY(
+		ML_HELP_name(MR_trace_help_system, word_on_heap,
+			MR_trace_help_stdout);
+	);
 }
 
 void
@@ -124,19 +134,24 @@
 	char	*msg;
 	char	*category_on_heap;
 	char	*item_on_heap;
+	bool	error;
 
 	MR_trace_help_ensure_init();
 
-	restore_transient_registers();
-	make_aligned_string_copy(category_on_heap, category);
-	make_aligned_string_copy(item_on_heap, item);
-	path = list_empty();
-	path = list_cons(item_on_heap, path);
-	path = list_cons(category_on_heap, path);
-	save_transient_registers();
+	MR_TRACE_USE_HP(
+		make_aligned_string_copy(category_on_heap, category);
+		make_aligned_string_copy(item_on_heap, item);
+		path = list_empty();
+		path = list_cons(item_on_heap, path);
+		path = list_cons(category_on_heap, path);
+	);
+
+	MR_TRACE_CALL_MERCURY(
+		ML_HELP_path(MR_trace_help_system, path, MR_trace_help_stdout, &result);
+		error = ML_HELP_result_is_error(result, &msg);
+	);
 
-	ML_HELP_path(MR_trace_help_system, path, MR_trace_help_stdout, &result);
-	if (ML_HELP_result_is_error(result, &msg)) {
+	if (error) {
 		printf("internal error in the trace help system: %s\n", msg);
 	}
 }
@@ -149,19 +164,19 @@
 	Word		output_stream_type;
 
 	if (! done) {
-		ML_get_type_info_for_type_info(&typeinfo_type);
-		ML_HELP_help_system_type(&MR_trace_help_system_type);
-		MR_trace_help_system = MR_make_permanent(
+		MR_TRACE_CALL_MERCURY(
+			ML_get_type_info_for_type_info(&typeinfo_type);
+			ML_HELP_help_system_type(&MR_trace_help_system_type);
+			ML_HELP_init(&MR_trace_help_system);
+			ML_io_output_stream_type(&output_stream_type);
+			ML_io_stdout_stream(&MR_trace_help_stdout);
+		);
+
+		MR_trace_help_system_type = MR_make_permanent(
 					MR_trace_help_system_type,
 					(Word *) typeinfo_type);
-
-		ML_HELP_init(&MR_trace_help_system);
-
 		MR_trace_help_system = MR_make_permanent(MR_trace_help_system,
 					(Word *) MR_trace_help_system_type);
-
-		ML_io_output_stream_type(&output_stream_type);
-		ML_io_stdout_stream(&MR_trace_help_stdout);
 		MR_trace_help_stdout = MR_make_permanent(MR_trace_help_stdout,
 					(Word *) output_stream_type);
 
Index: trace/mercury_trace_internal.c
===================================================================
RCS file: /home/mercury1/repository/mercury/trace/mercury_trace_internal.c,v
retrieving revision 1.8
diff -u -r1.8 mercury_trace_internal.c
--- mercury_trace_internal.c	1998/11/05 15:37:53	1.8
+++ mercury_trace_internal.c	1998/11/06 10:23:44
@@ -17,6 +17,7 @@
 #include "mercury_trace_help.h"
 #include "mercury_trace_spy.h"
 #include "mercury_trace_tables.h"
+#include "mercury_trace_util.h"
 #include "mercury_layout_util.h"
 #include "mercury_array_macros.h"
 #include "mercury_getopt.h"
@@ -1715,11 +1716,13 @@
 	** are not of interest to the user.
 	*/
 
-	if (MR_get_type_and_value_base(var, saved_regs,
-			base_sp, base_curfr, type_params, &type_info, &value))
-	{
+	print_value = MR_get_type_and_value_base(var, saved_regs,
+			base_sp, base_curfr, type_params, &type_info, &value);
+	if (print_value) {
 		printf("\t");
-		MR_write_variable(type_info, value);
+		MR_TRACE_CALL_MERCURY(
+			MR_write_variable(type_info, value);
+		);
 	}
 
 	printf("\n");
Index: trace/mercury_trace_util.h
===================================================================
RCS file: mercury_trace_util.h
diff -N mercury_trace_util.h
--- /dev/null	Fri Nov  6 21:21:31 1998
+++ mercury_trace_util.h	Fri Nov  6 21:23:10 1998
@@ -0,0 +1,41 @@
+/*
+** Copyright (C) 1998 The University of Melbourne.
+** This file may only be copied under the terms of the GNU Library General
+** Public License - see the file COPYING.LIB in the Mercury distribution.
+*/
+
+/*
+** mercury_trace_util.h defines macros for dealing with registers.
+**
+** These macros assume, and enforce, the conventions described
+** in trace/README.
+*/
+
+#ifndef MERCURY_TRACE_UTIL_H
+#define MERCURY_TRACE_UTIL_H
+
+/*
+** When using the heap pointer, we need to restore it, in case it is
+** transient.
+*/
+#define MR_TRACE_USE_HP(STATEMENTS) do {				\
+		restore_transient_registers();				\
+		STATEMENTS;						\
+		save_transient_registers();				\
+	} while (0)
+
+/*
+** When calling Mercury code defined using `pragma export', we need
+** to call save_registers() and restore_registers() around it.
+** That in turn needs to be preceded/followed by
+** restore/save_transient_registers() if it is in a C function.
+*/
+#define MR_TRACE_CALL_MERCURY(STATEMENTS) do {				\
+		restore_transient_registers();				\
+		save_registers();					\
+		STATEMENTS;						\
+		restore_registers();					\
+		save_transient_registers();				\
+	} while (0)
+
+#endif /* MERCURY_TRACE_UTIL_H */

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh>  |  of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3        |     -- the last words of T. S. Garp.



More information about the developers mailing list