| Message ID | 2cd2f3b9-3b67-dc5f-de37-94c9c5002e53@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fortran: Fix gfc_maybe_dereference_var [PR104430] | expand |
Hi Tobias, Am 07.03.22 um 15:01 schrieb Tobias Burnus: > The problem is that inside the main program, > y = f(z) > where the the result of z is > type(t) :: z(size(x%a)) > and 'x' is a dummy argument. > > 'x' looses the attr.dummy in gfc_add_interface_mapping > and this leads to an additional indirect ref in > gfc_maybe_dereference_var - but after the first indirect > ref, TREE_TYPE is alread a RECORD_TYPE ... > > The simple fix is to simply punt when the argument > is not a pointer or reference. > > This patch additionally fixes a comment in > conv_parent_component_references. LGTM. Regarding the commit message, there should be a period at the end of > (gfc_maybe_dereference_var): Avoid derefereing a nonpointer I think there are other PRs which profit from this fix. Can you please have a look at PR99585, and in particular the link in comment#0? ;-) > Those parts are obvious. The only potentially risky > change is the comparison change from a name-wise comparison > to a pointer-based comparison. I think it is fine and the > testsuite did not find any issue, but you prefer, I can > also exclude it. I was thinking about this, too. But your change will prevent running into trouble in the (unlikely?) case c being a NULL pointer. > OK for mainline? How about GCC 10/11 backports? > (I think leaving out the last change, it should be very safe to do.) OK, as this is both an ICE-on-valid and a regression. Thanks for the patch! Harald > Tobias > ----------------- > Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, > 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: > Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; > Registergericht München, HRB 106955
Hi Harald, On 07.03.22 20:58, Harald Anlauf wrote: > I think there are other PRs which profit from this fix. > Can you please have a look at PR99585, and in particular > the link in comment#0? ;-) Good pointer – the testcase looks nearly identical and it is indeed fixed. I included it in addition in the same testcase file. (See attached patch for the commit, .) Thanks, Tobias PS: Can I make you review my two pending patches? (NULL and SIZEOF) ;-) PPS: I lost a bit track working on other things – are there patches pending review? PPPS: I think someone still has to deal with the approved and pending patches by José ... ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
Hi Tobias, Am 08.03.22 um 21:19 schrieb Tobias Burnus: > PS: Can I make you review my two pending patches? (NULL and SIZEOF) ;-) I just approved the former one, but rather hope that Paul or Mikael or somebody else would jump in on the other one. > PPS: I lost a bit track working on other things – are there patches > pending review? > > PPPS: I think someone still has to deal with the approved and pending > patches by José ... What did prevent them getting processed after approval? Cheers, Harald
Fortran: Fix gfc_maybe_dereference_var [PR104430] PR fortran/104430 gcc/fortran/ChangeLog: * trans-expr.cc (conv_parent_component_references): Fix comment; simplify comparison. (gfc_maybe_dereference_var): Avoid derefereing a nonpointer gcc/testsuite/ChangeLog: * gfortran.dg/class_result_10.f90: New test. gcc/fortran/trans-expr.cc | 6 ++++-- gcc/testsuite/gfortran.dg/class_result_10.f90 | 25 +++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index eb6a78c3a62..e441952818b 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -2805,9 +2805,9 @@ conv_parent_component_references (gfc_se * se, gfc_ref * ref) dt = ref->u.c.sym; c = ref->u.c.component; - /* Return if the component is in the parent type. */ + /* Return if the component is in this type, i.e. not in the parent type. */ for (cmp = dt->components; cmp; cmp = cmp->next) - if (strcmp (c->name, cmp->name) == 0) + if (c == cmp) return; /* Build a gfc_ref to recursively call gfc_conv_component_ref. */ @@ -2867,6 +2867,8 @@ tree gfc_maybe_dereference_var (gfc_symbol *sym, tree var, bool descriptor_only_p, bool is_classarray) { + if (!POINTER_TYPE_P (TREE_TYPE (var))) + return var; if (is_CFI_desc (sym, NULL)) return build_fold_indirect_ref_loc (input_location, var); diff --git a/gcc/testsuite/gfortran.dg/class_result_10.f90 b/gcc/testsuite/gfortran.dg/class_result_10.f90 new file mode 100644 index 00000000000..acd9a73ebda --- /dev/null +++ b/gcc/testsuite/gfortran.dg/class_result_10.f90 @@ -0,0 +1,25 @@ +! { dg-do run} +! +! PR fortran/104430 +! +! Contributed by G. Steinmetz + +module m + type t + integer :: a + end type +contains + function f(x) result(z) + class(t) :: x(:) + type(t) :: z(size(x%a)) + z%a = 42 + end +end +program p + use m + class(t), allocatable :: y(:), z(:) + allocate (y(32)) + z = f(y) + if (size(z) /= 32) stop 1 + if (any (z%a /= 42)) stop 2 +end