The analyzer ought to report various file leaks for the reproducer in
PR analyzer/58237, such as:
void f1(const char *str)
{
FILE * fp = fopen(str, "r");
char buf[10];
while (fgets(buf, 10, fp) != NULL)
{
/* Do something with buf */
}
/* Missing call to fclose. Need warning here for resource leak */
}
but fails to do so, due to not recognizing fgets, and thus
conservatively assuming that it could close "fp".
This patch adds a function_set to sm-file.cc of numerous stdio.h
functions that are known to not close the file (and which require a
valid FILE *, but that's a matter for a followup), fixing the issue.
gcc/analyzer/ChangeLog:
PR analyzer/58237
* analyzer-selftests.cc (selftest::run_analyzer_selftests): Call
selftest::analyzer_sm_file_cc_tests.
* analyzer-selftests.h (selftest::analyzer_sm_file_cc_tests): New
decl.
* sm-file.cc: Include "analyzer/function-set.h" and
"analyzer/analyzer-selftests.h".
(get_file_using_fns): New function.
(is_file_using_fn_p): New function.
(fileptr_state_machine::on_stmt): Return true for known functions.
(selftest::analyzer_sm_file_cc_tests): New function.
gcc/testsuite/ChangeLog:
PR analyzer/58237
* gcc.dg/analyzer/file-1.c (test_4): New.
* gcc.dg/analyzer/file-pr58237.c: New test.
---
gcc/analyzer/analyzer-selftests.cc | 1 +
gcc/analyzer/analyzer-selftests.h | 1 +
gcc/analyzer/sm-file.cc | 102 ++++++++++++++++++-
gcc/testsuite/gcc.dg/analyzer/file-1.c | 12 +++
gcc/testsuite/gcc.dg/analyzer/file-pr58237.c | 72 +++++++++++++
5 files changed, 187 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/analyzer/file-pr58237.c
diff --git a/gcc/analyzer/analyzer-selftests.cc
b/gcc/analyzer/analyzer-selftests.cc
index f90dad8e991f..182862428e31 100644
--- a/gcc/analyzer/analyzer-selftests.cc
+++ b/gcc/analyzer/analyzer-selftests.cc
@@ -53,6 +53,7 @@ run_analyzer_selftests ()
analyzer_program_point_cc_tests ();
analyzer_program_state_cc_tests ();
analyzer_region_model_cc_tests ();
+ analyzer_sm_file_cc_tests ();
analyzer_sm_signal_cc_tests ();
#endif /* #if ENABLE_ANALYZER */
}
diff --git a/gcc/analyzer/analyzer-selftests.h
b/gcc/analyzer/analyzer-selftests.h
index 225b717c9d13..ce336c0b2b41 100644
--- a/gcc/analyzer/analyzer-selftests.h
+++ b/gcc/analyzer/analyzer-selftests.h
@@ -37,6 +37,7 @@ extern void analyzer_function_set_cc_tests ();
extern void analyzer_program_point_cc_tests ();
extern void analyzer_program_state_cc_tests ();
extern void analyzer_region_model_cc_tests ();
+extern void analyzer_sm_file_cc_tests ();
extern void analyzer_sm_signal_cc_tests ();
} /* end of namespace selftest. */
diff --git a/gcc/analyzer/sm-file.cc b/gcc/analyzer/sm-file.cc
index e4eeb1f41be9..9a8ce4911d07 100644
--- a/gcc/analyzer/sm-file.cc
+++ b/gcc/analyzer/sm-file.cc
@@ -31,6 +31,8 @@ along with GCC; see the file COPYING3. If not see
#include "analyzer/analyzer.h"
#include "analyzer/pending-diagnostic.h"
#include "analyzer/sm.h"
+#include "analyzer/function-set.h"
+#include "analyzer/analyzer-selftests.h"
#if ENABLE_ANALYZER
@@ -213,6 +215,82 @@ fileptr_state_machine::fileptr_state_machine (logger
*logger)
m_stop = add_state ("stop");
}
+/* Get a set of functions that are known to take a FILE * that must be open,
+ and are known to not close it. */
+
+static function_set
+get_file_using_fns ()
+{
+ // TODO: populate this list more fully
+ static const char * const funcnames[] = {
+ /* This array must be kept sorted. */
+ "__fbufsize",
+ "__flbf",
+ "__fpending",
+ "__fpurge"
+ "__freadable",
+ "__freading",
+ "__fsetlocking",
+ "__fwritable",
+ "__fwriting",
+ "clearerr",
+ "clearerr_unlocked",
+ "feof",
+ "feof_unlocked",
+ "ferror",
+ "ferror_unlocked",
+ "fflush", // safe to call with NULL
+ "fflush_unlocked", // safe to call with NULL
+ "fgetc",
+ "fgetc_unlocked",
+ "fgetpos",
+ "fgets",
+ "fgets_unlocked",
+ "fgetwc_unlocked",
+ "fgetws_unlocked",
+ "fileno",
+ "fileno_unlocked",
+ "fprintf",
+ "fputc",
+ "fputc_unlocked",
+ "fputs",
+ "fputs_unlocked",
+ "fputwc_unlocked",
+ "fputws_unlocked",
+ "fread_unlocked",
+ "fseek",
+ "fsetpos",
+ "ftell",
+ "fwrite_unlocked",
+ "getc",
+ "getc_unlocked",
+ "getwc_unlocked",
+ "putc",
+ "putc_unlocked",
+ "rewind",
+ "setbuf",
+ "setbuffer",
+ "setlinebuf",
+ "setvbuf",
+ "ungetc",
+ "vfprintf"
+ };
+ const size_t count
+ = sizeof(funcnames) / sizeof (funcnames[0]);
+ function_set fs (funcnames, count);
+ return fs;
+}
+
+/* Return true if FNDECL is known to require an open FILE *, and is known
+ to not close it. */
+
+static bool
+is_file_using_fn_p (tree fndecl)
+{
+ function_set fs = get_file_using_fns ();
+ return fs.contains_decl_p (fndecl);
+}
+
/* Implementation of state_machine::on_stmt vfunc for fileptr_state_machine.
*/
bool
@@ -257,7 +335,11 @@ fileptr_state_machine::on_stmt (sm_context *sm_ctxt,
return true;
}
- // TODO: operations on closed file
+ if (is_file_using_fn_p (callee_fndecl))
+ {
+ // TODO: operations on unchecked file
+ return true;
+ }
// etc
}
@@ -331,4 +413,22 @@ make_fileptr_state_machine (logger *logger)
return new fileptr_state_machine (logger);
}
+#if CHECKING_P
+
+namespace selftest {
+
+/* Run all of the selftests within this file. */
+
+void
+analyzer_sm_file_cc_tests ()
+{
+ function_set fs = get_file_using_fns ();
+ fs.assert_sorted ();
+ fs.assert_sane ();
+}
+
+} // namespace selftest
+
+#endif /* CHECKING_P */
+
#endif /* #if ENABLE_ANALYZER */
diff --git a/gcc/testsuite/gcc.dg/analyzer/file-1.c
b/gcc/testsuite/gcc.dg/analyzer/file-1.c
index 91d9685d9e9e..ba516afc8af0 100644
--- a/gcc/testsuite/gcc.dg/analyzer/file-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/file-1.c
@@ -35,3 +35,15 @@ test_3 (const char *path)
FILE *f = fopen (path, "r"); /* { dg-message "opened here" } */
return; /* { dg-warning "leak of FILE 'f'" } */
}
+
+void
+test_4 (const char *path)
+{
+ FILE *f = fopen (path, "r"); /* { dg-message "opened here" } */
+
+ /* Ensure we know about common fns that are known to not close the
+ file (e.g. "fseek"). */
+ fseek (f, 1024, SEEK_SET);
+
+ return; /* { dg-warning "leak of FILE 'f'" } */
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/file-pr58237.c
b/gcc/testsuite/gcc.dg/analyzer/file-pr58237.c
new file mode 100644
index 000000000000..68f49c25607a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/file-pr58237.c
@@ -0,0 +1,72 @@
+#include <stdio.h>
+
+void f0(const char *str)
+{
+ FILE * fp = fopen(str, "r"); /* { dg-message "opened here" } */
+ char buf[10];
+ fgets(buf, 10, fp);
+} /* { dg-warning "leak of FILE 'fp'" } */
+
+void f1(const char *str)
+{
+ FILE * fp = fopen(str, "r"); /* { dg-message "opened here" } */
+ char buf[10];
+
+ while (fgets(buf, 10, fp) != NULL)
+ {
+ /* Do something with buf */
+ }
+} /* { dg-warning "leak of FILE 'fp'" } */
+
+void f2(const char *str, int flag)
+{
+ FILE * fp = fopen(str, "r"); /* { dg-message "opened here" } */
+ char buf[10];
+
+ while (fgets(buf, 10, fp) != NULL)
+ {
+ /* Do something with buf */
+ }
+ if (flag) /* { dg-message "when 'flag == 0'" } */
+ fclose(fp);
+} /* { dg-warning "leak of FILE 'fp'" } */
+
+extern void called_by_f3( FILE * fp);
+
+void f3(const char *str)
+{
+ FILE * fp = fopen(str, "r");
+ char buf[10];
+
+ while (fgets(buf, 10, fp) != NULL)
+ {
+ /* Do something with buf */
+ }
+ /* Not sure if fclose executed by called_by_f3 or not. Say nothing */
+ called_by_f3(fp);
+}
+
+void f4(const char *str)
+{
+ FILE * fp = fopen(str, "r");
+ char buf[10];
+
+ while (fgets(buf, 10, fp) != NULL)
+ {
+ /* Do something with buf */
+ }
+ /* Nothing to say here. */
+ fclose(fp);
+}
+
+void main(int argc, const char * argv[])
+{
+ FILE * fp = fopen(argv[0], "r");
+ char buf[10];
+
+ while (fgets(buf, 10, fp) != NULL)
+ {
+ /* Do something with buf */
+ }
+ /* Nothing to say here, because we are in main. */
+}
--
2.21.0
