Hi Andrew,

This is OK by me.

I attach a slightly edited version of the patch itself in the hope that it
will make the code a bit clearer.

Thanks and welcome!

Paul


On Mon, 27 Nov 2023 at 17:35, Andrew Jenner <andrew@codesourcery.com> wrote:

> This is the second version of the patch - previous discussion at:
> https://gcc.gnu.org/pipermail/gcc-patches/2023-November/636671.html
>
> This patch adds the testcase from PR110415 and fixes the bug.
>
> The problem is that in a couple of places in trans_class_assignment in
> trans-expr.cc, we need to get the run-time size of the polymorphic
> object from the vtbl, but we are currently getting that vtbl from the
> lhs of the assignment rather than the rhs. This gives us the old value
> of the size but we need to pass the new size to __builtin_malloc and
> __builtin_realloc.
>
> I'm fixing this by adding a parameter to trans_class_vptr_len_assignment
> to retrieve the tree corresponding the vptr from the object on the rhs
> of the assignment, and then passing this where it is needed. In the case
> where trans_class_vptr_len_assignment returns NULL_TREE for the rhs vptr
> we use the lhs vptr as before.
>
> To get this to work I also needed to change the implementation of
> trans_class_vptr_len_assignment to create a temporary for the assignment
> in more circumstances. Currently, the "a = func()" assignment in MAIN__
> doesn't hit the "Create a temporary for complication expressions" case
> on line 9951 because "DECL_P (rse->expr)" is true - the expression has
> already been placed into a temporary. That means we don't hit the "if
> (temp_rhs ..." case on line 10038 and go on to get the vptr_expr from
> "gfc_lval_expr_from_sym (gfc_find_vtab (&re->ts))" on line 10057 which
> is the vtbl of the static type rather than the dynamic one from the rhs.
> So with this fix we create an extra temporary, but that should be
> optimised away in the middle-end so there should be no run-time effect.
>
> I'm not sure if this is the best way to fix this (the Fortran front-end
> is new territory for me) but I've verified that the testcase passes with
> this change, fails without it, and that the change does not introduce
> any FAILs when running the gfortran testcases on x86_64-pc-linux-gnu.
>
> After the previous submission, Tobias Burnus found a closely related
> problem and contributed testcases and a fix for it, which I have
> incorporated into this version of the patch. The problem in this case is
> with the __builtin_realloc call that is executed if one polymorphic
> variable is replaced by another. The return value of this call was being
> ignored rather than used to replace the pointer being reallocated.
>
> Is this OK for mainline, GCC 13 and OG13?
>
> Thanks,
>
> Andrew
>
> 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.