diff mbox

Add more testing to ubsan #1

Message ID 20131216221239.GY892@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Dec. 16, 2013, 10:12 p.m. UTC 
On Mon, Dec 16, 2013 at 01:50:09PM -0800, H.J. Lu wrote:
> On Mon, Dec 16, 2013 at 1:44 PM, Dominique Dhumieres <dominiq@lps.ens.fr> wrote:
> >> I got ...
> >
> > me too on x86_64-apple-darwin13 with -m32 (gcc and g++).
> > r206009 is OK, r206026 is not.
> >
> 
> Mine is r206017 is OK and r206027 is bad. The change
> must be introduced between r2060178 and r206026.

When testing the patch the overflow-2.c testcase didn't exist yet,
nor was ubsan on -m32 actually ever reporting overflows on the DImode
multiplication (it simply expanded it as normal DImode multiplication with
no overflow checking).

To me this looks like very old bug (r2174 added it), will bootstrap/regtest
this:

2013-12-16  Jakub Jelinek  <jakub@redhat.com>

	* expr.c (convert_modes): For SUBREG_PROMOTED_VAR_P use SUBREG_REG (x)
	instead of x as last gen_lowpart argument.



	Jakub

Comments

Jakub Jelinek Dec. 17, 2013, 6:23 a.m. UTC | #1 
On Mon, Dec 16, 2013 at 11:12:40PM +0100, Jakub Jelinek wrote:
> When testing the patch the overflow-2.c testcase didn't exist yet,
> nor was ubsan on -m32 actually ever reporting overflows on the DImode
> multiplication (it simply expanded it as normal DImode multiplication with
> no overflow checking).
> 
> To me this looks like very old bug (r2174 added it), will bootstrap/regtest
> this:
> 
> 2013-12-16  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* expr.c (convert_modes): For SUBREG_PROMOTED_VAR_P use SUBREG_REG (x)
> 	instead of x as last gen_lowpart argument.
> 
> --- gcc/expr.c.jj	2013-12-12 09:39:45.000000000 +0100
> +++ gcc/expr.c	2013-12-16 23:05:07.519747459 +0100
> @@ -719,7 +719,7 @@ convert_modes (enum machine_mode mode, e
>    if (GET_CODE (x) == SUBREG && SUBREG_PROMOTED_VAR_P (x)
>        && GET_MODE_SIZE (GET_MODE (SUBREG_REG (x))) >= GET_MODE_SIZE (mode)
>        && SUBREG_PROMOTED_UNSIGNED_P (x) == unsignedp)
> -    x = gen_lowpart (mode, x);
> +    x = gen_lowpart (mode, SUBREG_REG (x));
>  
>    if (GET_MODE (x) != VOIDmode)
>      oldmode = GET_MODE (x);
> 

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

	Jakub
Richard Biener Dec. 17, 2013, 8:20 a.m. UTC | #2 
Jakub Jelinek <jakub@redhat.com> wrote:
>On Mon, Dec 16, 2013 at 11:12:40PM +0100, Jakub Jelinek wrote:
>> When testing the patch the overflow-2.c testcase didn't exist yet,
>> nor was ubsan on -m32 actually ever reporting overflows on the DImode
>> multiplication (it simply expanded it as normal DImode multiplication
>with
>> no overflow checking).
>> 
>> To me this looks like very old bug (r2174 added it), will
>bootstrap/regtest
>> this:
>> 
>> 2013-12-16  Jakub Jelinek  <jakub@redhat.com>
>> 
>> 	* expr.c (convert_modes): For SUBREG_PROMOTED_VAR_P use SUBREG_REG
>(x)
>> 	instead of x as last gen_lowpart argument.
>> 
>> --- gcc/expr.c.jj	2013-12-12 09:39:45.000000000 +0100
>> +++ gcc/expr.c	2013-12-16 23:05:07.519747459 +0100
>> @@ -719,7 +719,7 @@ convert_modes (enum machine_mode mode, e
>>    if (GET_CODE (x) == SUBREG && SUBREG_PROMOTED_VAR_P (x)
>>        && GET_MODE_SIZE (GET_MODE (SUBREG_REG (x))) >= GET_MODE_SIZE
>(mode)
>>        && SUBREG_PROMOTED_UNSIGNED_P (x) == unsignedp)
>> -    x = gen_lowpart (mode, x);
>> +    x = gen_lowpart (mode, SUBREG_REG (x));
>>  
>>    if (GET_MODE (x) != VOIDmode)
>>      oldmode = GET_MODE (x);
>> 
>
>Successfully bootstrapped/regtested on x86_64-linux and i686-linux, ok
>for
>trunk?

Ok.

Thanks,
Richard.

>	Jakub
diff mbox

Patch

--- gcc/expr.c.jj	2013-12-12 09:39:45.000000000 +0100
+++ gcc/expr.c	2013-12-16 23:05:07.519747459 +0100
@@ -719,7 +719,7 @@  convert_modes (enum machine_mode mode, e
   if (GET_CODE (x) == SUBREG && SUBREG_PROMOTED_VAR_P (x)
       && GET_MODE_SIZE (GET_MODE (SUBREG_REG (x))) >= GET_MODE_SIZE (mode)
       && SUBREG_PROMOTED_UNSIGNED_P (x) == unsignedp)
-    x = gen_lowpart (mode, x);
+    x = gen_lowpart (mode, SUBREG_REG (x));
 
   if (GET_MODE (x) != VOIDmode)
     oldmode = GET_MODE (x);