Re: [PATCH] Fix PR61335

Re: [PATCH] Fix PR61335

[Tobias Burnus]

Uros Bizjak wrote:
> Attached patch initializes problematic array to zero instead of
> uninitialized value.
>
> 2014-06-17  Uros Bizjak  <ubizjak@gmail.com>
>
>     * gfortran.dg/pr61335.f90 (cp_unit_create): Initialize
>     unit_id and kind_id to zero.
>
> Tested on alphaev68-linux-gnu and x86_64-linux-gnu.
> OK for mainline?

Looks good to me, is obvious and shouldn't affect the test case.

In particular the variables in questions aren't used in the
code after their initialization with an undefined implicitly
declared variable, which is also otherwise unused.

Tobias

Re: [PATCH] Fix PR61335

[Uros Bizjak]

On Fri, Jun 6, 2014 at 10:07 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Fri, Jun 6, 2014 at 9:47 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>
>>> 2014-05-28  Richard Biener  <rguenther@suse.de>
>>>
>>> PR tree-optimization/61335
>>> * tree-vrp.c (vrp_visit_phi_node): If the compare of old and
>>> new range fails, drop to varying.
>>>
>>> * gfortran.dg/pr61335.f90: New testcase.
>>
>> This testcase triggers SIGFPE on alpha due to the use of denormal
>> operand. Maybe uninitialized value is used in line 48?
>
> SIGFPE also triggers at the same place on x86_64 with unmasked FPE
> exceptions (compile with -O0).

Attached patch initializes problematic array to zero instead of
uninitialized value.

2014-06-17  Uros Bizjak  <ubizjak@gmail.com>

    * gfortran.dg/pr61335.f90 (cp_unit_create): Initialize
    unit_id and kind_id to zero.

Tested on alphaev68-linux-gnu and x86_64-linux-gnu.

OK for mainline?

Uros.

Index: gfortran.dg/pr61335.f90
===================================================================
--- gfortran.dg/pr61335.f90     (revision 211723)
+++ gfortran.dg/pr61335.f90     (working copy)
@@ -45,8 +45,8 @@
     LOGICAL                                  :: failure

     failure=.FALSE.
-    unit_id=cp_units_none
-    kind_id=cp_ukind_none
+    unit_id=0
+    kind_id=0
     power=0
     i_low=1
     i_high=1

Re: [PATCH] Fix PR61335

[Uros Bizjak]

On Fri, Jun 6, 2014 at 9:47 AM, Uros Bizjak <ubizjak@gmail.com> wrote:

>> 2014-05-28  Richard Biener  <rguenther@suse.de>
>>
>> PR tree-optimization/61335
>> * tree-vrp.c (vrp_visit_phi_node): If the compare of old and
>> new range fails, drop to varying.
>>
>> * gfortran.dg/pr61335.f90: New testcase.
>
> This testcase triggers SIGFPE on alpha due to the use of denormal
> operand. Maybe uninitialized value is used in line 48?

SIGFPE also triggers at the same place on x86_64 with unmasked FPE
exceptions (compile with -O0).

(gdb) b main
Breakpoint 1 at 0x401602: file
/home/uros/gcc-svn/trunk/gcc/testsuite/gfortran.dg/pr61335.f90, line
115.
(gdb) r
Starting program: /home/uros/test/pr61335.exe
warning: no loadable sections found in added symbol-file
system-supplied DSO at 0x2aaaaaaab000

Breakpoint 1, main (argc=1, argv=0x7fffffffd88e) at
/home/uros/gcc-svn/trunk/gcc/testsuite/gfortran.dg/pr61335.f90:115
115     USE cp_units
(gdb) i r mxcsr
mxcsr          0x1f80   [ IM DM ZM OM UM PM ]
(gdb) set $mxcsr=0x1000
(gdb) i r mxcsr
mxcsr          0x1000   [ PM ]
(gdb) c
Continuing.

Program received signal SIGFPE, Arithmetic exception.
0x0000000000400b60 in cp_units::cp_unit_create (string=<error reading
variable: Cannot access memory at address 0x401c47>, _string=5)
    at /home/uros/gcc-svn/trunk/gcc/testsuite/gfortran.dg/pr61335.f90:49
49          kind_id=cp_ukind_none

   0x0000000000400b57 <+95>:    mov    -0x28(%rbp),%eax
   0x0000000000400b5a <+98>:    mov    %eax,-0x280(%rbp)
=> 0x0000000000400b60 <+104>:   cvttss2si -0x280(%rbp),%ecx

(gdb) i r mxcsr
mxcsr          0x1021   [ IE PE PM ]

Uros.

Re: [PATCH] Fix PR61335

[Uros Bizjak]

Hello!

> 2014-05-28  Richard Biener  <rguenther@suse.de>
>
> PR tree-optimization/61335
> * tree-vrp.c (vrp_visit_phi_node): If the compare of old and
> new range fails, drop to varying.
>
> * gfortran.dg/pr61335.f90: New testcase.

This testcase triggers SIGFPE on alpha due to the use of denormal
operand. Maybe uninitialized value is used in line 48?

Reading symbols from ./pr61335.exe...done.
(gdb) r
Starting program: /space/homedirs/uros/test/pr61335.exe

Program received signal SIGFPE, Arithmetic exception.
0x0000000120000b54 in cp_units::cp_unit_create (string=<error reading
variable: Cannot access memory at address 0x120004000>, _string=5)
    at /home/uros/gcc-svn/trunk/gcc/testsuite/gfortran.dg/pr61335.f90:48
48          unit_id=cp_units_none
(gdb) list
43                                                      len_string, next_power
44          INTEGER, DIMENSION(cp_unit_max_kinds)    :: kind_id, power, unit_id
45          LOGICAL                                  :: failure
46
47          failure=.FALSE.
48          unit_id=cp_units_none
49          kind_id=cp_ukind_none
50          power=0
51          i_low=1
52          i_high=1

The exception is triggered in 0x120000b50, but emitted on the next FP insn.

   0x0000000120000b4c <+76>:    lds     $f10,48(fp)
   0x0000000120000b50 <+80>:    cvttq/c $f10,$f10
=> 0x0000000120000b54 <+84>:    ftoit   $f10,t0

(gdb) b *0x120000b50
Breakpoint 1 at 0x120000b50: file
/home/uros/gcc-svn/trunk/gcc/testsuite/gfortran.dg/pr61335.f90, line
48.
(gdb) r
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: /space/homedirs/uros/test/pr61335.exe

Breakpoint 1, 0x0000000120000b50 in cp_units::cp_unit_create
(string=<error reading variable: Cannot access memory at address
0x120004000>,
    _string=5) at
/home/uros/gcc-svn/trunk/gcc/testsuite/gfortran.dg/pr61335.f90:48
48          unit_id=cp_units_none

(gdb) i r $f10
f10            8.0173244974249919e-310  (raw 0x0000939600000000)

The test passes with -mieee that allows denormals.

Uros.

[PATCH] Fix PR61335

[Richard Biener]

This fixes PR61335 - symbolic range propagation in VRP is so weak
that we didn't notice this very old serious bug ... compare_values
can return "don't know" which is -2, but PHI node visiting handles
it the same as -1 (less than...).  Oops.

Bootstrap and regtest running on x86_64-unknown-linux-gnu.

Richard.

2014-05-28  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/61335
	* tree-vrp.c (vrp_visit_phi_node): If the compare of old and
	new range fails, drop to varying.

	* gfortran.dg/pr61335.f90: New testcase.

Index: gcc/tree-vrp.c
===================================================================
*** gcc/tree-vrp.c	(revision 210973)
--- gcc/tree-vrp.c	(working copy)
*************** vrp_visit_phi_node (gimple phi)
*** 8323,8330 ****
--- 8323,8336 ----
        && edges == old_edges
        && lhs_vr->type != VR_UNDEFINED)
      {
+       /* Compare old and new ranges, fall back to varying if the
+          values are not comparable.  */
        int cmp_min = compare_values (lhs_vr->min, vr_result.min);
+       if (cmp_min == -2)
+ 	goto varying;
        int cmp_max = compare_values (lhs_vr->max, vr_result.max);
+       if (cmp_max == -2)
+ 	goto varying;
  
        /* For non VR_RANGE or for pointers fall back to varying if
  	 the range changed.  */
Index: gcc/testsuite/gfortran.dg/pr61335.f90
===================================================================
*** gcc/testsuite/gfortran.dg/pr61335.f90	(revision 0)
--- gcc/testsuite/gfortran.dg/pr61335.f90	(working copy)
***************
*** 0 ****
--- 1,117 ----
+ ! { dg-do run }
+ ! { dg-additional-options "-fbounds-check" }
+ MODULE cp_units
+ 
+   INTEGER, PARAMETER :: default_string_length=80, dp=KIND(0.0D0)
+ 
+   LOGICAL, PRIVATE, PARAMETER          :: debug_this_module=.TRUE.
+   CHARACTER(len=*), PARAMETER, PRIVATE :: moduleN = 'cp_units'
+   INTEGER, SAVE, PRIVATE               :: last_unit_id=0, last_unit_set_id=0
+ 
+   INTEGER, PARAMETER, PUBLIC :: cp_unit_max_kinds=8, cp_unit_basic_desc_length=15,&
+        cp_unit_desc_length=cp_unit_max_kinds*cp_unit_basic_desc_length, cp_ukind_max=9
+ 
+ CONTAINS
+ 
+   FUNCTION cp_to_string(i) RESULT(res)
+     INTEGER, INTENT(in)                      :: i
+     CHARACTER(len=6)                         :: res
+ 
+     INTEGER                                  :: iostat
+     REAL(KIND=dp)                            :: tmp_r
+ 
+     IF (i>999999 .OR. i<-99999) THEN
+        tmp_r=i
+        WRITE (res,fmt='(es6.1)',iostat=iostat) tmp_r
+     ELSE
+        WRITE (res,fmt='(i6)',iostat=iostat) i
+     END IF
+     IF (iostat/=0) THEN
+        STOP 7
+     END IF
+   END FUNCTION cp_to_string
+ 
+   SUBROUTINE cp_unit_create(string)
+     CHARACTER(len=*), INTENT(in)             :: string
+ 
+     CHARACTER(len=*), PARAMETER :: routineN = 'cp_unit_create', &
+       routineP = moduleN//':'//routineN
+ 
+     CHARACTER(default_string_length)         :: desc
+     CHARACTER(LEN=40)                        :: formatstr
+     INTEGER                                  :: i_high, i_low, i_unit, &
+                                                 len_string, next_power
+     INTEGER, DIMENSION(cp_unit_max_kinds)    :: kind_id, power, unit_id
+     LOGICAL                                  :: failure
+ 
+     failure=.FALSE.
+     unit_id=cp_units_none
+     kind_id=cp_ukind_none
+     power=0
+     i_low=1
+     i_high=1
+     len_string=LEN(string)
+     i_unit=0
+     next_power=1
+     DO WHILE(i_low<len_string)
+        IF (string(i_low:i_low)/=' ') EXIT
+        i_low=i_low+1
+     END DO
+     i_high=i_low
+     DO WHILE(i_high<=len_string)
+        IF ( string(i_high:i_high)==' '.OR.string(i_high:i_high)=='^'.OR.&
+             string(i_high:i_high)=='*'.OR.string(i_high:i_high)=='/') EXIT
+        i_high=i_high+1
+     END DO
+     DO WHILE(.NOT.failure)
+        IF (i_high<=i_low.OR.i_low>len_string) EXIT
+        i_unit=i_unit+1
+        IF (i_unit>cp_unit_max_kinds) THEN
+           EXIT
+        END IF
+        power(i_unit)=next_power
+        ! parse op
+        i_low=i_high
+        DO WHILE(i_low<=len_string)
+           IF (string(i_low:i_low)/=' ') EXIT
+           i_low=i_low+1
+        END DO
+        i_high=i_low
+        DO WHILE(i_high<=len_string)
+           IF ( string(i_high:i_high)==' '.OR.string(i_high:i_high)=='^'.OR.&
+                string(i_high:i_high)=='*'.OR.string(i_high:i_high)=='/') EXIT
+           i_high=i_high+1
+        END DO
+        IF (i_high<i_low.OR.i_low>len_string) EXIT
+ 
+        IF (i_high<=len_string) THEN
+           IF (string(i_low:i_high)=='^') THEN
+              i_low=i_high+1
+              DO WHILE(i_low<=len_string)
+                 IF (string(i_low:i_low)/=' ') EXIT
+                 i_low=i_low+1
+              END DO
+              i_high=i_low
+              DO WHILE(i_high<=len_string)
+                 SELECT CASE(string(i_high:i_high))
+                 CASE('+','-','0','1','2','3','4','5','6','7','8','9')
+                    i_high=i_high+1
+                 CASE default
+                    EXIT
+                 END SELECT
+              END DO
+              IF (i_high<=i_low.OR.i_low>len_string) THEN
+                 write(6,*) "BUG : XXX"//string//"XXX integer expected"
+                 STOP 1
+                 EXIT
+              END IF
+           END IF
+        ENDIF
+     END DO
+   END SUBROUTINE cp_unit_create
+ 
+ END MODULE cp_units
+ 
+ USE cp_units
+ CALL cp_unit_create("fs^-1")
+ END