Linux/m68k port (was: Debian bugs information: logs for bug#24611)

[Fergus Henderson]

On 21-Jul-1998, Tyson Dowd <trd at stimpy.cs.mu.oz.au> wrote:
> This one is from the Debian bug tracking system.
> I'll working on it at some stage, just thought peopel would
> like to know.

Thanks Tyson.

Here's some comments on the diff as it is.
Hopefully these will be helpful.
Feel free to forward these comments on if you want.

> I've now ported mercury to m68k. A patch is below. It would be nice if
> you could integrate it into the next Debian version, and also would
> forward it to the upstream author(s).
...
>  - runtime/faultaddr.h: New header (used only on Linux) that provides
>    a macro GET_FAULT_ADDR, that extracts the fault addr of a SIGSEGV
>    from the signal handler parameters.

The header file should be called mercury_faultaddr.h.
The macro should be called MR_GET_FAULT_ADDR.

>  - boehm_gc/os_dep.c: Use faultaddr.h for GET_FAULT_ADDR macro

Stuff in boehm_gc should not depend on stuff in runtime.

>  - configure, configure.in: Modified test for "working struct
>    sigcontext" to also use faultaddr.h; a "working sigcontext" doesn't
>    mean there must be a cr2 component :-)
> 
>    Also any non-i386 Linux arch can use shared libraries (all Linux
>    uses ELF).

Yes, but shared libraries might not be safe on unknown architectures,
because we use non-local gotos, and often this doesn't work with `-fpic'
without architecture-specific code in runtime/mercury_goto.h.

>  - debian/control: Set Architecture: to "i386 m68k" instead of "any",
>    because mercury now has been ported only to those two archs.

Alpha and PPC should be included in that list too,

boehm_gc/config.h:
> +# if defined(linux) && defined(__mc68000__)
> +#    define M68K
> +#    define LINUX
> +#    define mach_type_known
> +# endif

Should check for `__linux__' instead of, or as well as, `linux'.

> +++ mercury-0.7.3/configure.in  Fri Jul 10 00:08:26 1998
> @@ -345,8 +345,9 @@
>  #endif
> 
>  #include <stdio.h>
> +#include "runtime/faultaddr.h"
> 
> -extern void handler(int signum, struct sigcontext_struct info);
> +extern void handler(int signum, int code, struct sigcontext_struct *context);

I don't understand that one.
Won't that break things on x86?

> --- mercury-0.7.3-orig/runtime/faultaddr.h      Thu Jan  1 01:00:00 1970
> +++ mercury-0.7.3/runtime/faultaddr.h   Fri Jul 10 00:15:16 1998
> @@ -0,0 +1,42 @@
> +/*
> + * Determine fault address from struct sigcontext on Linux
> + */

Non-standard comment format.
Should be

	/*
	** Determine fault address from struct sigcontext on Linux
	*/

> +#if defined(__i386__)
> +
> +#define GET_FAULT_ADDR(sig,code,sc) \
> +    ((void *)(((struct sigcontext_struct *)(&code))->cr2))

Oh, now I see why it doesn't break things on x86.  Boy is that ugly.

I think it would be better to conditionalize the declaration
(perhaps via some macro in a header file) rather than having the
declaration lie and then using a cast here.

The macro definition should be intended two spaces, since it
is inside `#if'.

> +#else
> +
> +#error No fault addr extraction on this architecture

There shouldn't be a #error here; just leave GET_FAULT_ADDR
undefined, and you'll get a compile error if and only if it is used.

> +++ mercury-0.7.3/runtime/memory.c      Fri Jul 10 01:26:31 1998
> @@ -140,7 +140,11 @@
> 
>  #ifdef HAVE_SIGINFO
>    #if defined(HAVE_SIGCONTEXT_STRUCT)
> -    static     void    complex_sighandler(int, struct sigcontext_struct);
> +    #ifdef linux
> +      static   void    complex_sighandler(int, int, struct sigcontext_struct);
> +    #else
> +      static   void    complex_sighandler(int, struct sigcontext_struct);
> +    #endif

`#ifdef linux' is not quite right here.
For one thing, it should be `#ifdef __linux__'.
But in general it would be better to autoconf this somehow
rather than having specific OS tests here.

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