for review: software errors from C code

[Tyson Richard DOWD]

Andrew Bromage <bromage at cs.mu.OZ.AU> writes:

>G'day all.

>Tyson, could you review this please?

>Cheers,
>Andrew Bromage


>Estimated hours taken: 1

>Fix to allow software errors which happen in C library code to produce
>a stack dump.

>Note: Not every kind of fatal error in the C library is a "software
>error".  Only the ones which users have a hope of fixing really count
>(e.g. array bounds check errors).

Well, this isn't necessarily true - we still have use for stack
dumps as developers, but since it's some work to change this,
it's ok this way.

>Also hidden away in here are two small changes:

>	- a fix for a problem with the user-declared array_module
>	  which conflicts with the compiler-declared array_module.
>	  The fix is to call the user-declared one array_moduleX.

Use a better name.  array_module_internal or array_module_user
would have some meaning.

>Index: library/io.m
>===================================================================
>RCS file: /home/staff/zs/imp/mercury/library/io.m,v
>retrieving revision 1.153
>diff -u -r1.153 io.m
>--- io.m	1998/03/12 19:45:38	1.153
>+++ io.m	1998/03/26 05:19:49
>@@ -2187,7 +2187,7 @@
> 	}
> 	/* XXX should work even if ungetc() fails */
> 	if (ungetc(Character, mf->file) == EOF) {
>-		fatal_error(""io__putback_char: ungetc failed"");
>+		software_error(""io__putback_char: ungetc failed"");
> 	}
> 	update_io(IO0, IO);
> }").

This doesn't fit into your defintion of "user-fixable" errors above.
This is in fact an implementation weakness.  Some of the other ones are
similar.

>Index: library/require.m
>===================================================================
>RCS file: /home/staff/zs/imp/mercury/library/require.m,v
>retrieving revision 1.18
>diff -u -r1.18 require.m
>--- require.m	1998/03/11 05:57:41	1.18
>+++ require.m	1998/03/26 05:14:24
>@@ -56,10 +56,7 @@
> :- pragma no_inline(error/1). 
> 
> :- pragma c_code(error(Message::in), "
>-	fflush(stdout);
>-	fprintf(stderr, ""Software error: %s\\n"", Message);
>-	MR_dump_stack(MR_succip, MR_sp);
>-	exit(1);
>+	software_error(Message);
> #ifndef USE_GCC_NONLOCAL_GOTOS
> 	return 0;	/* suppress some dumb warnings */
> #endif

save and restore transient registers.  Well, actually, the restore
isn't necessary here.

>+++ mercury_misc.h	1998/03/26 05:10:34
>@@ -62,11 +62,14 @@
> 	#define NO_RETURN
> #endif
> extern	void	fatal_error(const char *msg) NO_RETURN;
>+extern	void	software_error(const char *msg) NO_RETURN;

Hmmm, if defined as no return, I guess it's possible that you don't
need to save and restore registers.  But best to do it anyway.

> /* 
>-** XXX checked_malloc() should be moved to mercury_memory.h or mercury_heap.h
>+** XXX checked_malloc() and checked_realloc() should be moved to
>+**     mercury_memory.h or mercury_heap.h
> */
> #include <stddef.h>	/* for size_t */
> void *checked_malloc(size_t n);
>+void *checked_realloc(void *old, size_t n);

Why not do it now?  It looks like the only way the runtime is going to
be cleaned up is if we do it piece by piece as we make other changes.

Other than the suggestions above, the change looks fine.  It's certainly
a good idea.