On Thu, Aug 01, 2024 at 09:03:39PM +0200, Mikael Morin wrote:
> Le 01/08/2024 à 12:00, Jakub Jelinek a écrit :
> > Hi!
> >
> > A static analyzer found what seems like a pasto in the PR45019 changes,
> > the second branch of || only accesses sym2 while the first one sym1 except
> > for this one spot.
> >
> > Not sure I'm able to construct a testcase for this though.
> >
> What is reported exactly by the static analyzer?
I'm not sure I'm allowed to cite it, it is some proprietary static analyzer
and I got that indirectly. But if I paraphrase it, the diagnostics was
a possible pasto. The static analyzer likely doesn't see that
sym1->attr.target implies attr1.target and sym2->attr.target implies
attr2.target.
> This looks like dead code to me.
Seems it wasn't originally dead when it was added in PR45019, but then the
PR62278 change made it dead.
But the function actually returns 0 rather than 1 that PR45019 meant.
So I bet in addition to fixing the pasto we should move that conditional
from where it is right now to the return 0; location after
check_data_pointer_types calls.
What I wonder is why the testcase doesn't actually fail.
And the pasto fix would guess fix
aliasing_dummy_5.f90 with
arg(2:3) = arr(1:2)
instead of
arr(2:3) = arg(1:2)
if the original testcase would actually fail.
> > In any case, bootstrapped/regtested on x86_64-linux and i686-linux
> > successfully, ok for trunk?
> >
> > 2024-08-01 Jakub Jelinek <ja...@redhat.com>
> >
> > * dependency.cc (gfc_check_dependency): Fix a pasto, check
> > sym1->as->type for AS_ASSUMED_SHAPE rather than sym2->as->type.
> > Formatting fix.
> >
> > --- gcc/fortran/dependency.cc.jj 2024-07-01 11:28:22.705237968 +0200
> > +++ gcc/fortran/dependency.cc 2024-07-31 20:53:34.471588828 +0200
> > @@ -1368,7 +1368,7 @@ gfc_check_dependency (gfc_expr *expr1, g
> > if ((attr1.pointer || attr1.target) && (attr2.pointer ||
> > attr2.target))
> > {
> > if (check_data_pointer_types (expr1, expr2)
> > - && check_data_pointer_types (expr2, expr1))
> > + && check_data_pointer_types (expr2, expr1))
> > return 0;
> > return 1;
> > @@ -1380,7 +1380,7 @@ gfc_check_dependency (gfc_expr *expr1, g
> > if (sym1->attr.target && sym2->attr.target
>
> if both target attributes are true here, both attr1.target and attr2.target
> are true as well, so the conditional before is true and this branch is
> skipped.
>
> > && ((sym1->attr.dummy && !sym1->attr.contiguous
> > && (!sym1->attr.dimension
> > - || sym2->as->type == AS_ASSUMED_SHAPE))
> > + || sym1->as->type == AS_ASSUMED_SHAPE))
> > || (sym2->attr.dummy && !sym2->attr.contiguous
> > && (!sym2->attr.dimension
> > || sym2->as->type == AS_ASSUMED_SHAPE))))
Jakub
