Commit 697251b7 by David Malcolm

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.
parent 81a68b9e
2020-01-14 David Malcolm <dmalcolm@redhat.com> 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 * analyzer-selftests.cc (selftest::run_analyzer_selftests): Call
selftest::analyzer_sm_signal_cc_tests. selftest::analyzer_sm_signal_cc_tests.
* analyzer-selftests.h (selftest::analyzer_sm_signal_cc_tests): * analyzer-selftests.h (selftest::analyzer_sm_signal_cc_tests):
......
...@@ -54,6 +54,7 @@ run_analyzer_selftests () ...@@ -54,6 +54,7 @@ run_analyzer_selftests ()
analyzer_program_point_cc_tests (); analyzer_program_point_cc_tests ();
analyzer_program_state_cc_tests (); analyzer_program_state_cc_tests ();
analyzer_region_model_cc_tests (); analyzer_region_model_cc_tests ();
analyzer_sm_file_cc_tests ();
analyzer_sm_signal_cc_tests (); analyzer_sm_signal_cc_tests ();
#endif /* #if ENABLE_ANALYZER */ #endif /* #if ENABLE_ANALYZER */
} }
......
...@@ -37,6 +37,7 @@ extern void analyzer_function_set_cc_tests (); ...@@ -37,6 +37,7 @@ extern void analyzer_function_set_cc_tests ();
extern void analyzer_program_point_cc_tests (); extern void analyzer_program_point_cc_tests ();
extern void analyzer_program_state_cc_tests (); extern void analyzer_program_state_cc_tests ();
extern void analyzer_region_model_cc_tests (); extern void analyzer_region_model_cc_tests ();
extern void analyzer_sm_file_cc_tests ();
extern void analyzer_sm_signal_cc_tests (); extern void analyzer_sm_signal_cc_tests ();
} /* end of namespace selftest. */ } /* end of namespace selftest. */
......
...@@ -33,9 +33,9 @@ along with GCC; see the file COPYING3. If not see ...@@ -33,9 +33,9 @@ along with GCC; see the file COPYING3. If not see
#include "diagnostic-event-id.h" #include "diagnostic-event-id.h"
#include "analyzer/analyzer-logging.h" #include "analyzer/analyzer-logging.h"
#include "analyzer/sm.h" #include "analyzer/sm.h"
#include "diagnostic-event-id.h"
#include "analyzer/sm.h"
#include "analyzer/pending-diagnostic.h" #include "analyzer/pending-diagnostic.h"
#include "analyzer/function-set.h"
#include "analyzer/analyzer-selftests.h"
#if ENABLE_ANALYZER #if ENABLE_ANALYZER
...@@ -218,6 +218,82 @@ fileptr_state_machine::fileptr_state_machine (logger *logger) ...@@ -218,6 +218,82 @@ fileptr_state_machine::fileptr_state_machine (logger *logger)
m_stop = add_state ("stop"); 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. */ /* Implementation of state_machine::on_stmt vfunc for fileptr_state_machine. */
bool bool
...@@ -262,7 +338,11 @@ fileptr_state_machine::on_stmt (sm_context *sm_ctxt, ...@@ -262,7 +338,11 @@ fileptr_state_machine::on_stmt (sm_context *sm_ctxt,
return true; return true;
} }
// TODO: operations on closed file if (is_file_using_fn_p (callee_fndecl))
{
// TODO: operations on unchecked file
return true;
}
// etc // etc
} }
...@@ -336,4 +416,22 @@ make_fileptr_state_machine (logger *logger) ...@@ -336,4 +416,22 @@ make_fileptr_state_machine (logger *logger)
return new fileptr_state_machine (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 */ #endif /* #if ENABLE_ANALYZER */
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> 2020-01-15 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/93262 PR tree-optimization/93262
......
...@@ -35,3 +35,15 @@ test_3 (const char *path) ...@@ -35,3 +35,15 @@ test_3 (const char *path)
FILE *f = fopen (path, "r"); /* { dg-message "opened here" } */ FILE *f = fopen (path, "r"); /* { dg-message "opened here" } */
return; /* { dg-warning "leak of FILE 'f'" } */ 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'" } */
}
#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. */
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment