analyzer: add known stdio functions to sm-file.cc (PR analyzer/58237)
authorDavid Malcolm <dmalcolm@redhat.com>
Thu, 19 Dec 2019 20:59:04 +0000 (15:59 -0500)
committerDavid Malcolm <dmalcolm@redhat.com>
Wed, 15 Jan 2020 01:39:26 +0000 (20:39 -0500)
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/ChangeLog
gcc/analyzer/analyzer-selftests.cc
gcc/analyzer/analyzer-selftests.h
gcc/analyzer/sm-file.cc
gcc/testsuite/ChangeLog
gcc/testsuite/gcc.dg/analyzer/file-1.c
gcc/testsuite/gcc.dg/analyzer/file-pr58237.c [new file with mode: 0644]

index bb3ac72..aad8528 100644 (file)
@@ -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):
index 2b8fa81..1272936 100644 (file)
@@ -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 */
 }
index 80be32f..62da6cd 100644 (file)
@@ -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.  */
index 375f522..f731981 100644 (file)
@@ -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 */
index 2e59f28..18c6a88 100644 (file)
@@ -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
index 91d9685..ba516af 100644 (file)
@@ -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 (file)
index 0000000..68f49c2
--- /dev/null
@@ -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. */
+}