Re: [PATCH] Fortran: fix issues with class(*) assignment [PR114827]

[Harald Anlauf]

Hi Paul,

On 4/30/24 07:50, Paul Richard Thomas wrote:

Hi Harald,

This patch is verging on 'obvious', ..... once one sees it :-)

Yes, it's good for mainline and all active branches, when available.

thanks for your quick review.

I haven't committed it yet, because I forgot to check what happens with
a class(*) allocatable function result on the r.h.s. of the assignment.
One now gets an ICE with the testcase in your submission

  https://gcc.gnu.org/pipermail/fortran/2024-April/060426.html

on the simple scalar assignment

  y = bar ()

instead of wrong code.  Not very helpful.

I tried the following change on top of the submitted patch:

diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 4ba40bfdbd3..cacf3c0dda1 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -11995,7 +11996,11 @@ trans_class_assignment (stmtblock_t *block,
gfc_expr *lhs, gfc_expr *rhs,
       /* Take into account _len of unlimited polymorphic entities.  */
       if (UNLIMITED_POLY (rhs))
        {
-         tree len = trans_get_upoly_len (block, rhs);
+         tree len;
+         if (rhs->expr_type == EXPR_VARIABLE)
+           len = trans_get_upoly_len (block, rhs);
+         else
+           len = gfc_class_len_get (gfc_get_class_from_expr (tmp));
          len = fold_build2_loc (input_location, MAX_EXPR, size_type_node,
                                 fold_convert (size_type_node, len),
                                 size_one_node);

This avoids the ICE, but depending on details of bar() this leads
to different wrong code from before, and

  function bar() result(res)
    class(*), allocatable :: res
    res = sca
  end function bar

behaves differently from

  function bar()
    class(*), allocatable :: bar
    bar = sca
  end function bar

The minimal and sort of "safe" fix to avoid a new ICE while keeping
the fix for simple assignments is to replace in the above snippet

       if (UNLIMITED_POLY (rhs))

by

       if (UNLIMITED_POLY (rhs) && rhs->expr_type == EXPR_VARIABLE)

omit the other changes above, and defer a fix for assignment of
function results, as looking at the dump-tree suggests that this
will be a bigger piece of work.  (The .span looks suspicious all
over the place...)

The good thing is: a simple test with array-valued function results
did not immediately break the submitted patch...  ;-)

What do you think?

Thanks,
Harald

Thanks

Paul

PS The fall-out pr114874 is so peculiar that I am dropping everything to
find the source.


On Mon, 29 Apr 2024 at 19:39, Harald Anlauf <anl...@gmx.de> wrote:

Dear all,

the attached patch fixes issues with assignments of unlimited polymorphic
entities that were found with the help of valgrind or asan, see PR.
Looking
further into it, it turns out that allocation sizes as well as array spans
could be set incorrectly, leading to wrong results or heap corruption.

The fix is rather straightforward: take into the _len of unlimited
polymorphic entities when it is non-zero to get the correct allocation
sizes and array spans.

The patch has been tested by the reporter, see PR.

Regtested on x86_64-pc-linux-gnu.  OK for 15-mainline?

I would like to backport this to active branches where appropriate,
starting with 14 after it reopens after release.  Is this OK?

Thanks,
Harald