Le 02/08/2024 à 10:12, Jakub Jelinek a écrit :
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.
nods
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.
No, the function check_data_pointer_types returns true if there is no
aliasing, according to its function comment, so after both calls
everything is safe and returning anything but 0 is overly pessimistic.
I'm inclined to just remove the dead code.
By the way, looking for a testcase exercising gfc_check_dependency, I
found an example showing check_data_pointer_types is not up to its
promise. This is https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116196
What I wonder is why the testcase doesn't actually fail.
From https://gcc.gnu.org/bugzilla/show_bug.cgi?id=45019#c7 :
I think the patch below looks fine, however, if I set a break point, the function
"gfc_check_dependency" is never called for test program.
So the dependency.c part of the patch is not exercised by the testcase.
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.
Mmh, aren't they both actually the same?
