[Patch, Fortran, OOP] PR 64230: [4.9/5 Regression] Invalid memory reference in a compiler-generated finalizer for allocatable component

[Patch, Fortran, OOP] PR 64230: [4.9/5 Regression] Invalid memory reference in a compiler-generated finalizer for allocatable component

[Janus Weil]

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

Hi,

this is a second patch dealing with finalization-related regressions,
after the one I submitted yesterday
(https://gcc.gnu.org/ml/fortran/2015-01/msg00109.html), which btw is
also still waiting for review ...

This patch fixes an invalid memory reference inside the finalizer
routine (at runtime), which apparently was caused by dereferencing a
pointer without checking if it's NULL. I simply insert a call to
ASSOCIATED.

I also rename two different runtime variables, which were both called
'ptr', to 'ptr1' and 'ptr2', just to make it easier to distinguish
them in the dump.

I also have the feeling the a lot of what is being done in
generate_finalization_wrapper and finalize_component (including my
changes) is a bit laborious. Some helper functions might be useful to
make all that code generation a bit more readable and less verbose. I
may attack this in a follow-up patch.

This one regtests cleanly on x86_64-unknown-linux-gnu. Ok for trunk and 4.9?

Cheers,
Janus



2015-01-19  Janus Weil  <janus@gcc.gnu.org>

    PR fortran/64230
    * class.c (finalize_component): New argument 'sub_ns'. Insert code to
    check if 'expr' is associated.
    (generate_finalization_wrapper): Rename 'ptr' symbols to 'ptr1' and
    'ptr2'. Pass 'sub_ns' to finalize_component.

2015-01-19  Janus Weil  <janus@gcc.gnu.org>

    PR fortran/64230
    * gfortran.dg/class_allocate_18.f90: Extended.

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

Index: gcc/fortran/class.c
===================================================================
--- gcc/fortran/class.c	(Revision 219840)
+++ gcc/fortran/class.c	(Arbeitskopie)
@@ -881,7 +881,8 @@ comp_is_finalizable (gfc_component *comp)
 
 static void
 finalize_component (gfc_expr *expr, gfc_symbol *derived, gfc_component *comp,
-		    gfc_symbol *stat, gfc_symbol *fini_coarray, gfc_code **code)
+		    gfc_symbol *stat, gfc_symbol *fini_coarray, gfc_code **code,
+		    gfc_namespace *sub_ns)
 {
   gfc_expr *e;
   gfc_ref *ref;
@@ -950,15 +951,32 @@ finalize_component (gfc_expr *expr, gfc_symbol *de
       dealloc->ext.alloc.list->expr = e;
       dealloc->expr1 = gfc_lval_expr_from_sym (stat);
 
+      gfc_code *cond = gfc_get_code (EXEC_IF);
+      cond->block = gfc_get_code (EXEC_IF);
+      cond->block->expr1 = gfc_get_expr ();
+      cond->block->expr1->expr_type = EXPR_FUNCTION;
+      gfc_get_sym_tree ("associated", sub_ns, &cond->block->expr1->symtree, false);
+      cond->block->expr1->symtree->n.sym->attr.flavor = FL_PROCEDURE;
+      cond->block->expr1->symtree->n.sym->attr.intrinsic = 1;
+      cond->block->expr1->symtree->n.sym->result = cond->block->expr1->symtree->n.sym;
+      gfc_commit_symbol (cond->block->expr1->symtree->n.sym);
+      cond->block->expr1->ts.type = BT_LOGICAL;
+      cond->block->expr1->ts.kind = gfc_default_logical_kind;
+      cond->block->expr1->value.function.isym = gfc_intrinsic_function_by_id (GFC_ISYM_ASSOCIATED);
+      cond->block->expr1->value.function.actual = gfc_get_actual_arglist ();
+      cond->block->expr1->value.function.actual->expr = gfc_copy_expr (expr);
+      cond->block->expr1->value.function.actual->next = gfc_get_actual_arglist ();
+      cond->block->next = dealloc;
+
       if (block)
-	block->next = dealloc;
+	block->next = cond;
       else if (*code)
 	{
-	  (*code)->next = dealloc;
+	  (*code)->next = cond;
 	  (*code) = (*code)->next;
 	}
       else
-	(*code) = dealloc;
+	(*code) = cond;
     }
   else if (comp->ts.type == BT_DERIVED
 	    && comp->ts.u.derived->f2k_derived
@@ -994,7 +1012,8 @@ finalize_component (gfc_expr *expr, gfc_symbol *de
       gfc_component *c;
 
       for (c = comp->ts.u.derived->components; c; c = c->next)
-	finalize_component (e, comp->ts.u.derived, c, stat, fini_coarray, code);
+	finalize_component (e, comp->ts.u.derived, c, stat, fini_coarray, code,
+			    sub_ns);
       gfc_free_expr (e);
     }
 }
@@ -1927,7 +1946,7 @@ generate_finalization_wrapper (gfc_symbol *derived
     {
       gfc_finalizer *fini, *fini_elem = NULL;
 
-      gfc_get_symbol ("ptr", sub_ns, &ptr);
+      gfc_get_symbol ("ptr1", sub_ns, &ptr);
       ptr->ts.type = BT_DERIVED;
       ptr->ts.u.derived = derived;
       ptr->attr.flavor = FL_VARIABLE;
@@ -2051,7 +2070,7 @@ generate_finalization_wrapper (gfc_symbol *derived
 
       if (!ptr)
 	{
-	  gfc_get_symbol ("ptr", sub_ns, &ptr);
+	  gfc_get_symbol ("ptr2", sub_ns, &ptr);
 	  ptr->ts.type = BT_DERIVED;
 	  ptr->ts.u.derived = derived;
 	  ptr->attr.flavor = FL_VARIABLE;
@@ -2100,7 +2119,7 @@ generate_finalization_wrapper (gfc_symbol *derived
 	    continue;
 
 	  finalize_component (gfc_lval_expr_from_sym (ptr), derived, comp,
-			      stat, fini_coarray, &block);
+			      stat, fini_coarray, &block, sub_ns);
 	  if (!last_code->block->next)
 	    last_code->block->next = block;
 	}

[-- Attachment #3: class_allocate_18.f90 --]
[-- Type: text/x-fortran, Size: 509 bytes --]

! { dg-do run }
! { dg-options "-fsanitize=undefined" }
!
! PR 64230: [4.9/5 Regression] Invalid memory reference in a compiler-generated finalizer for allocatable component
!
! Contributed by Mat Cross <mathewc@nag.co.uk>

Program main
  Implicit None
  Type :: t1
  End Type
  Type, Extends (t1) :: t2
    Integer, Allocatable :: i
  End Type
  Type, Extends (t2) :: t3
    Integer, Allocatable :: j
  End Type
  Class (t1), Allocatable :: t
  Allocate (t3 :: t)
  print *,"allocated!"
  Deallocate (t)
End

Re: [Patch, Fortran, OOP] PR 64230: [4.9/5 Regression] Invalid memory reference in a compiler-generated finalizer for allocatable component

[Janus Weil]

ping!



2015-01-19 15:41 GMT+01:00 Janus Weil <janus@gcc.gnu.org>:
> Hi,
>
> this is a second patch dealing with finalization-related regressions,
> after the one I submitted yesterday
> (https://gcc.gnu.org/ml/fortran/2015-01/msg00109.html), which btw is
> also still waiting for review ...
>
> This patch fixes an invalid memory reference inside the finalizer
> routine (at runtime), which apparently was caused by dereferencing a
> pointer without checking if it's NULL. I simply insert a call to
> ASSOCIATED.
>
> I also rename two different runtime variables, which were both called
> 'ptr', to 'ptr1' and 'ptr2', just to make it easier to distinguish
> them in the dump.
>
> I also have the feeling the a lot of what is being done in
> generate_finalization_wrapper and finalize_component (including my
> changes) is a bit laborious. Some helper functions might be useful to
> make all that code generation a bit more readable and less verbose. I
> may attack this in a follow-up patch.
>
> This one regtests cleanly on x86_64-unknown-linux-gnu. Ok for trunk and 4.9?
>
> Cheers,
> Janus
>
>
>
> 2015-01-19  Janus Weil  <janus@gcc.gnu.org>
>
>     PR fortran/64230
>     * class.c (finalize_component): New argument 'sub_ns'. Insert code to
>     check if 'expr' is associated.
>     (generate_finalization_wrapper): Rename 'ptr' symbols to 'ptr1' and
>     'ptr2'. Pass 'sub_ns' to finalize_component.
>
> 2015-01-19  Janus Weil  <janus@gcc.gnu.org>
>
>     PR fortran/64230
>     * gfortran.dg/class_allocate_18.f90: Extended.

Re: [Patch, Fortran, OOP] PR 64230: [4.9/5 Regression] Invalid memory reference in a compiler-generated finalizer for allocatable component

[Tobias Burnus]

Janus Weil wrote:
> this is a second patch dealing with finalization-related regressions,
> [...]
> This patch fixes an invalid memory reference inside the finalizer
> routine (at runtime), which apparently was caused by dereferencing a
> pointer without checking if it's NULL. I simply insert a call to
> ASSOCIATED.
> [...]
>
> This one regtests cleanly on x86_64-unknown-linux-gnu. Ok for trunk and 4.9?

OK. Thanks for the patch!

Tobias

> 2015-01-19  Janus Weil  <janus@gcc.gnu.org>
>
>      PR fortran/64230
>      * class.c (finalize_component): New argument 'sub_ns'. Insert code to
>      check if 'expr' is associated.
>      (generate_finalization_wrapper): Rename 'ptr' symbols to 'ptr1' and
>      'ptr2'. Pass 'sub_ns' to finalize_component.
>
> 2015-01-19  Janus Weil  <janus@gcc.gnu.org>
>
>      PR fortran/64230
>      * gfortran.dg/class_allocate_18.f90: Extended.

Re: [Patch, Fortran, OOP] PR 64230: [4.9/5 Regression] Invalid memory reference in a compiler-generated finalizer for allocatable component

[Janus Weil]

2015-01-24 18:18 GMT+01:00 Tobias Burnus <burnus@net-b.de>:
>> this is a second patch dealing with finalization-related regressions,
>> [...]
>> This patch fixes an invalid memory reference inside the finalizer
>> routine (at runtime), which apparently was caused by dereferencing a
>> pointer without checking if it's NULL. I simply insert a call to
>> ASSOCIATED.
>> [...]
>>
>> This one regtests cleanly on x86_64-unknown-linux-gnu. Ok for trunk and
>> 4.9?
>
>
> OK. Thanks for the patch!

Thanks for the review, Tobias. Committed to trunk as r220125. Will
backport to 4.9 soon.

Cheers,
Janus

Re: [Patch, Fortran, OOP] PR 64230: [4.9/5 Regression] Invalid memory reference in a compiler-generated finalizer for allocatable component

[Paul Richard Thomas]

Hi Janus,

The testcase has a dependence on libubsan.so, which my LD_LIBRARY_PATH
does not seem to be able to resolve. It therefore fails in the
regression test. When I point to ~/lib64, I get the message:

/svn/trunk/gcc/testsuite/gfortran.dg/class_allocate_18.f90:8: runtime
error: signed integer overflow: 214358001592 * 214358001560 cannot be
represented in type 'integer(kind=8)'

Cheers

Paul

On 26 January 2015 at 15:57, Janus Weil <janus@gcc.gnu.org> wrote:
> 2015-01-24 18:18 GMT+01:00 Tobias Burnus <burnus@net-b.de>:
>>> this is a second patch dealing with finalization-related regressions,
>>> [...]
>>> This patch fixes an invalid memory reference inside the finalizer
>>> routine (at runtime), which apparently was caused by dereferencing a
>>> pointer without checking if it's NULL. I simply insert a call to
>>> ASSOCIATED.
>>> [...]
>>>
>>> This one regtests cleanly on x86_64-unknown-linux-gnu. Ok for trunk and
>>> 4.9?
>>
>>
>> OK. Thanks for the patch!
>
> Thanks for the review, Tobias. Committed to trunk as r220125. Will
> backport to 4.9 soon.
>
> Cheers,
> Janus



-- 
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, OOP] PR 64230: [4.9/5 Regression] Invalid memory reference in a compiler-generated finalizer for allocatable component

[Janus Weil]

Hi Paul,

> The testcase has a dependence on libubsan.so, which my LD_LIBRARY_PATH
> does not seem to be able to resolve. It therefore fails in the
> regression test. When I point to ~/lib64, I get the message:
>
> /svn/trunk/gcc/testsuite/gfortran.dg/class_allocate_18.f90:8: runtime
> error: signed integer overflow: 214358001592 * 214358001560 cannot be
> represented in type 'integer(kind=8)'

of course I tested it but did not see any problem on my machine
(Ubuntu 14.10, x86_64-unknown-linux-gnu).

In fact class_allocate_18.f90 seems to be the only testcase in
gfortran.dg/ which uses any -fsanitize options, but there are plenty
in other sections (mostly c-c++-common/ubsan/ and g++.dg/ubsan/).

Could it be a problem with your local setup or a dejagnu issue that
only applears on certain platforms? I have no idea how to fix this. Do
you have any suggestion?

Cheers,
Janus



> On 26 January 2015 at 15:57, Janus Weil <janus@gcc.gnu.org> wrote:
>> 2015-01-24 18:18 GMT+01:00 Tobias Burnus <burnus@net-b.de>:
>>>> this is a second patch dealing with finalization-related regressions,
>>>> [...]
>>>> This patch fixes an invalid memory reference inside the finalizer
>>>> routine (at runtime), which apparently was caused by dereferencing a
>>>> pointer without checking if it's NULL. I simply insert a call to
>>>> ASSOCIATED.
>>>> [...]
>>>>
>>>> This one regtests cleanly on x86_64-unknown-linux-gnu. Ok for trunk and
>>>> 4.9?
>>>
>>>
>>> OK. Thanks for the patch!
>>
>> Thanks for the review, Tobias. Committed to trunk as r220125. Will
>> backport to 4.9 soon.
>>
>> Cheers,
>> Janus
>
>
>
> --
> 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, OOP] PR 64230: [4.9/5 Regression] Invalid memory reference in a compiler-generated finalizer for allocatable component

[Paul Richard Thomas]

Dear Janus,

I know somewhat less than nothing about such matters. I suggest that
you contact the authors of the testcases that use libubsan that you
mention.

Sorry

Paul

On 26 January 2015 at 22:20, Janus Weil <janus@gcc.gnu.org> wrote:
> Hi Paul,
>
>> The testcase has a dependence on libubsan.so, which my LD_LIBRARY_PATH
>> does not seem to be able to resolve. It therefore fails in the
>> regression test. When I point to ~/lib64, I get the message:
>>
>> /svn/trunk/gcc/testsuite/gfortran.dg/class_allocate_18.f90:8: runtime
>> error: signed integer overflow: 214358001592 * 214358001560 cannot be
>> represented in type 'integer(kind=8)'
>
> of course I tested it but did not see any problem on my machine
> (Ubuntu 14.10, x86_64-unknown-linux-gnu).
>
> In fact class_allocate_18.f90 seems to be the only testcase in
> gfortran.dg/ which uses any -fsanitize options, but there are plenty
> in other sections (mostly c-c++-common/ubsan/ and g++.dg/ubsan/).
>
> Could it be a problem with your local setup or a dejagnu issue that
> only applears on certain platforms? I have no idea how to fix this. Do
> you have any suggestion?
>
> Cheers,
> Janus
>
>
>
>> On 26 January 2015 at 15:57, Janus Weil <janus@gcc.gnu.org> wrote:
>>> 2015-01-24 18:18 GMT+01:00 Tobias Burnus <burnus@net-b.de>:
>>>>> this is a second patch dealing with finalization-related regressions,
>>>>> [...]
>>>>> This patch fixes an invalid memory reference inside the finalizer
>>>>> routine (at runtime), which apparently was caused by dereferencing a
>>>>> pointer without checking if it's NULL. I simply insert a call to
>>>>> ASSOCIATED.
>>>>> [...]
>>>>>
>>>>> This one regtests cleanly on x86_64-unknown-linux-gnu. Ok for trunk and
>>>>> 4.9?
>>>>
>>>>
>>>> OK. Thanks for the patch!
>>>
>>> Thanks for the review, Tobias. Committed to trunk as r220125. Will
>>> backport to 4.9 soon.
>>>
>>> Cheers,
>>> Janus
>>
>>
>>
>> --
>> Outside of a dog, a book is a man's best friend. Inside of a dog it's
>> too dark to read.
>>
>> Groucho Marx



-- 
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, OOP] PR 64230: [4.9/5 Regression] Invalid memory reference in a compiler-generated finalizer for allocatable component

[Andreas Schwab]

Janus Weil <janus@gcc.gnu.org> writes:

> 2015-01-19  Janus Weil  <janus@gcc.gnu.org>
>
>     PR fortran/64230
>     * gfortran.dg/class_allocate_18.f90: Extended.

FAIL: gfortran.dg/class_allocate_18.f90   -O0  (test for excess errors)
Excess errors:
/usr/ia64-suse-linux/bin/ld: cannot find -lubsan

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [Patch, Fortran, OOP] PR 64230: [4.9/5 Regression] Invalid memory reference in a compiler-generated finalizer for allocatable component

[Jakub Jelinek]

On Tue, Jan 27, 2015 at 10:24:47AM +0100, Andreas Schwab wrote:
> Janus Weil <janus@gcc.gnu.org> writes:
> 
> > 2015-01-19  Janus Weil  <janus@gcc.gnu.org>
> >
> >     PR fortran/64230
> >     * gfortran.dg/class_allocate_18.f90: Extended.
> 
> FAIL: gfortran.dg/class_allocate_18.f90   -O0  (test for excess errors)
> Excess errors:
> /usr/ia64-suse-linux/bin/ld: cannot find -lubsan

Yeah, if you want to add ubsan tests, you need to add gfortran.dg/ubsan/
directory and hack up ubsan.exp in there, from say gcc.dg/ubsan/ubsan.exp
and gfortran.dg/dg.exp.

	Jakub

Re: [Patch, Fortran, OOP] PR 64230: [4.9/5 Regression] Invalid memory reference in a compiler-generated finalizer for allocatable component

[Janus Weil]

> On Tue, Jan 27, 2015 at 10:24:47AM +0100, Andreas Schwab wrote:
>>
>> > 2015-01-19  Janus Weil  <janus@gcc.gnu.org>
>> >
>> >     PR fortran/64230
>> >     * gfortran.dg/class_allocate_18.f90: Extended.
>>
>> FAIL: gfortran.dg/class_allocate_18.f90   -O0  (test for excess errors)
>> Excess errors:
>> /usr/ia64-suse-linux/bin/ld: cannot find -lubsan

Sorry for the breakage, guys!


2015-01-27 10:30 GMT+01:00 Jakub Jelinek <jakub@redhat.com>:
> Yeah, if you want to add ubsan tests, you need to add gfortran.dg/ubsan/
> directory and hack up ubsan.exp in there

Thanks for the remark, I was suspecting something like that. However,
for this case it's not really worth the hassle. In fact the test case
does not really need the sanitizer and should also work without it. So
I'll just remove the -fsanitize option:

Index: gcc/testsuite/gfortran.dg/class_allocate_18.f90
===================================================================
--- gcc/testsuite/gfortran.dg/class_allocate_18.f90    (Revision 220180)
+++ gcc/testsuite/gfortran.dg/class_allocate_18.f90    (Arbeitskopie)
@@ -1,5 +1,4 @@
 ! { dg-do run }
-! { dg-options "-fsanitize=undefined" }
 !
 ! PR 64230: [4.9/5 Regression] Invalid memory reference in a
compiler-generated finalizer for allocatable component
 !


Cheers,
Janus

Re: [Patch, Fortran, OOP] PR 64230: [4.9/5 Regression] Invalid memory reference in a compiler-generated finalizer for allocatable component

[Jakub Jelinek]

On Tue, Jan 27, 2015 at 07:20:10PM +0100, Janus Weil wrote:
> 2015-01-27 10:30 GMT+01:00 Jakub Jelinek <jakub@redhat.com>:
> > Yeah, if you want to add ubsan tests, you need to add gfortran.dg/ubsan/
> > directory and hack up ubsan.exp in there
> 
> Thanks for the remark, I was suspecting something like that. However,
> for this case it's not really worth the hassle. In fact the test case
> does not really need the sanitizer and should also work without it. So
> I'll just remove the -fsanitize option:
> 
> Index: gcc/testsuite/gfortran.dg/class_allocate_18.f90
> ===================================================================
> --- gcc/testsuite/gfortran.dg/class_allocate_18.f90    (Revision 220180)
> +++ gcc/testsuite/gfortran.dg/class_allocate_18.f90    (Arbeitskopie)
> @@ -1,5 +1,4 @@
>  ! { dg-do run }
> -! { dg-options "-fsanitize=undefined" }
>  !
>  ! PR 64230: [4.9/5 Regression] Invalid memory reference in a
> compiler-generated finalizer for allocatable component
>  !

LGTM.

	Jakub

Re: [Patch, Fortran, OOP] PR 64230: [4.9/5 Regression] Invalid memory reference in a compiler-generated finalizer for allocatable component

[Janus Weil]

2015-01-27 19:23 GMT+01:00 Jakub Jelinek <jakub@redhat.com>:
> On Tue, Jan 27, 2015 at 07:20:10PM +0100, Janus Weil wrote:
>> 2015-01-27 10:30 GMT+01:00 Jakub Jelinek <jakub@redhat.com>:
>> > Yeah, if you want to add ubsan tests, you need to add gfortran.dg/ubsan/
>> > directory and hack up ubsan.exp in there
>>
>> Thanks for the remark, I was suspecting something like that. However,
>> for this case it's not really worth the hassle. In fact the test case
>> does not really need the sanitizer and should also work without it. So
>> I'll just remove the -fsanitize option:
>>
>> Index: gcc/testsuite/gfortran.dg/class_allocate_18.f90
>> ===================================================================
>> --- gcc/testsuite/gfortran.dg/class_allocate_18.f90    (Revision 220180)
>> +++ gcc/testsuite/gfortran.dg/class_allocate_18.f90    (Arbeitskopie)
>> @@ -1,5 +1,4 @@
>>  ! { dg-do run }
>> -! { dg-options "-fsanitize=undefined" }
>>  !
>>  ! PR 64230: [4.9/5 Regression] Invalid memory reference in a
>> compiler-generated finalizer for allocatable component
>>  !
>
> LGTM.

Good, committed as r220181. Since I had already backported the
original patch to 4.9 yesterday, I'll do the same there ...

Cheers,
Janus