public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug analyzer/109570] New: detect fclose on unopened or NULL files
@ 2023-04-20 13:33 vanyacpp at gmail dot com
2023-04-20 13:36 ` [Bug analyzer/109570] " vanyacpp at gmail dot com
` (6 more replies)
0 siblings, 7 replies; 8+ messages in thread
From: vanyacpp at gmail dot com @ 2023-04-20 13:33 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109570
Bug ID: 109570
Summary: detect fclose on unopened or NULL files
Product: gcc
Version: 14.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: analyzer
Assignee: dmalcolm at gcc dot gnu.org
Reporter: vanyacpp at gmail dot com
Target Milestone: ---
While cleaning up one not particularly well written program I noticed this code
fragment:
FILE* file = fopen(...);
if (!file)
{
fclose(file);
return false;
}
Passing NULL to fclose is undefined behavior. Perhaps -fanalyzer could warn
about code like this?
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug analyzer/109570] detect fclose on unopened or NULL files
2023-04-20 13:33 [Bug analyzer/109570] New: detect fclose on unopened or NULL files vanyacpp at gmail dot com
@ 2023-04-20 13:36 ` vanyacpp at gmail dot com
2023-04-20 21:31 ` dmalcolm at gcc dot gnu.org
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: vanyacpp at gmail dot com @ 2023-04-20 13:36 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109570
--- Comment #1 from Ivan Sorokin <vanyacpp at gmail dot com> ---
Generalizing. Perhaps similarly free(NULL) can be detected?
void* obj = malloc(...);
if (!obj)
{
free(obj);
return false;
}
Unliky fclose(NULL), free(NULL) is completely well defined operation, but it
does nothing and perhaps should be removed.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug analyzer/109570] detect fclose on unopened or NULL files
2023-04-20 13:33 [Bug analyzer/109570] New: detect fclose on unopened or NULL files vanyacpp at gmail dot com
2023-04-20 13:36 ` [Bug analyzer/109570] " vanyacpp at gmail dot com
@ 2023-04-20 21:31 ` dmalcolm at gcc dot gnu.org
2023-05-16 10:20 ` xry111 at gcc dot gnu.org
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: dmalcolm at gcc dot gnu.org @ 2023-04-20 21:31 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109570
--- Comment #2 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Thanks for filing this bug.
I think -fanalyzer should warn about fclose(NULL), but not for free(NULL).
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug analyzer/109570] detect fclose on unopened or NULL files
2023-04-20 13:33 [Bug analyzer/109570] New: detect fclose on unopened or NULL files vanyacpp at gmail dot com
2023-04-20 13:36 ` [Bug analyzer/109570] " vanyacpp at gmail dot com
2023-04-20 21:31 ` dmalcolm at gcc dot gnu.org
@ 2023-05-16 10:20 ` xry111 at gcc dot gnu.org
2023-05-16 20:29 ` clyon at gcc dot gnu.org
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: xry111 at gcc dot gnu.org @ 2023-05-16 10:20 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109570
Xi Ruoyao <xry111 at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
URL|https://sourceware.org/pipe |https://sourceware.org/git/
|rmail/libc-alpha/2023-April |?p=glibc.git;a=commit;h=71d
|/147509.html |9e0fe766a3c22a730995b9d0249
| |60970670af
--- Comment #3 from Xi Ruoyao <xry111 at gcc dot gnu.org> ---
With the Glibc change there should be an analyzer-null-argument warning now. I
guess we can close it as MOVED.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug analyzer/109570] detect fclose on unopened or NULL files
2023-04-20 13:33 [Bug analyzer/109570] New: detect fclose on unopened or NULL files vanyacpp at gmail dot com
` (2 preceding siblings ...)
2023-05-16 10:20 ` xry111 at gcc dot gnu.org
@ 2023-05-16 20:29 ` clyon at gcc dot gnu.org
2023-05-17 15:03 ` clyon at gcc dot gnu.org
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: clyon at gcc dot gnu.org @ 2023-05-16 20:29 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109570
Christophe Lyon <clyon at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |clyon at gcc dot gnu.org
--- Comment #4 from Christophe Lyon <clyon at gcc dot gnu.org> ---
The glibc change is now causing failures in the GCC testsuite:
FAIL: gcc.dg/analyzer/torture/conftest-1.c -O0 (test for excess errors)
Excess errors:
/gcc/testsuite/gcc.dg/analyzer/torture/conftest-1.c:6:24: warning: use of
possibly-NULL 'f' where non-null expected [CWE-690]
[-Wanalyzer-possible-null-argument]
FAIL: gcc.dg/analyzer/data-model-4.c (test for excess errors)
Excess errors:
/gcc/testsuite/gcc.dg/analyzer/data-model-4.c:11:24: warning: use of
possibly-NULL 'f' where non-null expected [CWE-690]
[-Wanalyzer-possible-null-argument]
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug analyzer/109570] detect fclose on unopened or NULL files
2023-04-20 13:33 [Bug analyzer/109570] New: detect fclose on unopened or NULL files vanyacpp at gmail dot com
` (3 preceding siblings ...)
2023-05-16 20:29 ` clyon at gcc dot gnu.org
@ 2023-05-17 15:03 ` clyon at gcc dot gnu.org
2023-05-17 15:14 ` xry111 at gcc dot gnu.org
2023-08-15 21:00 ` glebfm at altlinux dot org
6 siblings, 0 replies; 8+ messages in thread
From: clyon at gcc dot gnu.org @ 2023-05-17 15:03 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109570
--- Comment #5 from Christophe Lyon <clyon at gcc dot gnu.org> ---
Not sure how to update/fix the testcases though?
Since they get the declaration of fclose from stdio.h, we'd need to make
dg-error conditional to the glibc version in use, which seems unpractical.
Should we instead remove #include <stdio.h> and provide suitable declarations
in the testcase?
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug analyzer/109570] detect fclose on unopened or NULL files
2023-04-20 13:33 [Bug analyzer/109570] New: detect fclose on unopened or NULL files vanyacpp at gmail dot com
` (4 preceding siblings ...)
2023-05-17 15:03 ` clyon at gcc dot gnu.org
@ 2023-05-17 15:14 ` xry111 at gcc dot gnu.org
2023-08-15 21:00 ` glebfm at altlinux dot org
6 siblings, 0 replies; 8+ messages in thread
From: xry111 at gcc dot gnu.org @ 2023-05-17 15:14 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109570
--- Comment #6 from Xi Ruoyao <xry111 at gcc dot gnu.org> ---
(In reply to Christophe Lyon from comment #5)
> Not sure how to update/fix the testcases though?
> Since they get the declaration of fclose from stdio.h, we'd need to make
> dg-error conditional to the glibc version in use, which seems unpractical.
>
> Should we instead remove #include <stdio.h> and provide suitable
> declarations in the testcase?
I guess we need to change
return ferror (f) || fclose (f) != 0;
to
return !f || ferror (f) || fclose (f) != 0;
Because "failing to check if the file is opened successfully" is definitely a
bug, and these tests are intended not to raise warnings for a bug-free program.
BTW ferror(f) segfaults as well when f is NULL, so IMO we should mark it
nonnull in Glibc as well.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug analyzer/109570] detect fclose on unopened or NULL files
2023-04-20 13:33 [Bug analyzer/109570] New: detect fclose on unopened or NULL files vanyacpp at gmail dot com
` (5 preceding siblings ...)
2023-05-17 15:14 ` xry111 at gcc dot gnu.org
@ 2023-08-15 21:00 ` glebfm at altlinux dot org
6 siblings, 0 replies; 8+ messages in thread
From: glebfm at altlinux dot org @ 2023-08-15 21:00 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109570
Gleb Fotengauer-Malinovskiy <glebfm at altlinux dot org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |glebfm at altlinux dot org
--- Comment #7 from Gleb Fotengauer-Malinovskiy <glebfm at altlinux dot org> ---
FYI, the change in the glibc also has an effect on autoconf-based projects,
when the -fanalyzer flag is used in combination with the -Werror flag, see:
https://git.savannah.gnu.org/cgit/autoconf.git/commit/?id=ea3e0cec2e66132e34228546256a1657c7b9b2e9
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-08-15 21:00 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-20 13:33 [Bug analyzer/109570] New: detect fclose on unopened or NULL files vanyacpp at gmail dot com
2023-04-20 13:36 ` [Bug analyzer/109570] " vanyacpp at gmail dot com
2023-04-20 21:31 ` dmalcolm at gcc dot gnu.org
2023-05-16 10:20 ` xry111 at gcc dot gnu.org
2023-05-16 20:29 ` clyon at gcc dot gnu.org
2023-05-17 15:03 ` clyon at gcc dot gnu.org
2023-05-17 15:14 ` xry111 at gcc dot gnu.org
2023-08-15 21:00 ` glebfm at altlinux dot 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).