Overflow fix (was Problems with trampoline.S with 64 bit binutils)

Overflow fix (was Problems with trampoline.S with 64 bit binutils)

[Alan Modra]

Is it OK for me to check this one in, Ian?  It's almost in the "obvious
fix" category...

On Tue, 27 Jul 1999, I wrote:

> This is more elegant, and the concern voiced in the comment is no longer
> relevant as we check for size < sizof(valueT)
> 
> --- binutils-current/gas/write.c~	Fri Jul 16 17:57:19 1999
> +++ binutils-current/gas/write.c	Tue Jul 27 20:32:04 1999
> @@ -2733,24 +2733,13 @@
>  	{
>  	  if ((size_t) size < sizeof (valueT))
>  	    {
> -	      valueT mask, hibit;
> +	      valueT mask;
>  
> -	      /* set all bits to one */
>  	      mask = 0;
> -	      mask--;
> -	      /* Technically, combining these produces an undefined result
> -		 if size is sizeof (valueT), though I think these two
> -		 half-way operations should both be defined.  And the
> -		 compiler should be able to combine them if it's valid on
> -		 the host architecture.  */
> -	      mask <<= size * 4;
> -	      mask <<= size * 4;
> -	      hibit = (valueT) 1 << (size * 8 - 1);
> -	      if (((add_number & mask) != 0
> -		   || (fixP->fx_signed
> -		       && (add_number & hibit) != 0))
> -		  && ((add_number & mask) != mask
> -		      || (add_number & hibit) == 0))
> +	      mask--;	      /* set all bits to one */
> +	      mask <<= size * 8 - (fixP->fx_signed ? 1 : 0);
> +	      if ((add_number & mask) != 0
> +		  && (add_number & mask) != mask)
>  		{
>  		  char buf[50], buf2[50];
>  		  sprint_value (buf, fragP->fr_address + where);

Re: Overflow fix (was Problems with trampoline.S with 64 bit binutils)

[Ian Lance Taylor]

   Date: Wed, 28 Jul 1999 18:39:18 +0930 (CST)
   From: Alan Modra <alan@SPRI.Levels.UniSA.Edu.Au>

   Is it OK for me to check this one in, Ian?  It's almost in the "obvious
   fix" category...

Almost.

   > +	      mask <<= size * 8 - (fixP->fx_signed ? 1 : 0);

If size is 4 and fixP->fx_signed is not set, then this will do a left
shift by 32.  If mask is itself 32 bits, then this is not defined by
ANSI C.  There are in fact compilers which will turn this into a
no-op: they generate a shift by 32 instruction, which is masked into a
shift by 0 instruction.

That's why I had the two shifts in the old code, and I think we need
to keep that.

Ian

Re: Overflow fix (was Problems with trampoline.S with 64 bit binutils)

[Alan Modra]

On 28 Jul 1999, Ian Lance Taylor wrote:

>    Date: Wed, 28 Jul 1999 18:39:18 +0930 (CST)
>    From: Alan Modra <alan@SPRI.Levels.UniSA.Edu.Au>
> 
>    Is it OK for me to check this one in, Ian?  It's almost in the "obvious
>    fix" category...
> 
> Almost.
> 
>    > +	      mask <<= size * 8 - (fixP->fx_signed ? 1 : 0);
> 
> If size is 4 and fixP->fx_signed is not set, then this will do a left
> shift by 32.  If mask is itself 32 bits, then this is not defined by
> ANSI C.  There are in fact compilers which will turn this into a
> no-op: they generate a shift by 32 instruction, which is masked into a
> shift by 0 instruction.
> 
> That's why I had the two shifts in the old code, and I think we need
> to keep that.

size == 4  =>  sizeof (mask) > 4.  Surely ANSI C doesn't restrict shift
counts of 64 bit quantities to 32??

Re: Overflow fix (was Problems with trampoline.S with 64 bit binutils)

[Ian Lance Taylor]

   Date: Thu, 29 Jul 1999 08:27:32 +0930 (CST)
   From: Alan Modra <alan@SPRI.Levels.UniSA.Edu.Au>

   On 28 Jul 1999, Ian Lance Taylor wrote:

   >    Date: Wed, 28 Jul 1999 18:39:18 +0930 (CST)
   >    From: Alan Modra <alan@SPRI.Levels.UniSA.Edu.Au>
   > 
   >    Is it OK for me to check this one in, Ian?  It's almost in the "obvious
   >    fix" category...
   > 
   > Almost.
   > 
   >    > +	      mask <<= size * 8 - (fixP->fx_signed ? 1 : 0);
   > 
   > If size is 4 and fixP->fx_signed is not set, then this will do a left
   > shift by 32.  If mask is itself 32 bits, then this is not defined by
   > ANSI C.  There are in fact compilers which will turn this into a
   > no-op: they generate a shift by 32 instruction, which is masked into a
   > shift by 0 instruction.
   > 
   > That's why I had the two shifts in the old code, and I think we need
   > to keep that.

   size == 4  =>  sizeof (mask) > 4.  Surely ANSI C doesn't restrict shift
   counts of 64 bit quantities to 32??

Whoops, sorry, I totally missed the initial if condition.  Yes, please
check in this patch.

Ian

make check

[Alan Modra]

I've been slightly annoyed that "make check" always did a "make all" for
quite a while, but never annoyed enough to fix it until now.

Jason, on Oct 27 1997 you added the check-target-libio dependency on
all-target-libio.  Has the need for this gone?

Ulrich, do we need "check" to depend on "all" in your po/Makefiles?

I think I need an OK from both of you (and Ian) before checking this in.

Alan Modra


--- binutils-current/bfd/po/Make-in~	Mon May  3 16:58:58 1999
+++ binutils-current/bfd/po/Make-in	Thu Jul 29 11:09:46 1999
@@ -178,7 +178,7 @@
 	done
 	rm -f $(gettextsrcdir)/po-Makefile.in.in
 
-check: all
+check:
 
 cat-id-tbl.o: ../intl/libgettext.h
 
--- binutils-current/Makefile.in~	Wed Jul 28 14:00:16 1999
+++ binutils-current/Makefile.in	Thu Jul 29 11:38:31 1999
@@ -1583,7 +1583,7 @@
 all-target-libgloss: configure-target-libgloss configure-target-newlib
 configure-target-libio: $(ALL_GCC)
 all-target-libio: configure-target-libio all-gas all-ld all-gcc all-target-libiberty all-target-newlib
-check-target-libio: all-target-libstdc++
+check-target-libio:
 all-libgui: all-tcl all-tk all-tcl8.1 all-tk8.1 all-itcl
 all-libiberty:
 configure-target-librx: $(ALL_GCC) configure-target-newlib
--- binutils-current/binutils/po/Make-in~	Mon May  3 16:59:10 1999
+++ binutils-current/binutils/po/Make-in	Thu Jul 29 11:10:29 1999
@@ -178,7 +178,7 @@
 	done
 	rm -f $(gettextsrcdir)/po-Makefile.in.in
 
-check: all
+check:
 
 cat-id-tbl.o: ../intl/libgettext.h
 
--- binutils-current/gas/po/Make-in~	Mon May  3 16:58:45 1999
+++ binutils-current/gas/po/Make-in	Thu Jul 29 11:11:15 1999
@@ -178,7 +178,7 @@
 	done
 	rm -f $(gettextsrcdir)/po-Makefile.in.in
 
-check: all
+check:
 
 cat-id-tbl.o: ../intl/libgettext.h
 
--- binutils-current/gprof/po/Make-in~	Mon May  3 16:59:11 1999
+++ binutils-current/gprof/po/Make-in	Thu Jul 29 11:15:17 1999
@@ -178,7 +178,7 @@
 	done
 	rm -f $(gettextsrcdir)/po-Makefile.in.in
 
-check: all
+check:
 
 cat-id-tbl.o: ../intl/libgettext.h
 
--- binutils-current/intl/Makefile.in~	Mon May  3 16:59:05 1999
+++ binutils-current/intl/Makefile.in	Thu Jul 29 11:10:00 1999
@@ -99,7 +99,7 @@
 ../po/cat-id-tbl.$lo: ../po/cat-id-tbl.c $(top_srcdir)/po/$(PACKAGE).pot
 	cd ../po && $(MAKE) cat-id-tbl.$lo
 
-check: all
+check:
 
 # This installation goal is only used in GNU gettext.  Packages which
 # only use the library should use install instead.
--- binutils-current/ld/po/Make-in~	Mon May  3 16:59:07 1999
+++ binutils-current/ld/po/Make-in	Thu Jul 29 11:10:51 1999
@@ -178,7 +178,7 @@
 	done
 	rm -f $(gettextsrcdir)/po-Makefile.in.in
 
-check: all
+check:
 
 cat-id-tbl.o: ../intl/libgettext.h
 
--- binutils-current/opcodes/po/Make-in~	Mon May  3 16:59:01 1999
+++ binutils-current/opcodes/po/Make-in	Thu Jul 29 11:03:48 1999
@@ -178,7 +178,7 @@
 	done
 	rm -f $(gettextsrcdir)/po-Makefile.in.in
 
-check: all
+check:
 
 cat-id-tbl.o: ../intl/libgettext.h
 

more "make check" issues

[Alan Modra]

This one I've been ignoring for a long time.
                === binutils tests ===

Running /usr/src/binutils-current/binutils/testsuite/binutils-all/ar.exp ...
ERROR: (DejaGnu) proc "is_remote host" does not exist.
The error code is 


This one is new.
                === ld tests ===

Running /usr/src/binutils-current/ld/testsuite/ld-bootstrap/bootstrap.exp ...
ERROR: (DejaGnu) proc "target_info name" does not exist.
The error code is 


Target is i586-linuxlibc1 if it matters.  Time to update my dejagnu?

Re: make check

[Jason Merrill]

This seems like it's the domain of the GNU coding standards.  What do they
say?  I don't care much one way or the other.

Jason

Re: make check

[Ian Lance Taylor]

   Date: Thu, 29 Jul 1999 11:58:24 +0930 (CST)
   From: Alan Modra <alan@SPRI.Levels.UniSA.Edu.Au>

   I've been slightly annoyed that "make check" always did a "make all" for
   quite a while, but never annoyed enough to fix it until now.

   Jason, on Oct 27 1997 you added the check-target-libio dependency on
   all-target-libio.  Has the need for this gone?

   Ulrich, do we need "check" to depend on "all" in your po/Makefiles?

   I think I need an OK from both of you (and Ian) before checking this in.

These changes are OK with me.  Note that you can change them in the
binutils repository without affecting any other repository.

Of course, it would probably be nicer if we all agreed on the proper
solution.  I tend to agree that ``make check'' should not imply ``make
all.''  The GNU standards say this:
    Perform self-tests (if any).  The user must build the program before
    running the tests, but need not install the program; you should write
    the self-tests so that they work when the program is built but not
    installed.
which is ambiguous, but I think it implies that ``make check'' need
not imply ``make all''.

Ian

Re: make check

[Ulrich Drepper]

Ian Lance Taylor <ian@zembu.com> writes:

>    Ulrich, do we need "check" to depend on "all" in your po/Makefiles?

Yes, this is what we are doing everywhere.

-- 
---------------.      drepper at gnu.org  ,-.   1325 Chesapeake Terrace
Ulrich Drepper  \    ,-------------------'   \  Sunnyvale, CA 94089 USA
Cygnus Solutions `--' drepper at cygnus.com   `------------------------

Re: make check

[Alan Modra]

On 29 Jul 1999, Ulrich Drepper wrote:

> > Ulrich, do we need "check" to depend on "all" in your po/Makefiles?
> 
> Yes, this is what we are doing everywhere.

Which in the binutils/*/po/ Makefiles is particularly silly.  "make check"
in these directories doesn't do any checking!

Not that this particularly concerns me, as leaving in the dependency there
just does a local "make all" in that directory.  The dependency I will
remove, since Jason didn't object, is

check-target-libio: all-target-libstdc++

in binutils/Makefile, as this results in a global "make all"

Re: more "make check" issues

[Ian Lance Taylor]

   Date: Thu, 29 Jul 1999 12:05:22 +0930 (CST)
   From: Alan Modra <alan@SPRI.Levels.UniSA.Edu.Au>

   This one I've been ignoring for a long time.
		   === binutils tests ===

   Running /usr/src/binutils-current/binutils/testsuite/binutils-all/ar.exp ...
   ERROR: (DejaGnu) proc "is_remote host" does not exist.
   The error code is 


   This one is new.
		   === ld tests ===

   Running /usr/src/binutils-current/ld/testsuite/ld-bootstrap/bootstrap.exp ...
   ERROR: (DejaGnu) proc "target_info name" does not exist.
   The error code is 


   Target is i586-linuxlibc1 if it matters.  Time to update my dejagnu?

I've been assuming that the dejagnu sources which can be found at
    ftp://sourceware.cygnus.com/pub/gcc/infrastructure
will work for the binutils.  Can anybody confirm or deny that?

Ian