Patchwork Fix two undefined behaviors in gcc sources

login
register
mail settings
Submitter Jakub Jelinek
Date March 21, 2014, 8:30 p.m.
Message ID <20140321203025.GV1817@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/332744/
State New
Headers show

Comments

Jakub Jelinek - March 21, 2014, 8:30 p.m.
Hi!

--with-build-config=bootstrap-ubsan on i686-linux showed
most often these two issues.

The first one is that 32-bit signed time_t is multiplied by 1000, which
overflows the int type.  The result is then cast to unsigned, so even on
64-bit hosts we don't care about the upper bits.

The other change fixes a badly written portable rotate, which was shitfing
up (correctly) by 0 for i == 0, but for all other i values was shifting up
64-bit value by 64.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2014-03-21  Jakub Jelinek  <jakub@redhat.com>

	* toplev.c (init_local_tick): Avoid signed integer multiplication
	overflow.
	* genautomata.c (reserv_sets_hash_value): Fix rotate idiom, avoid
	shift by first operand's bitsize.


	Jakub
Richard Guenther - March 22, 2014, 12:51 p.m.
On March 21, 2014 9:30:25 PM CET, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>--with-build-config=bootstrap-ubsan on i686-linux showed
>most often these two issues.
>
>The first one is that 32-bit signed time_t is multiplied by 1000, which
>overflows the int type.  The result is then cast to unsigned, so even
>on
>64-bit hosts we don't care about the upper bits.
>
>The other change fixes a badly written portable rotate, which was
>shitfing
>up (correctly) by 0 for i == 0, but for all other i values was shifting
>up
>64-bit value by 64.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.
Thanks,
Richard.

>2014-03-21  Jakub Jelinek  <jakub@redhat.com>
>
>	* toplev.c (init_local_tick): Avoid signed integer multiplication
>	overflow.
>	* genautomata.c (reserv_sets_hash_value): Fix rotate idiom, avoid
>	shift by first operand's bitsize.
>
>--- gcc/toplev.c.jj	2014-01-08 10:23:22.000000000 +0100
>+++ gcc/toplev.c	2014-03-21 11:32:25.929893013 +0100
>@@ -261,7 +261,7 @@ init_local_tick (void)
> 	struct timeval tv;
> 
> 	gettimeofday (&tv, NULL);
>-	local_tick = tv.tv_sec * 1000 + tv.tv_usec / 1000;
>+	local_tick = (unsigned) tv.tv_sec * 1000 + tv.tv_usec / 1000;
>       }
> #else
>       {
>--- gcc/genautomata.c.jj	2014-02-12 08:33:34.000000000 +0100
>+++ gcc/genautomata.c	2014-03-21 12:01:14.245727924 +0100
>@@ -3494,7 +3494,7 @@ reserv_sets_hash_value (reserv_sets_t re
>     {
>       reservs_num--;
>       hash_value += ((*reserv_ptr >> i)
>-		     | (*reserv_ptr << ((sizeof (set_el_t) * CHAR_BIT) & -i)));
>+		     | (*reserv_ptr << (((sizeof (set_el_t) * CHAR_BIT) - 1) &
>-i)));
>       i++;
>       if (i == sizeof (set_el_t) * CHAR_BIT)
> 	i = 0;
>
>	Jakub

Patch

--- gcc/toplev.c.jj	2014-01-08 10:23:22.000000000 +0100
+++ gcc/toplev.c	2014-03-21 11:32:25.929893013 +0100
@@ -261,7 +261,7 @@  init_local_tick (void)
 	struct timeval tv;
 
 	gettimeofday (&tv, NULL);
-	local_tick = tv.tv_sec * 1000 + tv.tv_usec / 1000;
+	local_tick = (unsigned) tv.tv_sec * 1000 + tv.tv_usec / 1000;
       }
 #else
       {
--- gcc/genautomata.c.jj	2014-02-12 08:33:34.000000000 +0100
+++ gcc/genautomata.c	2014-03-21 12:01:14.245727924 +0100
@@ -3494,7 +3494,7 @@  reserv_sets_hash_value (reserv_sets_t re
     {
       reservs_num--;
       hash_value += ((*reserv_ptr >> i)
-		     | (*reserv_ptr << ((sizeof (set_el_t) * CHAR_BIT) & -i)));
+		     | (*reserv_ptr << (((sizeof (set_el_t) * CHAR_BIT) - 1) & -i)));
       i++;
       if (i == sizeof (set_el_t) * CHAR_BIT)
 	i = 0;