[m-dev.] for review: improved low-level debugging messages

[Fergus Henderson]

On 17-Mar-1999, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> 
> Estimated hours taken: 0.5
> 
> runtime/mercury_misc.[ch]:
> 	Improve the facilities for printing pointers into the det and nondet
> 	stacks for diagnostic purposes.
> 
> runtime/mercury_stack_trace.c:
> 	Use the improved facilities in mercury_misc to improve the output.
> 
> trace/mercury_trace_internal.c:
> 	Use the improved facilties in mercury_misc to improve the
> 	formatting of the output of the stack_regs command.
...
> +++ mercury_misc.c	1999/03/16 01:19:16
> @@ -156,7 +156,7 @@
>  incr_hp_debug_msg(Word val, const Word *addr)
>  {
>  #ifdef CONSERVATIVE_GC
> -	printf("allocated %ld words at 0x%p\n", (long) (Integer) val, addr);
> +	printf("allocated %ld words at %p\n", (long) (Integer) val, addr);

Both the old code and the new code are buggy, because there is no guarantee
that (long) is the same size as `void *'.

One fix is to change it to "0x%lu".  It would be a good idea to
at the same time change the cast so that it casts to (unsigned long)
rather than to (long) (Integer).

Another fix would be to change it to "%p", and cast to `(void *)' rather than
`(long) (Integer)'.  However, the format of "%p" is implementation-defined,
and some implementations print out hexadecimal values without prefixing
them with "0x", which can sometimes be confusing or ambiguous.
So I would recommend using the former fix.  On the other hand, the latter
fix is often more convenient.  Take your pick.

> @@ -324,7 +287,7 @@
>  	printf("\t prevfr    "); printnondstack(MR_prevfr_slot(fr));
>  
>  	for (i = 1; &MR_based_framevar(fr,i) > MR_prevfr_slot(fr); i++) {
> -		printf("\t framevar(%d)  %ld 0x%lx\n",
> +		printf("\t framevar(%d)  %ld %lx\n",
>  			i, (long) (Integer) MR_based_framevar(fr,i),
>  			(unsigned long) MR_based_framevar(fr,i));
>  	}

What's the rationale for this change?
The log message says

 	Improve the facilities for printing pointers into the det and nondet
 	stacks for diagnostic purposes.

but why is removing the "0x" here an improvement?

> @@ -396,24 +359,93 @@
>  #endif /* defined(MR_DEBUG_GOTOS) */
>  
>  void 
> +printdetstackptr(const Word *s)
> +{
> +	print_detstackptr(stdout, s);
> +	return;
> +}
> +
> +void 
> +print_detstackptr(FILE *fp, const Word *s)
> +{
> +	fprintf(fp, "det %3ld (%p)",
> +		(long) (Integer) (s - MR_CONTEXT(detstack_zone)->min),
> +		(const void *) s);
> +	return;
> +}

These new functions should get an `MR_' prefix.

> +++ mercury_misc.h	1999/03/16 01:19:16
> +extern	void	printdetstack(const Word *s);
> +extern	void	printdetstackptr(const Word *s);
> +extern	void	print_detstackptr(FILE *fp, const Word *s);
> +extern	void	printnondstack(const Word *s);
> +extern	void	printnondstackptr(const Word *s);
> +extern	void	print_nondstackptr(FILE *fp, const Word *s);

Likewise.

-- 
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.