public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug fortran/95512] New: gcc/fortran/trans-decl.c:1066: array sanity check after use
@ 2020-06-03 19:46 dcb314 at hotmail dot com
2020-06-05 21:24 ` [Bug fortran/95512] " anlauf at gcc dot gnu.org
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: dcb314 at hotmail dot com @ 2020-06-03 19:46 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95512
Bug ID: 95512
Summary: gcc/fortran/trans-decl.c:1066: array sanity check
after use
Product: gcc
Version: unknown
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: fortran
Assignee: unassigned at gcc dot gnu.org
Reporter: dcb314 at hotmail dot com
Target Milestone: ---
Static analyser cppcheck says:
gcc/fortran/trans-decl.c:1066:11: style: Array index 'dim' is used before
limits check. [arrayIndexThenCheck]
Source code is
/* Don't try to use the unknown ubound for the last coarray dimension.
*/
if (GFC_TYPE_ARRAY_UBOUND (type, dim) == NULL_TREE
&& dim < GFC_TYPE_ARRAY_RANK (type) + GFC_TYPE_ARRAY_CORANK (type) -
1)
Maybe better code:
/* Don't try to use the unknown ubound for the last coarray dimension.
*/
if (dim < GFC_TYPE_ARRAY_RANK (type) + GFC_TYPE_ARRAY_CORANK (type) - 1
&& GFC_TYPE_ARRAY_UBOUND (type, dim) == NULL_TREE)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug fortran/95512] gcc/fortran/trans-decl.c:1066: array sanity check after use
2020-06-03 19:46 [Bug fortran/95512] New: gcc/fortran/trans-decl.c:1066: array sanity check after use dcb314 at hotmail dot com
@ 2020-06-05 21:24 ` anlauf at gcc dot gnu.org
2020-06-06 6:26 ` dcb314 at hotmail dot com
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: anlauf at gcc dot gnu.org @ 2020-06-05 21:24 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95512
anlauf at gcc dot gnu.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Last reconfirmed| |2020-06-05
Ever confirmed|0 |1
Status|UNCONFIRMED |WAITING
--- Comment #1 from anlauf at gcc dot gnu.org ---
This looks like a false positive: dim is the index of the enclosing for loop.
It is also funny that it warns about this statement and not the if preceeding
this one.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug fortran/95512] gcc/fortran/trans-decl.c:1066: array sanity check after use
2020-06-03 19:46 [Bug fortran/95512] New: gcc/fortran/trans-decl.c:1066: array sanity check after use dcb314 at hotmail dot com
2020-06-05 21:24 ` [Bug fortran/95512] " anlauf at gcc dot gnu.org
@ 2020-06-06 6:26 ` dcb314 at hotmail dot com
2020-06-06 19:59 ` anlauf at gcc dot gnu.org
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: dcb314 at hotmail dot com @ 2020-06-06 6:26 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95512
--- Comment #2 from David Binderman <dcb314 at hotmail dot com> ---
I fail to understand how this can be a false positive.
if (array[ X] && X < something - 1)
looks like a pretty convincing case of use before sanity checking to me.
It is a standard pattern to sanity check array indexes before use.
The previous if doesn't sanity check the array index, so there is
no chance of getting the sanity check in a non standard place.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug fortran/95512] gcc/fortran/trans-decl.c:1066: array sanity check after use
2020-06-03 19:46 [Bug fortran/95512] New: gcc/fortran/trans-decl.c:1066: array sanity check after use dcb314 at hotmail dot com
2020-06-05 21:24 ` [Bug fortran/95512] " anlauf at gcc dot gnu.org
2020-06-06 6:26 ` dcb314 at hotmail dot com
@ 2020-06-06 19:59 ` anlauf at gcc dot gnu.org
2020-06-06 20:42 ` dcb314 at hotmail dot com
2020-06-14 12:54 ` tkoenig at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: anlauf at gcc dot gnu.org @ 2020-06-06 19:59 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95512
--- Comment #3 from anlauf at gcc dot gnu.org ---
I didn't write the code, but for more context:
trans.h has:
#define GFC_TYPE_ARRAY_LBOUND(node, dim) \
(TYPE_LANG_SPECIFIC(node)->lbound[dim])
#define GFC_TYPE_ARRAY_UBOUND(node, dim) \
(TYPE_LANG_SPECIFIC(node)->ubound[dim])
The loop in trans-decl.c:
for (dim = GFC_TYPE_ARRAY_RANK (type);
dim < GFC_TYPE_ARRAY_RANK (type) + GFC_TYPE_ARRAY_CORANK (type); dim++)
{
if (GFC_TYPE_ARRAY_LBOUND (type, dim) == NULL_TREE)
{
GFC_TYPE_ARRAY_LBOUND (type, dim) = create_index_var ("lbound",
nest);
TREE_NO_WARNING (GFC_TYPE_ARRAY_LBOUND (type, dim)) = 1;
}
/* Don't try to use the unknown ubound for the last coarray dimension.
*/
if (GFC_TYPE_ARRAY_UBOUND (type, dim) == NULL_TREE
&& dim < GFC_TYPE_ARRAY_RANK (type) + GFC_TYPE_ARRAY_CORANK (type) -
1)
{
GFC_TYPE_ARRAY_UBOUND (type, dim) = create_index_var ("ubound",
nest);
TREE_NO_WARNING (GFC_TYPE_ARRAY_UBOUND (type, dim)) = 1;
}
}
So the relevant check is in the loop header, and the current check is there
for the last index.
Maybe it is bad style, but I still consider it a false positive.
cppcheck's view is probably too narrow to understand the range of dim.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug fortran/95512] gcc/fortran/trans-decl.c:1066: array sanity check after use
2020-06-03 19:46 [Bug fortran/95512] New: gcc/fortran/trans-decl.c:1066: array sanity check after use dcb314 at hotmail dot com
` (2 preceding siblings ...)
2020-06-06 19:59 ` anlauf at gcc dot gnu.org
@ 2020-06-06 20:42 ` dcb314 at hotmail dot com
2020-06-14 12:54 ` tkoenig at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: dcb314 at hotmail dot com @ 2020-06-06 20:42 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95512
--- Comment #4 from David Binderman <dcb314 at hotmail dot com> ---
(In reply to anlauf from comment #3)
> I didn't write the code, but for more context:
Thanks for the extra context.
> Maybe it is bad style, but I still consider it a false positive.
If you go back and check what cppcheck is saying, it is merely
mentioning what it thinks is bad style.
> cppcheck's view is probably too narrow to understand the range of dim.
Agreed. AFAIK cppcheck merely searches and reports
on source code patterns like
if (arrayName[ X] && X < someExpression)
Whether you want to keep the code in bad style is up to you.
I did originally suggest some code that looked IMHO to be better style.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug fortran/95512] gcc/fortran/trans-decl.c:1066: array sanity check after use
2020-06-03 19:46 [Bug fortran/95512] New: gcc/fortran/trans-decl.c:1066: array sanity check after use dcb314 at hotmail dot com
` (3 preceding siblings ...)
2020-06-06 20:42 ` dcb314 at hotmail dot com
@ 2020-06-14 12:54 ` tkoenig at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: tkoenig at gcc dot gnu.org @ 2020-06-14 12:54 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95512
Thomas Koenig <tkoenig at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |INVALID
CC| |tkoenig at gcc dot gnu.org
Status|WAITING |RESOLVED
--- Comment #5 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
I don't think it is necessary to change the code to avoid false positives
like this. Hence, closing.
However, it was good to raise the issue so it could be checked.
Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-06-14 12:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-03 19:46 [Bug fortran/95512] New: gcc/fortran/trans-decl.c:1066: array sanity check after use dcb314 at hotmail dot com
2020-06-05 21:24 ` [Bug fortran/95512] " anlauf at gcc dot gnu.org
2020-06-06 6:26 ` dcb314 at hotmail dot com
2020-06-06 19:59 ` anlauf at gcc dot gnu.org
2020-06-06 20:42 ` dcb314 at hotmail dot com
2020-06-14 12:54 ` tkoenig at gcc dot gnu.org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).