for review: stack dumps.

[Tyson Dowd]

On 04-Mar-1998, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> On 03-Mar-1998, Tyson Dowd <trd at cs.mu.OZ.AU> wrote:
> > This adds stack traces (well, at the moment, just a "stack dump") to
> > the library and runtime system.
> > 
> > You need to compile with MCFLAGS = --stack-trace and
> > MGNUCFLAGS = -DMR_STACK_TRACE (or EXTRA_MCFLAGS and EXTRA_MGNUCFLAGS)
> 
> `mgnuc' should support a `--stack-trace' option (which would
> just enable -DMR_STACK_TRACE).

This is fine.

> 
> > to obtain an executable capable of displaying a stack dump.  This isn't
> > intended to be the final mechanism -- probably a new grade or debugging
> > grade would be the best solution.
> 
> Do you need to compile the whole thing (library, runtime, etc.)
> with MR_STACK_TRACE defined, or does it work OK if only part
> of the program is compiled with this flag?

For it to be useful, probably yes.  You could just let it print out
information until it cannot go any further, but it wouldn't work very
well in practice.  (You start to do a stack dump, and the algorithm
immediately stops because there is no information available about
error/1, or map__lookup, or whatever, and has to stop.  Not much of a
stack dump).

> 
> Adding a new grade is not that hard.

Yes, but I wonder whether a debugging grade that includes a low-impact
execution trace (turned off with a global flag by default) and stack
traces would be a better way to do it.  Or it would probably be possible
to merge stack traces with profiling too.

> If this change introduces a new incompatible binary format,
> then you must at least change runtime/mercury_grade.h to
> include this flag in the mangled grade identifier MR_GRADE
> (which is used to ensure that the linker will catch attempts
> to link incompatible objects).

Ok, will do this.

> 
> > --- require.m	1998/01/23 12:33:31	1.17
> > +++ require.m	1998/03/03 06:12:41
> > @@ -47,7 +47,10 @@
> > +	dump_stack(MR_succip, MR_sp);
> 
> Should be `MR_dump_stack'.
>            ^^^
> 
> > --- mercury_goto.h	1998/01/06 07:05:59	1.5
> > +++ mercury_goto.h	1998/02/19 05:21:38
> > @@ -11,6 +11,7 @@
> >  
> >  #include "mercury_types.h"	/* for `Code *' */
> >  #include "mercury_debug.h"	/* for debuggoto() */
> > +#include "mercury_stack_trace.h"
> >  #include "mercury_accurate_gc.h"
> 
> Why does mercury_goto.h need to include mercury_stack_trace.h and
> mercury_accurate_gc.h?

Because they conditionally set MR_INSERT_LABELS and other defines.

> 
> > -#if defined(SPEED) && !defined(DEBUG_GOTOS) && !defined(NATIVE_GC)
> > +#if defined(SPEED) && !defined(DEBUG_GOTOS) && !defined(MR_INSERT_LABELS)
> >  #define	make_label(n, a, l)	/* nothing */
> >  #else
> >  #define	make_label(n, a, l)	make_entry(n, a, l)
> >  #endif
> 
> Shouldn't defined(DEBUG_GOTOS) or !defined(SPEED) imply MR_INSERT_LABELS?
> In other words, shouldn't this just be
> 
> 	#if defined(MR_INSERT_LABELS)
> 	...
> 
> ?
> 
> Likewise for most of the other #ifs in this diff (and the one for the
> compiler directory) which test MR_INSERT_LABELS and also something else.

Perhaps.  I was going for a minimalist change.  I guess it all really
depends on your next question.

> What does MR_INSERT_LABELS mean?  This should be documented somewhere
> (runtime/CFLAGSFILE is one possible place; the documentation there
> is rather out-of-date, though).

I'll look into it.

I'll address the other comments and post a new diff.