[m-dev.] for review: introduce the trace and browser directories

Fergus Henderson fjh at cs.mu.OZ.AU
Mon Sep 28 18:40:39 AEST 1998


On 28-Sep-1998, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> 
> After this change, the linking order becomes:
> 
> 	program object files, and the object of the auto-generated init file
>	...

The auto-generated init file object comes before the program object files.
This is important, because with `--split-c-files' the program
object files instead become libraries of program object files,
and the init file object must be first to ensure that the program
object files get pulled in.  So that order should be

	the object of the auto-generated init file
 	program object files (or libraries thereof, with --split-c-files)
	...

> 	trace library (libmer_trace.a)
> 	browser library (libmer_browser.a)
> 	standard library (libmer_std.a)
> 	runtime library (libmer_rt.a)
> 	Boehm collector (libgc.a)
> 
> In case the program does not contain calls to the tracer but the standard
> library does, which can happen if the library is compiled with tracing in a
> non-debug grade, the above structure allows circularities.

The circularities can happen even if the program does contain calls
to the tracer.

> To avoid them,
> MR_trace, which is in the runtime, calls either a dummy tracer in the runtime
> or a real tracer in the trace directory, depending on the value assigned to
> a new global variable by the automatically generated init file.

Hmm, that description makes it sound like the code is

	if (new_global_variable == something) {
		dummy_tracer()
	} else {
		real_tracer();
	}

which would not solve the problem, because the call to real_tracer()
is still a direct call.

The solution to the circularity is to make the call to real_tracer()
an indirect one.  The log message should explain this.

Hence, I suggest you replace the above two sentences in the log message with
the following:

	To avoid circularities, libraries cannot contain direct calls to
	any routines that are defined in libraries (or object files) that
	occur earlier in the above list.  Any such calls must be made into
	indirect calls via function pointers.

	In particular, there was a circularity caused by the library calling
	MR_trace() which invokes the tracer which in turn invokes the
	library.  This circularity was broken by having MR_trace(),
	which is defined in the runtime, call the tracer indirectly via
	a global variable named MR_trace_func_ptr.  This global variable
	is initialized by the auto-generated *_init.c file.

	To avoid linking in the tracer even when it is not being used,
	this global variable is only set to point to MR_trace_real()
	if you're using a debugging grade or if c2init was invoked
	with the `-t' flag.  Otherwise it is set to MR_trace_fake()
	which just prints an error message telling the user to
	rebuild the executable with debugging enabled.

One more point: I think you need to modify scripts/Mmake.rules 
or scripts/Mmake.vars.in to ensure that Mmake passes the GRADEFLAGS
to c2init.

> {runtime,trace}/mercury_trace.[ch]:
> {runtime,trace}/mercury_trace_external.[ch]:
> {runtime,trace}/mercury_trace_internal.[ch]:
> 	Move these files to from the runtime to the trace directory.

s/to from/from/

> +++ Mmake.common.in	1998/09/24 11:55:42
> @@ -164,9 +164,23 @@
> +
> +# The names of the various libraries.
> +# The archives and shared object objects have a "lib" prefix and a ".a" or
> +# ".so" suffix around these names; the initialization files have just a
> +# ".init" suffix.

s/".so"/".so" (or ".dll")/

> +++ browser_library.m	Fri Sep 25 17:00:00 1998
> @@ -0,0 +1,31 @@
> +%---------------------------------------------------------------------------%
> +% 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.
> +%---------------------------------------------------------------------------%
> +
> +:- module browser_library.
> +
> +:- interface.
> +
> +:- pred browser_library__version(string::out) is det.
> +
> +:- implementation.
> +
> +:- import_module help.
> +:- import_module debugger_interface.
> +
> +% Se library/library.m for why we implement this predicate this way.

s/Se/See/

> +:- pragma c_code(browser_library__version(Version::out),
> +		will_not_call_mercury, "
> +	ConstString version_string = 
> +		MR_VERSION "", configured for "" MR_FULLARCH;
> +	/*
> +	** Cast away const needed here, because Mercury declares Version
> +	** with type String rather than ConstString.
> +	*/
> +	Version = (String) (Word) version_string;
> +").

If you want to include MR_VERSION in the version number,
then you need something similar to the following stuff
from library/Mmakefile:

	# Ensure we recompile library__version if VERSION is changed.
	$(os_subdir)library.o \
	$(os_subdir)library.pic_o \
		: $(RUNTIME_DIR)/mercury_conf.h

> Index: runtime/mercury_trace_base.c
...
> +void
> +MR_trace_fake(const MR_Stack_Layout_Label *layout, MR_trace_port port,
> +	Word seqno, Word depth, const char * path, int max_mr_num)
> +{
> +	fatal_error("This executable is not set up for debugging.\n"
> +		"Rebuild the <main>_init.c file, "
> +		"and give the -t flag to c2init.\n");
> +}

The second sentence of that error message is likely to mystify many
users, I think.  Typically users will just use `mmc' or `mmake' and so
they may not be even aware of the `c2init' script.  In addition, the
`<main>_init.c' file may be hidden away in the `Mercury/cs'
subdirectory.  Thus it may be very difficult for users to figure out
what they need to do.

Ideally I think the second sentence should be replaced by
something like "See the <blah> section of the Debugging chapter
of the Mercury User's Guide for details on how to build programs
with debugging enabled.", with the relevant section explaining
in a bit more detail how to do it.

For the moment you should at very least add an XXX.
And perhaps change the message to "For details on how to build programs
with debugging enabled, send email to <mercury at cs.mu.oz.au>." ;-)

> Index: trace/Mmakefile
...
> +# the following header files are created automatically by Makefile.DLLs
> +LIBMER_DLL_H	 = lib$(TRACE_LIB_NAME)_dll.h
> +LIBMER_GLOBALS_H = lib$(TRACE_LIB_NAME)_globals.h

Those variable names (LIBMER*) are misleading, I think.

> Index: util/Mmakefile
...
>  MGNUC   = MERCURY_C_INCL_DIR=$(RUNTIME_DIR) $(SCRIPTS_DIR)/mgnuc
> -CFLAGS	= -I$(RUNTIME_DIR) $(EXTRA_CFLAGS)
> +CFLAGS	= -I$(RUNTIME_DIR) $(EXTRA_CFLAGS) -O0
>  # we need -I ../runtime for "mercury_getopt.h"
> +# the -O0 is to get around a stupid compiler bug on cyclone

I think you should explain in a bit more detail here,
i.e. include the same comments as in the log message,
and also give the version number of gcc.
(Also "compiler bug" is a bit ambiguous -- which compiler, gcc or mercury?)

>  static void 
>  output_main(void)
>  {
> -	printf(mercury_funcs, entry_point, entry_point);
> +	const char *trace_func;
> +
> +	trace_func = need_tracing ? "MR_trace_real" : "MR_trace_fake";

I think some additional parentheses would make the code more readable here:

	trace_func = (need_tracing ? "MR_trace_real" : "MR_trace_fake");

--------------------

When you've addressed those issues, could you please post
a new log message and a relative diff?

Thanks,
	Fergus.

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



More information about the developers mailing list