[m-dev.] for review: a big step towards the trace-based debugger (part 3 of 3)

[Tyson Dowd]

On 20-Mar-1998, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> +** MR_trace_event_number is a simple counter of events. This is used in
> +** two places: here, for display to the user and for skipping a given number
> +** of events, and when printing an abort message, so that the programmer
> +** can zero in on the source of the problem more quickly.
>  */

I think this is a very cool idea.

>  
> -static	int	MR_trace_event_number = 0;
> +int	MR_trace_event_number = 0;
>  
>  /*
>  ** MR_trace_cmd and MR_trace_seqno are globals variables that we use
> @@ -61,17 +85,16 @@
>  */
>  
>  typedef enum {
> -	MR_CMD_CONT,	/* c: continue to end, not printing the trace	  */
> -	MR_CMD_DUMP,	/* d: continue to end, printing the trace	  */
> -	MR_CMD_NEXT,	/* n: go to the next trace event		  */
> -	MR_CMD_SKIP,	/* s: skip the current call, not printing trace	  */
> -	MR_CMD_JUMP	/* j: jump to end of current call, printing trace */
> +	MR_CMD_GOTO,
> +	MR_CMD_FINISH,
> +	MR_CMD_TO_END
>  } MR_trace_cmd_type;
>  
>  /*
>  ** This type must match the definition of classify_request in
>  ** library/debugger_interface.m.
>  */
> +
>  typedef enum {
>  	MR_REQUEST_HELLO_REPLY = 0,  /* initiate debugging session	    */
>  	MR_REQUEST_FORWARD_MOVE = 1, /* go to the next matching trace event */
> @@ -81,8 +104,10 @@
>  	MR_REQUEST_ERROR = 5         /* something went wrong                */
>  } MR_debugger_request_type;
>  
> -static	MR_trace_cmd_type	MR_trace_cmd = MR_CMD_NEXT;
> -static	int			MR_trace_seqno = 0;
> +static	MR_trace_cmd_type	MR_trace_cmd = MR_CMD_GOTO;
> +static	int			MR_trace_stop_seqno = 0;
> +static	int			MR_trace_stop_event = 0;
> +static	bool			MR_trace_print_intermediate = FALSE;
>  
>  typedef	enum {
>  	MR_INTERACT,
> @@ -90,25 +115,28 @@
>  } MR_trace_interact;
>  
>  static void	MR_trace_event(MR_trace_interact interact,
> -			const MR_stack_layout_entry *layout,
> +			const MR_Stack_Layout_Label *layout,
>  			MR_trace_port port, int seqno, int depth,
> -			const char *path);
> +			const char *path, int max_r_num);
> +static void	MR_copy_saved_regs_to_regs(int max_mr_num);
> +static void	MR_copy_regs_to_saved_regs(int max_mr_num);
>  static void	MR_trace_display_user(MR_trace_interact interact,
> -			const MR_stack_layout_entry *layout,
> +			const MR_Stack_Layout_Label *layout,
>  			MR_trace_port port, int seqno, int depth,
>  			const char *path);
>  static void	MR_trace_browse(int var_count,
> -			const MR_stack_layout_vars *var_info);
> -static void	MR_trace_browse_var(char *name,
> -			const MR_stack_layout_var *var);
> -static int	MR_trace_get_cmd(void);
> +			const MR_Stack_Layout_Vars *var_info);
> +static void	MR_trace_browse_var(const char *name,
> +			const MR_Stack_Layout_Var *var, Word *type_params);
> +static int	MR_trace_skip_spaces(int c);
> +static void	MR_trace_discard_to_eol(int c);
>  static void	MR_trace_help(void);
>  
>  static Word	MR_trace_make_var_list(MR_trace_port port,
> -			const MR_stack_layout_entry *layout);
> +			const MR_Stack_Layout_Label *layout);
>  static Word	MR_trace_lookup_live_lval(MR_Live_Lval locn, bool *succeeded);
> -static bool	MR_trace_get_type_and_value(const MR_stack_layout_var *var,
> -			Word *type_info, Word *value);
> +static bool	MR_trace_get_type_and_value(const MR_Stack_Layout_Var *var,
> +			Word *type_params, Word *type_info, Word *value);
>  
>  /*
>  We could use
> @@ -118,7 +146,7 @@
>  check for an additional flag in the MERCURY_OPTIONS
>  environment variable and set MR_use_debugger accordingly.
>  */
> -#ifdef MR_USE_DEBUGGER
> +#ifdef MR_USE_EXTERNAL_DEBUGGER
>  

The comment at the top of the file about the trace_type
(internal/external) should probably mention the existance of the #define
for MR_USE_EXTERNAL_DEBUGGER being needed if external debugging is
to actually work. 

> +	** If any code invoked by MR_trace is itself traced,
> +	** MR_saved_regs will be overwritten, leading to a crash later on.
> +	** This is one reason (but not the only one) why we turn off
> +	** tracing when we call back Mercury code from this file.
> +	*/

What are the other reasons?  They aren't specified up with the
documentation near MR_trace_enabled (or whatever it's called).

Some of the documentation in this file is a bit too distributed.
(I'm not sure it can be fixed easily though).

> New File: scripts/mmd
> ===================================================================
> #!/bin/sh
> case $# in
> 	0)	echo "Usage: mmd <executable> [<arg> ...]"
> 		exit 1;;
> esac
> 
> MERCURY_OPTIONS="$MERCURY_OPTIONS -Di"
> export MERCURY_OPTIONS
> exec "$@"


I don't particularly care for the name "mmd".  How about "mdebug",
"mercury-debug"?  I know we want to leave namespace for an Opium-style
debugger, but I don't think the way to do it is with "mmd".  I'm
happy for you to commit mmd for the moment, but I'd like to find a
better name for a release.


> 
> New File: tests/debugger/Mmakefile
> ===================================================================
> #-----------------------------------------------------------------------------#
> 
> MMD_WRAPPER = yes
> 

What does this do?  It doesn't appear to be mentioned below.

> New File: tests/debugger/interpreter.exp
> ===================================================================

Due to the (myriad) limitations of USENET news, this doesn't message
didn't get posted to the newsgroup because of the length of these
lines.  Can everyone in future just be aware of this and try to avoid
very long lines? (not that you had much choice here, Zoltan).

Apart from my comments, this is good stuff!  Looking forward to never
seeing Sicstus again (except as a column in some benchmarks, of course).

Diffs relative to this diff for next round, but I don't expect too many
problems.

Please make sure this change doesn't break stack traces before commiting.

-- 
       Tyson Dowd           # So I asked Sarah: what's the serial number on
                            # your computer? She replied:
     trd at cs.mu.oz.au        #          A-C-2-4-0-V-/-5-0-H-Z
http://www.cs.mu.oz.au/~trd #