[Patch, fortran] PR66929 fix iso_varying_string ICE

[Patch, fortran] PR66929 fix iso_varying_string ICE

[Mikael Morin]

[-- Attachment #1: Type: text/plain, Size: 398 bytes --]

Hello,

The fix for PR61831 committed recently [1] introduced/uncovered a NULLL 
pointer dereference with iso_varying_string, because a generic symbol 
(which has a NULL result) is used as procedure symbol, instead of the 
specific one.
Fixed by using esym if it's available.

Regression-tested on x86_64-linux. OK for trunk?

Mikael

[1]: https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01389.html


[-- Attachment #2: pr66929_1.CL --]
[-- Type: text/plain, Size: 295 bytes --]

2015-07-20  Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/61831
	PR fortran/66929
	* trans-array.c (gfc_get_proc_ifc_for_expr): Use esym as procedure
	symbol if available.

2015-07-20  Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/61831
	PR fortran/66929
	* gfortran.dg/generic_30.f90: New.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: pr66929_1.diff --]
[-- Type: text/x-patch; name="pr66929_1.diff", Size: 619 bytes --]

Index: trans-array.c
===================================================================
--- trans-array.c	(révision 225979)
+++ trans-array.c	(copie de travail)
@@ -9166,7 +9166,11 @@ gfc_get_proc_ifc_for_expr (gfc_expr *procedure_ref
     return NULL;
 
   /* Normal procedure case.  */
-  sym = procedure_ref->symtree->n.sym;
+  if (procedure_ref->expr_type == EXPR_FUNCTION
+      && procedure_ref->value.function.esym)
+    sym = procedure_ref->value.function.esym;
+  else
+    sym = procedure_ref->symtree->n.sym;
 
   /* Typebound procedure case.  */
   for (ref = procedure_ref->ref; ref; ref = ref->next)


[-- Attachment #4: generic_30.f90 --]
[-- Type: text/x-fortran, Size: 1360 bytes --]

! { dg-do compile }
!
! PR fortran/66929
! Generic procedures as actual argument used to lead to
! a NULL pointer dereference in gfc_get_proc_ifc_for_expr
! because the generic symbol was used as procedure symbol,
! instead of the specific one.

module iso_varying_string
  type, public :: varying_string
     character(LEN=1), dimension(:), allocatable :: chars
  end type varying_string
  interface operator(/=)
     module procedure op_ne_VS_CH
  end interface operator (/=)
  interface trim
     module procedure trim_
  end interface
contains
  elemental function op_ne_VS_CH (string_a, string_b) result (op_ne)
    type(varying_string), intent(in) :: string_a
    character(LEN=*), intent(in)     :: string_b
    logical                          :: op_ne
    op_ne = .true.
  end function op_ne_VS_CH
  elemental function trim_ (string) result (trim_string)
    type(varying_string), intent(in) :: string
    type(varying_string)             :: trim_string
    trim_string = varying_string(["t", "r", "i", "m", "m", "e", "d"])
  end function trim_
end module iso_varying_string
module syntax_rules
  use iso_varying_string, string_t => varying_string
contains
  subroutine set_rule_type_and_key
    type(string_t) :: key
    if (trim (key) /= "") then
      print *, "non-empty"
    end if
  end subroutine set_rule_type_and_key
end module syntax_rules

Re: [Patch, fortran] PR66929 fix iso_varying_string ICE

[Paul Richard Thomas]

Hi Mikael,

This looks fine to me - OK for trunk.

Thanks for the patch

Paul

On 21 July 2015 at 14:53, Mikael Morin <mikael.morin@sfr.fr> wrote:
> Hello,
>
> The fix for PR61831 committed recently [1] introduced/uncovered a NULLL
> pointer dereference with iso_varying_string, because a generic symbol (which
> has a NULL result) is used as procedure symbol, instead of the specific one.
> Fixed by using esym if it's available.
>
> Regression-tested on x86_64-linux. OK for trunk?
>
> Mikael
>
> [1]: https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01389.html
>



-- 
Outside of a dog, a book is a man's best friend. Inside of a dog it's
too dark to read.

Groucho Marx

Re: [Patch, fortran] PR66929 fix iso_varying_string ICE

[Mikael Morin]

[-- Attachment #1: Type: text/plain, Size: 1020 bytes --]

Le 21/07/2015 23:10, Paul Richard Thomas a écrit :
> Hi Mikael,
>
> This looks fine to me - OK for trunk.
>
> Thanks for the patch
>
> Paul
>
> On 21 July 2015 at 14:53, Mikael Morin <mikael.morin@sfr.fr> wrote:
>> Hello,
>>
>> The fix for PR61831 committed recently [1] introduced/uncovered a NULLL
>> pointer dereference with iso_varying_string, because a generic symbol (which
>> has a NULL result) is used as procedure symbol, instead of the specific one.
>> Fixed by using esym if it's available.
>>
>> Regression-tested on x86_64-linux. OK for trunk?
>>
>> Mikael
>>
>> [1]: https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01389.html
>>
>
Hello,

I would like to backport the patch.
As the bug was discovered with the patch [1] above, the test 
generic_30.f90 works on the branches, which don't have that patch.
Meanwhile, I have managed to find a test generic_31.f90 that exhibits a 
wrong code already on the branch, which justifies the backport.

Regression tested on the 5 branch, OK for 5 and 4.9?

Mikael



[-- Attachment #2: pr66929_1_backport.CL --]
[-- Type: text/plain, Size: 295 bytes --]

2015-07-25  Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/66929
	* trans-array.c (gfc_get_proc_ifc_for_expr): Use esym as procedure
	symbol if available.

2015-07-25  Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/66929
	* gfortran.dg/generic_30.f90: New.
	* gfortran.dg/generic_31.f90: New.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: pr66929_1.diff --]
[-- Type: text/x-patch; name="pr66929_1.diff", Size: 619 bytes --]

Index: trans-array.c
===================================================================
--- trans-array.c	(révision 225979)
+++ trans-array.c	(copie de travail)
@@ -9166,7 +9166,11 @@ gfc_get_proc_ifc_for_expr (gfc_expr *procedure_ref
     return NULL;
 
   /* Normal procedure case.  */
-  sym = procedure_ref->symtree->n.sym;
+  if (procedure_ref->expr_type == EXPR_FUNCTION
+      && procedure_ref->value.function.esym)
+    sym = procedure_ref->value.function.esym;
+  else
+    sym = procedure_ref->symtree->n.sym;
 
   /* Typebound procedure case.  */
   for (ref = procedure_ref->ref; ref; ref = ref->next)


[-- Attachment #4: generic_30.f90 --]
[-- Type: text/x-fortran, Size: 1360 bytes --]

! { dg-do compile }
!
! PR fortran/66929
! Generic procedures as actual argument used to lead to
! a NULL pointer dereference in gfc_get_proc_ifc_for_expr
! because the generic symbol was used as procedure symbol,
! instead of the specific one.

module iso_varying_string
  type, public :: varying_string
     character(LEN=1), dimension(:), allocatable :: chars
  end type varying_string
  interface operator(/=)
     module procedure op_ne_VS_CH
  end interface operator (/=)
  interface trim
     module procedure trim_
  end interface
contains
  elemental function op_ne_VS_CH (string_a, string_b) result (op_ne)
    type(varying_string), intent(in) :: string_a
    character(LEN=*), intent(in)     :: string_b
    logical                          :: op_ne
    op_ne = .true.
  end function op_ne_VS_CH
  elemental function trim_ (string) result (trim_string)
    type(varying_string), intent(in) :: string
    type(varying_string)             :: trim_string
    trim_string = varying_string(["t", "r", "i", "m", "m", "e", "d"])
  end function trim_
end module iso_varying_string
module syntax_rules
  use iso_varying_string, string_t => varying_string
contains
  subroutine set_rule_type_and_key
    type(string_t) :: key
    if (trim (key) /= "") then
      print *, "non-empty"
    end if
  end subroutine set_rule_type_and_key
end module syntax_rules


[-- Attachment #5: generic_31.f90 --]
[-- Type: text/x-fortran, Size: 794 bytes --]

! { dg-do run }
!
! PR fortran/66929
! Check that the specific FIRST symbol is used for the call to FOO,
! so that the J argument is not assumed to be present

module m
  interface foo
    module procedure first
  end interface foo
contains
  elemental function bar(j) result(r)
    integer, intent(in), optional :: j
    integer :: r, s(2)
    ! We used to have NULL dereference here, in case of a missing J argument
    s = foo(j, [3, 7])
    r = sum(s)
  end function bar
  elemental function first(i, j) result(r)
    integer, intent(in), optional :: i
    integer, intent(in) :: j
    integer :: r
    if (present(i)) then
      r = i
    else
      r = -5
    end if
  end function first
end module m
program p
  use m
  integer :: i
  i = bar()
  if (i /= -10) call abort
end program p

*ping* Re: [Patch, fortran] PR66929 fix iso_varying_string ICE

[Mikael Morin]

Le 25/07/2015 20:33, Mikael Morin a écrit :
> Le 21/07/2015 23:10, Paul Richard Thomas a écrit :
>> On 21 July 2015 at 14:53, Mikael Morin <mikael.morin@sfr.fr> wrote:
>>> Hello,
>>>
>>> The fix for PR61831 committed recently [1] introduced/uncovered a NULLL
>>> pointer dereference with iso_varying_string, because a generic symbol
>>> (which
>>> has a NULL result) is used as procedure symbol, instead of the
>>> specific one.
>>> Fixed by using esym if it's available.
>>>
>>> Regression-tested on x86_64-linux. OK for trunk?
>>>
>>> Mikael
>>>
>>> [1]: https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01389.html
>>>
>>
> Hello,
>
> I would like to backport the patch.
> As the bug was discovered with the patch [1] above, the test
> generic_30.f90 works on the branches, which don't have that patch.
> Meanwhile, I have managed to find a test generic_31.f90 that exhibits a
> wrong code already on the branch, which justifies the backport.
>
> Regression tested on the 5 branch, OK for 5 and 4.9?
>

Ping: https://gcc.gnu.org/ml/gcc-patches/2015-07/msg02160.html

Re: *ping* Re: [Patch, fortran] PR66929 fix iso_varying_string ICE

[Paul Richard Thomas]

Hi Mikael,

This is fine for backporting.

Thanks for doing this.

Paul

On 6 August 2015 at 12:09, Mikael Morin <mikael.morin@sfr.fr> wrote:
> Le 25/07/2015 20:33, Mikael Morin a écrit :
>>
>> Le 21/07/2015 23:10, Paul Richard Thomas a écrit :
>>>
>>> On 21 July 2015 at 14:53, Mikael Morin <mikael.morin@sfr.fr> wrote:
>>>>
>>>> Hello,
>>>>
>>>> The fix for PR61831 committed recently [1] introduced/uncovered a NULLL
>>>> pointer dereference with iso_varying_string, because a generic symbol
>>>> (which
>>>> has a NULL result) is used as procedure symbol, instead of the
>>>> specific one.
>>>> Fixed by using esym if it's available.
>>>>
>>>> Regression-tested on x86_64-linux. OK for trunk?
>>>>
>>>> Mikael
>>>>
>>>> [1]: https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01389.html
>>>>
>>>
>> Hello,
>>
>> I would like to backport the patch.
>> As the bug was discovered with the patch [1] above, the test
>> generic_30.f90 works on the branches, which don't have that patch.
>> Meanwhile, I have managed to find a test generic_31.f90 that exhibits a
>> wrong code already on the branch, which justifies the backport.
>>
>> Regression tested on the 5 branch, OK for 5 and 4.9?
>>
>
> Ping: https://gcc.gnu.org/ml/gcc-patches/2015-07/msg02160.html



-- 
Outside of a dog, a book is a man's best friend. Inside of a dog it's
too dark to read.

Groucho Marx

[fortran, committed] Forward port test generic_31.f90 from the 5 branch

[Mikael Morin]

[-- Attachment #1: Type: text/plain, Size: 141 bytes --]

Hello,

I have forward-ported the test that justified backport of the pr66929 
patch on the 5 branch:
https://gcc.gnu.org/r227010

Mikael




[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: r227010.diff --]
[-- Type: text/x-patch; name="r227010.diff", Size: 1485 bytes --]

Index: gcc/testsuite/gfortran.dg/generic_31.f90
===================================================================
--- gcc/testsuite/gfortran.dg/generic_31.f90	(révision 0)
+++ gcc/testsuite/gfortran.dg/generic_31.f90	(révision 227010)
@@ -0,0 +1,35 @@
+! { dg-do run }
+!
+! PR fortran/66929
+! Check that the specific FIRST symbol is used for the call to FOO,
+! so that the J argument is not assumed to be present
+
+module m
+  interface foo
+    module procedure first
+  end interface foo
+contains
+  elemental function bar(j) result(r)
+    integer, intent(in), optional :: j
+    integer :: r, s(2)
+    ! We used to have NULL dereference here, in case of a missing J argument
+    s = foo(j, [3, 7])
+    r = sum(s)
+  end function bar
+  elemental function first(i, j) result(r)
+    integer, intent(in), optional :: i
+    integer, intent(in) :: j
+    integer :: r
+    if (present(i)) then
+      r = i
+    else
+      r = -5
+    end if
+  end function first
+end module m
+program p
+  use m
+  integer :: i
+  i = bar()
+  if (i /= -10) call abort
+end program p
Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog	(révision 227009)
+++ gcc/testsuite/ChangeLog	(révision 227010)
@@ -1,3 +1,8 @@
+2015-08-19  Mikael Morin  <mikael@gcc.gnu.org>
+
+	PR fortran/66929
+	* gfortran.dg/generic_31.f90: New.
+
 2015-08-19  Marek Polacek  <polacek@redhat.com>
 
 	PR middle-end/67133