Bug 110415 - (Re)allocation on assignment to allocatable polymorphic variable from allocatable polymorphic function result
Summary: (Re)allocation on assignment to allocatable polymorphic variable from allocat...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 13.1.1
: P3 normal
Target Milestone: 13.4
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks: Finalization
  Show dependency treegraph
 
Reported: 2023-06-26 15:47 UTC by Brad Richardson
Modified: 2024-05-22 18:52 UTC (History)
8 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2023-06-26 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Brad Richardson 2023-06-26 15:47:20 UTC
A reproducer and explanation can be found at: https://github.com/HPC-Bugs/reproducers/tree/main/compiler/Fortran/gfortran/polymorphic-function-assignment
Comment 1 kargls 2023-06-26 17:21:03 UTC
Confirmed.

Thanks, Brad.  I vaguely recall a similar bug report, perhaps, PR107489.

Here's the code in question

type, abstract :: p
end type

type, extends(p) :: c
end type

class(p), allocatable :: a

a = func()
contains
  function func() result(a)
    class(p), allocatable :: a

    a = c()
  end function
end
Comment 2 Matt Thompson 2023-10-30 20:05:54 UTC
I was about to file a bug, but I was pointed to search for bug by Brad Richardon and...yep, this is the one we hit. We found it in one of our base layers, yaFyaml:

https://github.com/Goddard-Fortran-Ecosystem/yaFyaml

but it's a paradigm we use in codes that use this as well, so...

Thus, +1 from me and I can confirm this is still a bug in 13.2.0. I haven't tried GNU 14, but I'm guessing since this is still at "NEW" it would be an issue there as well.
Comment 3 Tobias Burnus 2023-11-20 14:07:19 UTC
Andrew Jenner's submitted patch (gcc-patches@ only):
  https://gcc.gnu.org/pipermail/gcc-patches/2023-November/636671.html
and (fortran@ only):
  https://gcc.gnu.org/pipermail/fortran/2023-November/059928.html
(Replies should got to both lists ...)

* * *

Technical it is a regression caused by
 https://gcc.gnu.org/r13-6747-gd7caf313525a46f200d7f5db1ba893f853774aee
but before that commit there was no finalization.

Comparing the versions:
  GCC 7+8: ICE in build_function_decl
  GCC 10+11+12: memory leak in 'func'
  GCC 13+mainline: segfault at runtime  (at 'a = func()' in the main program).

* * *

I had analyzed the issue the elsewhere, let's copy it here for completeness and possibly to aid the patch review. (Note: The following was written before the patch was written and analyzed the current status.)

---<cut-----

The 'func' has the prototype 'struct __class_MAIN___P_a func ()', i.e. returns the class-wrapper directly - and that part looks okay.

However, the assignment somehow mixes everything up:

    D.4349 = a->_vptr;  // save old value of vptr
    D.4328 = func ();   // new value

    desc.0.data = (void * restrict) D.4328._data;
// As scalar, there is not really a problem, but an
//    desc.0.dtype.elem_len = D.4328->_vptr->size;
// is missing here.
    desc.0.span = (integer(kind=8)) desc.0.dtype.elem_len;

    if (__builtin_expect ((integer(kind=8)) (a->_data == 0B), 0, 42))
        a->_data = (struct p *) __builtin_malloc (MAX_EXPR <(unsigned long) a->_vptr->_size, 1>);
  // WRONG: That should use D.4328->_vptr->size!

    else
      {
        if (a->_vptr != D.4349)
          {
            __builtin_realloc ((void *) a->_data, a->_vptr->_size);

Likewise: a->_vptr should be D.4328->_vptr.

Alternatively, a->_vptr had to be updated before the 'if' block.
Comment 4 GCC Commits 2023-11-28 15:28:00 UTC
The master branch has been updated by Andrew Jenner <andrewjenner@gcc.gnu.org>:

https://gcc.gnu.org/g:b247e917ff13328298c1eecf8563b12edd7ade04

commit r14-5931-gb247e917ff13328298c1eecf8563b12edd7ade04
Author: Andrew Jenner <andrew@codesourcery.com>
Date:   Tue Nov 28 15:27:05 2023 +0000

    Fortran: fix reallocation on assignment of polymorphic variables [PR110415]
    
    This patch fixes two bugs related to polymorphic class assignment in the
    Fortran front-end. One (described in PR110415) is an issue with the malloc
    and realloc calls using the size from the old vptr rather than the new one.
    The other is caused by the return value from the realloc call being ignored.
    Testcases are added for these issues.
    
    2023-11-28  Andrew Jenner  <andrew@codesourcery.com>
    
    gcc/fortran/
            PR fortran/110415
            * trans-expr.cc (trans_class_vptr_len_assignment): Add
            from_vptrp parameter. Populate it. Don't check for DECL_P
            when deciding whether to create temporary.
            (trans_class_pointer_fcn, gfc_trans_pointer_assignment): Add
            NULL argument to trans_class_vptr_len_assignment calls.
            (trans_class_assignment): Get rhs_vptr from
            trans_class_vptr_len_assignment and use it for determining size
            for allocation/reallocation. Use return value from realloc.
    
    gcc/testsuite/
            PR fortran/110415
            * gfortran.dg/pr110415.f90: New test.
            * gfortran.dg/asan/pr110415-2.f90: New test.
            * gfortran.dg/asan/pr110415-3.f90: New test.
    
    Co-Authored-By: Tobias Burnus  <tobias@codesourcery.com>
Comment 5 Andrew Jenner 2023-11-28 15:39:48 UTC
Fixed on mainline and OG13. I will apply the commit to GCC 13 after the grace period.
Comment 6 anlauf 2024-05-21 19:16:01 UTC
(In reply to Andrew Jenner from comment #5)
> Fixed on mainline and OG13. I will apply the commit to GCC 13 after the
> grace period.

This never happened...  Are you still planning to do it?
Comment 7 GCC Commits 2024-05-22 18:49:45 UTC
The releases/gcc-13 branch has been updated by Harald Anlauf <anlauf@gcc.gnu.org>:

https://gcc.gnu.org/g:2ebf3af1f84d54fbda172eff105a8842c685d11d

commit r13-8793-g2ebf3af1f84d54fbda172eff105a8842c685d11d
Author: Andrew Jenner <andrew@codesourcery.com>
Date:   Tue Nov 28 15:27:05 2023 +0000

    Fortran: fix reallocation on assignment of polymorphic variables [PR110415]
    
    This patch fixes two bugs related to polymorphic class assignment in the
    Fortran front-end. One (described in PR110415) is an issue with the malloc
    and realloc calls using the size from the old vptr rather than the new one.
    The other is caused by the return value from the realloc call being ignored.
    Testcases are added for these issues.
    
    2023-11-28  Andrew Jenner  <andrew@codesourcery.com>
    
    gcc/fortran/
            PR fortran/110415
            * trans-expr.cc (trans_class_vptr_len_assignment): Add
            from_vptrp parameter. Populate it. Don't check for DECL_P
            when deciding whether to create temporary.
            (trans_class_pointer_fcn, gfc_trans_pointer_assignment): Add
            NULL argument to trans_class_vptr_len_assignment calls.
            (trans_class_assignment): Get rhs_vptr from
            trans_class_vptr_len_assignment and use it for determining size
            for allocation/reallocation. Use return value from realloc.
    
    gcc/testsuite/
            PR fortran/110415
            * gfortran.dg/pr110415.f90: New test.
            * gfortran.dg/asan/pr110415-2.f90: New test.
            * gfortran.dg/asan/pr110415-3.f90: New test.
    
    Co-Authored-By: Tobias Burnus  <tobias@codesourcery.com>
    (cherry picked from commit b247e917ff13328298c1eecf8563b12edd7ade04)
Comment 8 anlauf 2024-05-22 18:52:29 UTC
Backported to 13-branch after encouragement by Tobias off-list.