aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Malcolm <dmalcolm@redhat.com>2019-12-19 15:59:04 -0500
committerDavid Malcolm <dmalcolm@redhat.com>2020-01-14 20:39:26 -0500
commit697251b7a1bb7c14d3805de22248e83a23b90d1a (patch)
tree4411d9448340e818e2a373260460f56207f14087
parent81a68b9e3774401a99719ea29640d13125745b41 (diff)
downloadgcc-697251b7a1bb7c14d3805de22248e83a23b90d1a.zip
gcc-697251b7a1bb7c14d3805de22248e83a23b90d1a.tar.gz
gcc-697251b7a1bb7c14d3805de22248e83a23b90d1a.tar.bz2
analyzer: add known stdio functions to sm-file.cc (PR analyzer/58237)
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.
-rw-r--r--gcc/analyzer/ChangeLog14
-rw-r--r--gcc/analyzer/analyzer-selftests.cc1
-rw-r--r--gcc/analyzer/analyzer-selftests.h1
-rw-r--r--gcc/analyzer/sm-file.cc104
-rw-r--r--gcc/testsuite/ChangeLog6
-rw-r--r--gcc/testsuite/gcc.dg/analyzer/file-1.c12
-rw-r--r--gcc/testsuite/gcc.dg/analyzer/file-pr58237.c72
7 files changed, 207 insertions, 3 deletions
diff --git a/gcc/analyzer/ChangeLog b/gcc/analyzer/ChangeLog
index bb3ac72..aad8528 100644
--- a/gcc/analyzer/ChangeLog
+++ b/gcc/analyzer/ChangeLog
@@ -1,5 +1,19 @@
2020-01-14 David Malcolm <dmalcolm@redhat.com>
+ 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.
+
+2020-01-14 David Malcolm <dmalcolm@redhat.com>
+
* analyzer-selftests.cc (selftest::run_analyzer_selftests): Call
selftest::analyzer_sm_signal_cc_tests.
* analyzer-selftests.h (selftest::analyzer_sm_signal_cc_tests):
diff --git a/gcc/analyzer/analyzer-selftests.cc b/gcc/analyzer/analyzer-selftests.cc
index 2b8fa81..1272936 100644
--- a/gcc/analyzer/analyzer-selftests.cc
+++ b/gcc/analyzer/analyzer-selftests.cc
@@ -54,6 +54,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 80be32f..62da6cd 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 375f522..f731981 100644
--- a/gcc/analyzer/sm-file.cc
+++ b/gcc/analyzer/sm-file.cc
@@ -33,9 +33,9 @@ along with GCC; see the file COPYING3. If not see
#include "diagnostic-event-id.h"
#include "analyzer/analyzer-logging.h"
#include "analyzer/sm.h"
-#include "diagnostic-event-id.h"
-#include "analyzer/sm.h"
#include "analyzer/pending-diagnostic.h"
+#include "analyzer/function-set.h"
+#include "analyzer/analyzer-selftests.h"
#if ENABLE_ANALYZER
@@ -218,6 +218,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
@@ -262,7 +338,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
}
@@ -336,4 +416,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/ChangeLog b/gcc/testsuite/ChangeLog
index 2e59f28..18c6a88 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2020-01-14 David Malcolm <dmalcolm@redhat.com>
+
+ PR analyzer/58237
+ * gcc.dg/analyzer/file-1.c (test_4): New.
+ * gcc.dg/analyzer/file-pr58237.c: New test.
+
2020-01-15 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/93262
diff --git a/gcc/testsuite/gcc.dg/analyzer/file-1.c b/gcc/testsuite/gcc.dg/analyzer/file-1.c
index 91d9685..ba516af 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 0000000..68f49c2
--- /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. */
+}