for review: breaking up mercury_trace.c (round two)

Zoltan Somogyi zs at cs.mu.OZ.AU
Fri Apr 17 15:20:30 AEST 1998


This is a diff relative to my previous message, to address the concerns
raised in reviews.

runtime/mercury_trace.[ch]:
runtime/mercury_trace_internal.[ch]:
runtime/mercury_trace_external.[ch]:
runtime/mercury_trace_util.[ch]:
	Divide the old mercury_trace.c into several components, with one
	module for the internal debugger, one for the interface to the
	external debugger, one for utilities needed by both. Mercury_trace.c
	now has only the top-level stuff that steers between the two
	debuggers.

runtime/Mmakefile:
	Update for the new source and header files.

Zoltan.

--- OLD	Thu Apr  9 18:56:29 1998
+++ NEW	Fri Apr 17 17:16:23 1998
@@ -16,10 +16,9 @@
 ** Utility functions usable by both the internal and external debuggers
 ** are in mercury_trace_util.c.
 **
-** This source file has two header files. One, mercury_trace.h, contains
-** the interface we export to user level programs. The other, which is
-** in mercury_trace_cmd.h contains the interface we export to the
-** internal debugger to allow it to control how events are handled.
+** The C functions in all these modules which use any of the Mercury registers
+** expect the transient registers to be in fake_reg, and the others to be in
+** their normal homes.
 **
 ** Main author: Zoltan Somogyi.
 */
@@ -26,7 +25,6 @@
 
 #include "mercury_imp.h"
 #include "mercury_trace.h"
-#include "mercury_trace_cmd.h"
 #include "mercury_trace_util.h"
 #include "mercury_trace_internal.h"
 #include "mercury_trace_external.h"
@@ -86,12 +84,9 @@
 
 int	MR_trace_event_number = 0;
 
-MR_trace_cmd_type	MR_trace_cmd = MR_CMD_GOTO;
-int			MR_trace_stop_seqno = 0;
-int			MR_trace_stop_event = 0;
-bool			MR_trace_print_intermediate = FALSE;
+static	MR_trace_cmd_info	MR_trace_ctrl = { MR_CMD_GOTO, 0, 0, FALSE };
 
-static void	MR_trace_event(MR_trace_interact interact,
+static	void	MR_trace_event(MR_trace_cmd_info *cmd,
 			const MR_Stack_Layout_Label *layout,
 			MR_trace_port port, int seqno, int depth,
 			const char *path, int max_r_num);
@@ -124,15 +119,15 @@
 	int seqno, int depth, const char *path, int max_r_num)
 {
 	MR_trace_event_number++;
-	switch (MR_trace_cmd) {
+	switch (MR_trace_ctrl.MR_trace_cmd) {
 		case MR_CMD_FINISH:
-			if (MR_trace_stop_seqno == seqno
+			if (MR_trace_ctrl.MR_trace_stop_seqno == seqno
 			&& MR_port_is_final(port)) {
-				MR_trace_event(MR_INTERACT, layout,
+				MR_trace_event(&MR_trace_ctrl, layout,
 					port, seqno, depth, path, max_r_num);
 
-			} else if (MR_trace_print_intermediate) {
-				MR_trace_event(MR_NO_INTERACT, layout,
+			} else if (MR_trace_ctrl.MR_trace_print_intermediate) {
+				MR_trace_event(NULL, layout,
 					port, seqno, depth, path, max_r_num);
 			}
 
@@ -139,12 +134,13 @@
 			break;
 
 		case MR_CMD_GOTO:
-			if (MR_trace_event_number >= MR_trace_stop_event
+			if (MR_trace_event_number >=
+				MR_trace_ctrl.MR_trace_stop_event
 			|| MR_event_matches_spy_point(layout)) {
-				MR_trace_event(MR_INTERACT, layout,
+				MR_trace_event(&MR_trace_ctrl, layout,
 					port, seqno, depth, path, max_r_num);
-			} else if (MR_trace_print_intermediate) {
-				MR_trace_event(MR_NO_INTERACT, layout,
+			} else if (MR_trace_ctrl.MR_trace_print_intermediate) {
+				MR_trace_event(NULL, layout,
 					port, seqno, depth, path, max_r_num);
 			}
 
@@ -152,10 +148,10 @@
 
 		case MR_CMD_RESUME_FORWARD:
 			if (MR_port_is_final(port)) {
-				MR_trace_event(MR_INTERACT, layout,
+				MR_trace_event(&MR_trace_ctrl, layout,
 					port, seqno, depth, path, max_r_num);
-			} else if (MR_trace_print_intermediate) {
-				MR_trace_event(MR_NO_INTERACT, layout,
+			} else if (MR_trace_ctrl.MR_trace_print_intermediate) {
+				MR_trace_event(NULL, layout,
 					port, seqno, depth, path, max_r_num);
 			}
 
@@ -163,10 +159,10 @@
 
 		case MR_CMD_TO_END:
 			if (MR_event_matches_spy_point(layout)) {
-				MR_trace_event(MR_INTERACT, layout,
+				MR_trace_event(&MR_trace_ctrl, layout,
 					port, seqno, depth, path, max_r_num);
-			} else if (MR_trace_print_intermediate) {
-				MR_trace_event(MR_NO_INTERACT, layout,
+			} else if (MR_trace_ctrl.MR_trace_print_intermediate) {
+				MR_trace_event(NULL, layout,
 					port, seqno, depth, path, max_r_num);
 			}
 
@@ -179,7 +175,7 @@
 }
 
 static void
-MR_trace_event(MR_trace_interact interact,
+MR_trace_event(MR_trace_cmd_info *cmd,
 	const MR_Stack_Layout_Label *layout, MR_trace_port port,
 	int seqno, int depth, const char *path, int max_r_num)
 {
@@ -194,10 +190,9 @@
 	MR_copy_regs_to_saved_regs(max_mr_num);
 #ifdef MR_USE_EXTERNAL_DEBUGGER
 	if (MR_trace_debugger == MR_TRACE_EXTERNAL) {
-		MR_debugger_step(interact, layout, port, seqno, depth, path);
+		MR_trace_event_external(layout, port, seqno, depth, path);
 	} else {
-		MR_trace_display_user(interact, layout, port, seqno, depth,
-			path);
+		MR_trace_event_internal(cmd, layout, port, seqno, depth, path);
 	}
 #else
 	/*
@@ -205,7 +200,7 @@
 	** This is enforced by mercury_wrapper.c.
 	*/
 
-	MR_trace_display_user(interact, layout, port, seqno, depth, path);
+	MR_trace_event_internal(cmd, layout, port, seqno, depth, path);
 #endif
 	MR_copy_saved_regs_to_regs(max_mr_num);
 }
@@ -250,9 +245,14 @@
 */
 
 /*
-** mercury_trace.h - defines the interface between
-** the tracing subsystem and compiled code.
+** mercury_trace.h has two functions.
 **
+** (a)	It defines the interface between the tracing subsystem of the runtime
+**	and compiled code.
+**
+** (b)	It defines the interface by which the internal and external debuggers
+**	can control how the tracing subsystem treats events.
+**
 ** The macros and functions defined in this module are intended to be called
 ** only from code generated by the Mercury compiler, and from hand-written
 ** code in the Mercury runtime or the Mercury standard library.
@@ -261,6 +261,8 @@
 #ifndef MERCURY_TRACE_H
 #define MERCURY_TRACE_H
 
+/* The interface between the tracing subsystem and compiled code. */
+
 #define	MR_trace_incr_seq()	(++MR_trace_call_seqno)
 #define	MR_trace_incr_depth()	(++MR_trace_call_depth)
 #define	MR_trace_reset_depth(d)	do { MR_trace_call_depth = (d); } while (0)
@@ -321,25 +323,11 @@
 
 extern	int		MR_trace_event_number;
 
-#endif /* MERCURY_TRACE_H */
-::::::::::::::
-mercury_trace_cmd.h
-::::::::::::::
-/*
-** 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.
-*/
-
-/*
-** This file defines the commands meaningful to the trace event handler
-** and declares the variables whose values give the parameters of those
-** commands.
-*/
+/* The interface between the debuggers and the tracing subsystem. */
 
 /*
-** MR_trace_cmd says what mode the tracer is in, i.e. what the last
-** trace command was.
+** MR_trace_cmd says what mode the tracer is in, i.e. how events should be
+** treated.
 **
 ** If MR_trace_cmd == MR_CMD_GOTO, the event handler will stop at the next
 ** event whose event number is equal to or greater than MR_trace_stop_event.
@@ -365,22 +353,16 @@
 	MR_CMD_TO_END		/* do not stop until the end of execution  */
 } MR_trace_cmd_type;
 
-extern	MR_trace_cmd_type	MR_trace_cmd;
-extern	int			MR_trace_stop_seqno;
-extern	int			MR_trace_stop_event;
-extern	bool			MR_trace_print_intermediate;
+typedef struct {
+	MR_trace_cmd_type	MR_trace_cmd;	
+	int			MR_trace_stop_seqno;
+	int			MR_trace_stop_event;
+	bool			MR_trace_print_intermediate;
+} MR_trace_cmd_info;
 
 #define	MR_port_is_final(port)	(port == MR_PORT_EXIT || port == MR_PORT_FAIL)
 
-/*
-** When displaying an event to the user, is interaction allowed?
-** I.e. are we stopping at this event?
-*/
-
-typedef	enum {
-	MR_INTERACT,
-	MR_NO_INTERACT
-} MR_trace_interact;
+#endif /* MERCURY_TRACE_H */
 ::::::::::::::
 mercury_trace_external.c
 ::::::::::::::
@@ -398,11 +380,14 @@
 ** "Opium: An extendable trace analyser for Prolog" by Mireille Ducasse,
 ** available from http://www.irisa.fr/lande/ducasse.
 **
+** The code for using an external debugger is conditionalized
+** on MR_USE_EXTERNAL_DEBUGGER (which by default is not enabled)
+** because it uses sockets, which are not portable.
+** Ideally we ought to use autoconf for that...
+**
 ** Main authors: Erwan Jahier and Fergus Henderson.
 */
 
-#ifdef MR_USE_EXTERNAL_DEBUGGER
-
 #include "mercury_imp.h"
 #include "mercury_trace.h"
 #include "mercury_trace_external.h"
@@ -417,6 +402,8 @@
 #include <netinet/in.h>
 #include <netdb.h>
 
+#ifdef MR_USE_EXTERNAL_DEBUGGER
+
 /*
 ** This type must match the definition of classify_request in
 ** library/debugger_interface.m.
@@ -431,15 +418,6 @@
 	MR_REQUEST_ERROR = 5         /* something went wrong                */
 } MR_debugger_request_type;
 
-/*
-We could use
-	if (MR_use_debugger) { ...
-instead of #if; this would be better in the long run.
-This would require changing mercury_wrapper.c to
-check for an additional flag in the MERCURY_OPTIONS
-environment variable and set MR_use_debugger accordingly.
-*/
-
 static MercuryFile MR_debugger_socket_in;
 static MercuryFile MR_debugger_socket_out;
 
@@ -692,8 +670,7 @@
 }
 
 void
-MR_debugger_step(MR_trace_interact interact,
-	const MR_Stack_Layout_Label *layout,
+MR_trace_event_external(const MR_Stack_Layout_Label *layout,
 	MR_trace_port port, int seqno, int depth, const char *path)
 {
 	static bool searching = FALSE;
@@ -836,16 +813,18 @@
 */
 
 #ifndef	MERCURY_TRACE_EXTERNAL_H
+#define	MERCURY_TRACE_EXTERNAL_H
+
 #ifdef	MR_USE_EXTERNAL_DEBUGGER
 
 extern	void	MR_trace_init_external(void);
 extern	void	MR_trace_end_external(void);
-extern	void	MR_debugger_step(MR_trace_interact interact,
-			const MR_Stack_Layout_Label *layout,
+extern	void	MR_trace_event_external(const MR_Stack_Layout_Label *layout,
 			MR_trace_port port, int seqno, int depth,
 			const char *path);
 
 #endif	/* MR_USE_EXTERNAL_DEBUGGER */
+
 #endif	/* MERCURY_TRACE_EXTERNAL_H */
 ::::::::::::::
 mercury_trace_internal.c
@@ -864,7 +843,6 @@
 
 #include "mercury_imp.h"
 #include "mercury_trace.h"
-#include "mercury_trace_cmd.h"
 #include "mercury_trace_internal.h"
 #include "mercury_trace_util.h"
 #include <stdio.h>
@@ -898,7 +876,7 @@
 static void	MR_trace_help(void);
 
 void
-MR_trace_display_user(MR_trace_interact interact,
+MR_trace_event_internal(MR_trace_cmd_info *cmd,
 	const MR_Stack_Layout_Label *layout,
 	MR_trace_port port, int seqno, int depth, const char *path)
 {
@@ -947,7 +925,7 @@
 			break;
 
 		default:
-			fatal_error("MR_trace_display_user called "
+			fatal_error("MR_trace_event_internal called "
 					"with bad port");
 	}
 
@@ -1002,12 +980,12 @@
 		(long) layout->MR_sll_entry->MR_sle_mode,
 		path);
 
-	while (interact == MR_INTERACT) {
+	for (;;) {
 		printf("mtrace> ");
 
 		count = 1;
 		count_given = FALSE;
-		MR_trace_print_intermediate = FALSE;
+		cmd->MR_trace_print_intermediate = FALSE;
 
 		c = MR_trace_skip_spaces(' ');
 		if (isdigit(c)) {
@@ -1024,19 +1002,19 @@
 
 		switch (c) {
 			case 'S':
-				MR_trace_print_intermediate = TRUE;
+				cmd->MR_trace_print_intermediate = TRUE;
 				/* fall through */
 
 			case 's':
 			case '\n':
-				MR_trace_cmd = MR_CMD_GOTO;
-				MR_trace_stop_event =
+				cmd->MR_trace_cmd = MR_CMD_GOTO;
+				cmd->MR_trace_stop_event =
 					MR_trace_event_number + count;
 				MR_trace_discard_to_eol(c);
 				break;
 
 			case 'G':
-				MR_trace_print_intermediate = TRUE;
+				cmd->MR_trace_print_intermediate = TRUE;
 				/* fall through */
 
 			case 'g':
@@ -1046,13 +1024,13 @@
 					continue;
 				}
 
-				MR_trace_cmd = MR_CMD_GOTO;
-				MR_trace_stop_event = count;
+				cmd->MR_trace_cmd = MR_CMD_GOTO;
+				cmd->MR_trace_stop_event = count;
 				MR_trace_discard_to_eol(c);
 				break;
 
 			case 'F':
-				MR_trace_print_intermediate = TRUE;
+				cmd->MR_trace_print_intermediate = TRUE;
 				/* fall through */
 
 			case 'f':
@@ -1062,8 +1040,8 @@
 						"already final\n");
 					continue;
 				} else {
-					MR_trace_cmd = MR_CMD_FINISH;
-					MR_trace_stop_seqno = seqno;
+					cmd->MR_trace_cmd = MR_CMD_FINISH;
+					cmd->MR_trace_stop_seqno = seqno;
 				}
 
 				MR_trace_discard_to_eol(c);
@@ -1070,7 +1048,7 @@
 				break;
 
 			case 'C':
-				MR_trace_print_intermediate = TRUE;
+				cmd->MR_trace_print_intermediate = TRUE;
 				/* fall through */
 
 			case 'c':
@@ -1077,7 +1055,7 @@
 				if (count_given)
 					printf("mtrace: count ignored\n");
 
-				MR_trace_cmd = MR_CMD_TO_END;
+				cmd->MR_trace_cmd = MR_CMD_TO_END;
 				MR_trace_discard_to_eol(c);
 				break;
 
@@ -1096,7 +1074,7 @@
 				if (count_given)
 					printf("mtrace: count ignored\n");
 
-				MR_trace_cmd = MR_CMD_RESUME_FORWARD;
+				cmd->MR_trace_cmd = MR_CMD_RESUME_FORWARD;
 				MR_trace_discard_to_eol(c);
 				break;
 
@@ -1158,7 +1136,7 @@
 				continue;
 		}
 
-		interact = MR_NO_INTERACT;
+		break;
 	}
 }
 
@@ -1245,21 +1223,18 @@
 		** Probably that was due to a bug which has since been
 		** fixed, so we should change the code below back again...
 		**
-		** call_engine expects the transient registers to be
+		** call_engine() expects the transient registers to be
 		** in fake_reg, others in their normal homes.
-		** The code below works by placing r1, r2 and all other
-		** transient registers both in their normal homes and
-		** and in fake_reg as well.
+		** That is the case on entry to this function.
+		** But r1 or r2 may be transient, so we need to save/restore
+		** transient regs around the assignments to them.
 		*/
 
 		MR_trace_enabled = FALSE;
-		for (i = 0; i < MR_MAX_SPECIAL_REG_MR; i++) {
-			fake_reg[i] = MR_saved_regs[i];
-		}
-		restore_registers();
+		restore_transient_registers();
 		r1 = type_info;
 		r2 = value;
-		save_transient_registers(); /* r1 or r2 may be transient */
+		save_transient_registers();
 		call_engine(MR_library_trace_browser);
 		MR_trace_enabled = TRUE;
 	}
@@ -1449,8 +1424,9 @@
 */
 
 #ifndef	MERCURY_TRACE_INTERNAL_H
+#define	MERCURY_TRACE_INTERNAL_H
 
-extern	void	MR_trace_display_user(MR_trace_interact interact,
+extern	void	MR_trace_event_internal(MR_trace_cmd_info *cmd,
 			const MR_Stack_Layout_Label *layout,
 			MR_trace_port port, int seqno, int depth,
 			const char *path);
@@ -1551,13 +1527,13 @@
 	MR_Live_Type			live_type;
 	Word				type_info;
 
-	restore_transient_registers();
-
 	var_count = layout->MR_sll_var_count;
 	vars = &layout->MR_sll_var_info;
 
 	/* build up the live variable list, starting from the end */
+	restore_transient_registers();
 	univ_list = list_empty();
+	save_transient_registers();
 	for (i = var_count - 1; i >= 0; i--) {
 		/*
 		** Look up the name, the type and value
@@ -1588,16 +1564,24 @@
 			continue;
 		}
 
-		/* create a term of type `univ' to hold the type & value */
+		/*
+		** Create a term of type `univ' to hold the type & value,
+		** and cons it onto the list.
+		** Note that the calls to save/restore transient registers
+		** can't be hoisted out of the loop, because
+		** MR_trace_get_type_and_value() calls ML_create_type_info()
+		** which may allocate memory using incr_saved_hp.
+		*/
+
+		restore_transient_registers();
 		incr_hp(univ, 2);
 		field(mktag(0), univ, UNIV_OFFSET_FOR_TYPEINFO) = type_info;
 		field(mktag(0), univ, UNIV_OFFSET_FOR_DATA) = value;
 		
 		univ_list = list_cons(univ, univ_list);
+		save_transient_registers();
 	}
 
-	save_transient_registers();
-
 	return univ_list;
 }
 
@@ -1711,6 +1695,7 @@
 */
 
 #ifndef	MERCURY_TRACE_UTIL_H
+#define	MERCURY_TRACE_UTIL_H
 
 extern	Word    MR_saved_regs[MAX_FAKE_REG];
 extern	void	MR_copy_regs_to_saved_regs(int max_mr_num);



More information about the developers mailing list