[Patch, fortran] PR81758 - [7/8 Regression] [OOP] Broken vtab

[Patch, fortran] PR81758 - [7/8 Regression] [OOP] Broken vtab

[Paul Richard Thomas]

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

Dear All,

Thanks to Dimitry Liakh for both reporting the problem and doing a lot
of the diagnostic work. Once the offending line in a very complicated
code was located, the fix was trivial. Generating a reduced testcase
took rather longer :-)

The comment in the testcase tells the story. The fix is a one-liner
that follows immediately from the explanation.

Bootstrapped and regtested on FC23/x86_64 - OK for trunk and 7-branch.

Cheers

Paul

2017-10-26  Paul Thomas  <pault@gcc.gnu.org>

    PR fortran/81758
    * trans-expr.c (trans_class_vptr_len_assignment): 'vptr_expr'
    must only be set if the right hand side expression is of class
    type.

2017-10-26  Paul Thomas  <pault@gcc.gnu.org>

    PR fortran/81758
    * gfortran.dg/class_63.f90: New test.

[-- Attachment #2: submit.diff --]
[-- Type: text/plain, Size: 3828 bytes --]

Index: gcc/fortran/trans-expr.c
===================================================================
*** gcc/fortran/trans-expr.c	(revision 253976)
--- gcc/fortran/trans-expr.c	(working copy)
*************** trans_class_vptr_len_assignment (stmtblo
*** 8051,8057 ****
      {
        /* Get the vptr from the rhs expression only, when it is variable.
  	 Functions are expected to be assigned to a temporary beforehand.  */
!       vptr_expr = re->expr_type == EXPR_VARIABLE
  	  ? gfc_find_and_cut_at_last_class_ref (re)
  	  : NULL;
        if (vptr_expr != NULL && vptr_expr->ts.type == BT_CLASS)
--- 8051,8057 ----
      {
        /* Get the vptr from the rhs expression only, when it is variable.
  	 Functions are expected to be assigned to a temporary beforehand.  */
!       vptr_expr = (re->expr_type == EXPR_VARIABLE && re->ts.type == BT_CLASS)
  	  ? gfc_find_and_cut_at_last_class_ref (re)
  	  : NULL;
        if (vptr_expr != NULL && vptr_expr->ts.type == BT_CLASS)
Index: gcc/testsuite/gfortran.dg/class_63.f90
===================================================================
*** gcc/testsuite/gfortran.dg/class_63.f90	(nonexistent)
--- gcc/testsuite/gfortran.dg/class_63.f90	(working copy)
***************
*** 0 ****
--- 1,80 ----
+ ! { dg-do run }
+ !
+ ! Tests the fix for PR81758, in which the vpointer for 'ptr' in
+ ! function 'pointer_value' would be set to the vtable of the component
+ ! 'container' rather than that of the component 'vec_elem'. In this test
+ ! case it is ensured that there is a single typebound procedure for both
+ ! types, so that different values are returned. In the original problem
+ ! completely different procedures were involved so that a segfault resulted.
+ !
+ ! Reduced from the original code of Dimitry Liakh  <liakhdi@ornl.gov> by
+ !                                   Paul Thomas  <pault@gcc.gnu.org>
+ !
+ module types
+   type, public:: gfc_container_t
+   contains
+     procedure, public:: get_value => ContTypeGetValue
+   end type gfc_container_t
+ 
+   !Element of a container:
+   type, public:: gfc_cont_elem_t
+     integer :: value_p
+   contains
+     procedure, public:: get_value => ContElemGetValue
+   end type gfc_cont_elem_t
+ 
+   !Vector element:
+   type, extends(gfc_cont_elem_t), public:: vector_elem_t
+   end type vector_elem_t
+ 
+   !Vector:
+   type, extends(gfc_container_t), public:: vector_t
+     type(vector_elem_t), allocatable, private :: vec_elem
+   end type vector_t
+ 
+   type, public :: vector_iter_t
+     class(vector_t), pointer, private :: container => NULL()
+   contains
+     procedure, public:: get_vector_value => vector_Value
+     procedure, public:: get_pointer_value => pointer_value
+   end type
+ 
+ contains
+   integer function ContElemGetValue (this)
+     class(gfc_cont_elem_t) :: this
+     ContElemGetValue = this%value_p
+   end function
+ 
+   integer function ContTypeGetValue (this)
+     class(gfc_container_t) :: this
+     ContTypeGetValue = 0
+   end function
+ 
+   integer function vector_Value (this)
+     class(vector_iter_t) :: this
+     vector_value = this%container%vec_elem%get_value()
+   end function
+ 
+   integer function pointer_value (this)
+     class(vector_iter_t), target :: this
+     class(gfc_cont_elem_t), pointer :: ptr
+     ptr => this%container%vec_elem
+     pointer_value = ptr%get_value()
+   end function
+ 
+   subroutine factory (arg)
+     class (vector_iter_t), pointer :: arg
+     allocate (vector_iter_t :: arg)
+     allocate (vector_t :: arg%container)
+     allocate (arg%container%vec_elem)
+     arg%container%vec_elem%value_p = 99
+   end subroutine
+ end module
+ 
+   use types
+   class (vector_iter_t), pointer :: x
+ 
+   call factory (x)
+   if (x%get_vector_value() .ne. 99) call abort
+   if (x%get_pointer_value() .ne. 99) call abort
+ end

Re: [Patch, fortran] PR81758 - [7/8 Regression] [OOP] Broken vtab

[Andre Vehreschild]

Hi Paul,

Without having tested the patch, it looks reasonable to me. So ok from my side.

- Andre

Am 26. Oktober 2017 21:12:45 MESZ schrieb Paul Richard Thomas <paul.richard.thomas@gmail.com>:
>Dear All,
>
>Thanks to Dimitry Liakh for both reporting the problem and doing a lot
>of the diagnostic work. Once the offending line in a very complicated
>code was located, the fix was trivial. Generating a reduced testcase
>took rather longer :-)
>
>The comment in the testcase tells the story. The fix is a one-liner
>that follows immediately from the explanation.
>
>Bootstrapped and regtested on FC23/x86_64 - OK for trunk and 7-branch.
>
>Cheers
>
>Paul
>
>2017-10-26  Paul Thomas  <pault@gcc.gnu.org>
>
>    PR fortran/81758
>    * trans-expr.c (trans_class_vptr_len_assignment): 'vptr_expr'
>    must only be set if the right hand side expression is of class
>    type.
>
>2017-10-26  Paul Thomas  <pault@gcc.gnu.org>
>
>    PR fortran/81758
>    * gfortran.dg/class_63.f90: New test.

-- 
Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen
Tel.: +49 241 929 10 18 * vehre@gmx.de

Re: [Patch, fortran] PR81758 - [7/8 Regression] [OOP] Broken vtab

[Jerry DeLisle]

On 10/26/2017 12:20 PM, Andre Vehreschild wrote:
> Hi Paul,
> 
> Without having tested the patch, it looks reasonable to me. So ok from my side.
> 
> - Andre
> 

Seconded, thanks.

Jerry

Re: [Patch, fortran] PR81758 - [7/8 Regression] [OOP] Broken vtab

[Paul Richard Thomas]

Thanks Andre and Jerry.

Committed to 8- and 7-branches as revisions 254195 & 6.

I must pay tribute to Dmitry's help with debugging this. This was at
least 50% of the work in sorting out what was a rather well hidden
gotcha. Once the line that exposed the bug was identified, the fix was
verging on 'obvious'. The testcase took a little longer though :-)

Cheers

Paul




On 27 October 2017 at 17:01, Jerry DeLisle <jvdelisle@charter.net> wrote:
> On 10/26/2017 12:20 PM, Andre Vehreschild wrote:
>> Hi Paul,
>>
>> Without having tested the patch, it looks reasonable to me. So ok from my side.
>>
>> - Andre
>>
>
> Seconded, thanks.
>
> Jerry



-- 
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein