Re: [patch] Rewrite libc/sys/h8300hms/_exit.c in assembly.

[Jeff Johnston <jjohnstn at redhat dot com>]

Kazu Hirata wrote:
> Hi,
>
> Attached is a patch to rewrite _exit.c in assembly.
>
> The first two asm statements in each function in _exit.c write their
> results into r1 and r2, but nothing guarantees that their values are
> kept until the "sleep" instruction, which "uses" the values.
>
> The patch fixes the problem by rewriting the whole thing in assembly.
>
> This bug is triggered by gcc from tree-ssa branch, where "rc << 8" is
> performed in SImode, requiring a scratch register, which happens to be
> one of the registers that needs to be preserved until "sleep".
>
> Tested by building newlib with gcc.  OK to apply?

Kazu,

  I have one concern with your patch.  It appears that C functions map to names 
prepended by _ on h8300hms.  For example, the C library refers to _read, but it 
is defined in assembly as __read in read.S.  If you supply _exit() in assembly 
that may override C's exit() function which among other things, runs the 
atexit() list.  In the old C code, the __exit() routine should have been 
extraneous.  Please try taking out _exit in your assembly code _exit.S and 
trying to link a simple application which sets up an atexit() function and exits 
with a non-zero return code.  It should link with the __exit routine in your 
assembly code and run the atexit() list successfully.

-- Jeff J.


> Kazu Hirata
>
> 2004-03-10  Kazu Hirata  <kazu@cs.umass.edu>
>
> 	* libc/sys/h8300hms/Makefile.am (lib_a_SOURCES): Replace
> 	_exit.c with _exit.S.
> 	* libc/sys/h8300hms/Makefile.in: Regenerate.
> 	* libc/sys/h8300hms/_exit.c: Remove.
> 	* libc/sys/h8300hms/_exit.S: New.
>
> Index: Makefile.am
> ===================================================================
> RCS file: /cvs/src/src/newlib/libc/sys/h8300hms/Makefile.am,v
> retrieving revision 1.3
> diff -u -r1.3 Makefile.am
> --- Makefile.am	25 Feb 2003 20:36:29 -0000	1.3
> +++ Makefile.am	10 Mar 2004 18:35:09 -0000
> @@ -6,7 +6,7 @@
>  
>  noinst_LIBRARIES = lib.a
>  
> -lib_a_SOURCES = syscalls.c _exit.c sbrk.c misc.c crt1.c \
> +lib_a_SOURCES = syscalls.c _exit.S sbrk.c misc.c crt1.c \
>  	close.S fstat.S lseek.S open.S read.S stat.S write.S
>  
>  all: crt0.o
> Index: Makefile.in
> ===================================================================
> RCS file: /cvs/src/src/newlib/libc/sys/h8300hms/Makefile.in,v
> retrieving revision 1.8
> diff -u -r1.8 Makefile.in
> --- Makefile.in	23 Jan 2004 21:37:38 -0000	1.8
> +++ Makefile.in	10 Mar 2004 18:35:09 -0000
> @@ -89,7 +89,8 @@
>  
>  noinst_LIBRARIES = lib.a
>  
> -lib_a_SOURCES = syscalls.c _exit.c sbrk.c misc.c crt1.c 	close.S fstat.S lseek.S open.S read.S stat.S write.S
> +lib_a_SOURCES = syscalls.c _exit.S sbrk.c misc.c crt1.c \
> +	close.S fstat.S lseek.S open.S read.S stat.S write.S
>  
>  
>  ACLOCAL_AMFLAGS = -I ../../..
> Index: _exit.c
> ===================================================================
> RCS file: /cvs/src/src/newlib/libc/sys/h8300hms/_exit.c,v
> retrieving revision 1.2
> diff -u -r1.2 _exit.c
> --- _exit.c	31 Jan 2003 20:44:26 -0000	1.2
> +++ _exit.c	10 Mar 2004 18:37:17 -0000
> @@ -1,36 +0,1 @@
> -/* FIXME: which one? */
> -
> -#include <_ansi.h>
> -
> -/* `sleep' is passed an argument in r0 that indicates the reason
> -   the program is exiting.  The format of r0 is defined in devo/include/wait.h.
> -*/
> -
> -void
> -_DEFUN (_exit,(rc),
> -     int rc)
> -{
> -  short rc2;
> -
> -  rc2 = 0xdead;
> -  asm("mov.w %0,r1" : : "r" (rc2) : "r1");
> -  rc2 = 0xbeef;
> -  asm("mov.w %0,r2" : : "r" (rc2) : "r2");
> -  rc2 = rc << 8;
> -  asm("mov.w %0,r0\n\tsleep" : : "r" (rc2) : "r0");
> -}
> -
> -void
> -_DEFUN (__exit,(rc),
> -     int rc)
> -{
> -  short rc2;
> -
> -  rc2 = 0xdead;
> -  asm("mov.w %0,r1" : : "r" (rc2) : "r1");
> -  rc2 = 0xbeef;
> -  asm("mov.w %0,r2" : : "r" (rc2) : "r2");
> -  rc2 = rc << 8;
> -  asm("mov.w %0,r0\n\tsleep" : : "r" (rc2) : "r0");
> -}
> --- /dev/null	2003-09-15 09:40:47.000000000 -0400
> +++ _exit.S	2004-03-10 13:34:08.827057456 -0500
> @@ -0,0 +1,28 @@
> +#include "setarch.h"
> +
> +/* `sleep' is passed an argument in r0 that indicates the reason
> +   the program is exiting.  The format of r0 is defined in devo/include/wait.h.
> +*/
> +
> +/* FIXME: which one? */
> +/* extern void _exit (int rc); */
> +/* extern void __exit (int rc); */
> +
> +	.section .text
> +	.align 2
> +	.global __exit
> +__exit:
> +	mov.w	#0xdead,r1
> +	mov.w	#0xbeef,r2
> +	mov.b	r0l,r0h
> +	sub.b	r0l,r0l
> +	sleep
> +	.align 2
> +	.global ___exit
> +___exit:
> +	mov.w	#0xdead,r1
> +	mov.w	#0xbeef,r2
> +	mov.b	r0l,r0h
> +	sub.b	r0l,r0l
> +	sleep
> +	.end