From 24918a9b20c771984f41f90c3352c3ebee8b5e6f Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Thu, 19 Dec 2019 16:10:41 -0500 Subject: analyzer: add -Wanalyzer-use-of-closed-file gcc/analyzer/ChangeLog: * analyzer.opt (Wanalyzer-use-of-closed-file): New option. * sm-file.cc (class use_of_closed_file): New file_diagnostic subclass. (find_file_param): New function. (fileptr_state_machine::on_stmt): Complain about operations on closed files. gcc/ChangeLog: * doc/invoke.texi (-Wanalyzer-use-of-closed-file): Document new option. gcc/testsuite/ChangeLog: * gcc.dg/analyzer/file-1.c (test_5): New test. --- gcc/analyzer/analyzer.opt | 4 ++ gcc/analyzer/sm-file.cc | 92 +++++++++++++++++++++++++++++++++- gcc/doc/invoke.texi | 13 +++++ gcc/testsuite/gcc.dg/analyzer/file-1.c | 8 +++ 4 files changed, 116 insertions(+), 1 deletion(-) diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt index af8d81d..3311e97 100644 --- a/gcc/analyzer/analyzer.opt +++ b/gcc/analyzer/analyzer.opt @@ -94,6 +94,10 @@ Wanalyzer-use-after-free Common Var(warn_analyzer_use_after_free) Init(1) Warning Warn about code paths in which a freed value is used. +Wanalyzer-use-of-closed-file +Common Var(warn_analyzer_use_of_closed_file) Init(1) Warning +Warn about code paths in which a FILE * is used after being closed. + Wanalyzer-use-of-pointer-in-stale-stack-frame Common Var(warn_analyzer_use_of_pointer_in_stale_stack_frame) Init(1) Warning Warn about code paths in which a pointer to a stale stack frame is used. diff --git a/gcc/analyzer/sm-file.cc b/gcc/analyzer/sm-file.cc index f731981..e7fb0f0 100644 --- a/gcc/analyzer/sm-file.cc +++ b/gcc/analyzer/sm-file.cc @@ -161,6 +161,46 @@ private: diagnostic_event_id_t m_first_fclose_event; }; +class use_of_closed_file : public file_diagnostic +{ +public: + use_of_closed_file (const fileptr_state_machine &sm, tree arg) + : file_diagnostic (sm, arg) + {} + + const char *get_kind () const FINAL OVERRIDE { return "use_of_closed_file"; } + + bool emit (rich_location *rich_loc) FINAL OVERRIDE + { + // FIXME: CWE? + return warning_at (rich_loc, OPT_Wanalyzer_use_of_closed_file, + "use of closed FILE %qE", + m_arg); + } + + label_text describe_state_change (const evdesc::state_change &change) + OVERRIDE + { + if (change.m_new_state == m_sm.m_closed) + { + m_fclose_event = change.m_event_id; + return change.formatted_print ("file closed here"); + } + return file_diagnostic::describe_state_change (change); + } + + label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE + { + if (m_fclose_event.known_p ()) + return ev.formatted_print ("use of closed FILE %qE; closed at %@", + ev.m_expr, &m_fclose_event); + return ev.formatted_print ("use of closed FILE %qE", ev.m_expr); + } + +private: + diagnostic_event_id_t m_fclose_event; +}; + class file_leak : public file_diagnostic { public: @@ -294,6 +334,41 @@ is_file_using_fn_p (tree fndecl) return fs.contains_decl_p (fndecl); } +/* If FNDECL takes a FILE * param, write the 0-based index of the first + such param to *OUT_IDX and return true. */ + +static bool +find_file_param (tree fndecl, int *out_idx) +{ + int idx = 0; + for (tree iter_param_types = TYPE_ARG_TYPES (TREE_TYPE (fndecl)); + iter_param_types; + iter_param_types = TREE_CHAIN (iter_param_types), idx++) + { + tree param_type = TREE_VALUE (iter_param_types); + + /* Looks like we can't rely on fileptr_type_node being set up; + instead, look for a ptr to record called "FILE". */ + + if (!POINTER_TYPE_P (param_type)) + continue; + tree type = TREE_TYPE (param_type); + if (TREE_CODE (type) == RECORD_TYPE + && TYPE_NAME (type) + && TREE_CODE (TYPE_NAME (type)) == TYPE_DECL) + { + tree tdecl = TYPE_NAME (type); + if (DECL_NAME (tdecl) + && strcmp (IDENTIFIER_POINTER (DECL_NAME (tdecl)), "FILE") == 0) + { + *out_idx = idx; + return true; + } + } + } + return false; +} + /* Implementation of state_machine::on_stmt vfunc for fileptr_state_machine. */ bool @@ -340,12 +415,27 @@ fileptr_state_machine::on_stmt (sm_context *sm_ctxt, if (is_file_using_fn_p (callee_fndecl)) { - // TODO: operations on unchecked file + int param_idx; + if (find_file_param (callee_fndecl, ¶m_idx)) + { + /* Look up FILE * param. */ + tree arg = gimple_call_arg (call, param_idx); + arg = sm_ctxt->get_readable_tree (arg); + + // TODO: operations on unchecked file + + /* Complain about operations on closed files. */ + sm_ctxt->warn_for_state (node, stmt, arg, m_closed, + new use_of_closed_file (*this, arg)); + sm_ctxt->on_transition (node, stmt, arg, m_closed, m_stop); + } return true; } // etc } + // TODO: fcloseall() (a GNU extension) + return false; } diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 1374204..5438748 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -309,6 +309,7 @@ Objective-C and Objective-C++ Dialects}. -Wno-analyzer-tainted-array-index @gol -Wno-analyzer-unsafe-call-within-signal-handler @gol -Wno-analyzer-use-after-free @gol +-Wno-analyzer-use-of-closed-file @gol -Wno-analyzer-use-of-pointer-in-stale-stack-frame @gol -Wno-analyzer-use-of-uninitialized-value @gol -Wanalyzer-too-complex @gol @@ -409,6 +410,7 @@ Objective-C and Objective-C++ Dialects}. -Wanalyzer-tainted-array-index @gol -Wanalyzer-unsafe-call-within-signal-handler @gol -Wanalyzer-use-after-free @gol +-Wanalyzer-use-of-closed-file @gol -Wanalyzer-use-of-pointer-in-stale-stack-frame @gol -Wanalyzer-use-of-uninitialized-value @gol -Wanalyzer-too-complex @gol @@ -6615,6 +6617,16 @@ This warning requires @option{-fanalyzer}, which enables it; use This diagnostic warns for paths through the code in which a pointer is used after @code{free} is called on it. +@item -Wno-analyzer-use-of-closed-file +@opindex Wanalyzer-use-of-closed-file +@opindex Wno-analyzer-use-of-closed-file +This warning requires @option{-fanalyzer}, which enables it; use +@option{-Wno-analyzer-use-of-closed-file} to disable it. + +This diagnostic warns for paths through the code in which a +@code{} @code{FILE *} stream object is used after +@code{fclose} is called on it. + @item -Wno-analyzer-use-of-pointer-in-stale-stack-frame @opindex Wanalyzer-use-of-pointer-in-stale-stack-frame @opindex Wno-analyzer-use-of-pointer-in-stale-stack-frame @@ -8330,6 +8342,7 @@ Enabling this option effectively enables the following warnings: -Wanalyzer-tainted-array-index @gol -Wanalyzer-unsafe-call-within-signal-handler @gol -Wanalyzer-use-after-free @gol +-Wanalyzer-use-of-closed-file @gol -Wanalyzer-use-of-uninitialized-value @gol -Wanalyzer-use-of-pointer-in-stale-stack-frame @gol } diff --git a/gcc/testsuite/gcc.dg/analyzer/file-1.c b/gcc/testsuite/gcc.dg/analyzer/file-1.c index ba516af..8531fb0 100644 --- a/gcc/testsuite/gcc.dg/analyzer/file-1.c +++ b/gcc/testsuite/gcc.dg/analyzer/file-1.c @@ -47,3 +47,11 @@ test_4 (const char *path) return; /* { dg-warning "leak of FILE 'f'" } */ } + +void +test_5 (FILE *f, const char *msg) +{ + fclose (f); /* { dg-message "\\(1\\) file closed here" } */ + fprintf (f, "foo: %s", msg); /* { dg-warning "use of closed FILE 'f'" } */ + /* { dg-message "\\(2\\) use of closed FILE 'f'; closed at \\(1\\)" "" { target *-*-* } .-1 } */ +} -- cgit v1.1