[4/4] analyzer: add -Wanalyzer-use-of-closed-file

Message ID 20191220012138.6820-5-dmalcolm@redhat.com
State New
Headers show
Series
  • analyzer: add class function_set and use in various places
Related show

Commit Message

David Malcolm Dec. 20, 2019, 1:21 a.m.
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(-)

-- 
2.21.0

Patch

diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt
index d722667dea09..2f5008d1ea3b 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 9a8ce4911d07..3205474b03d3 100644
--- a/gcc/analyzer/sm-file.cc
+++ b/gcc/analyzer/sm-file.cc
@@ -158,6 +158,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:
@@ -291,6 +331,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
@@ -337,12 +412,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, &param_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 3a8c83592ef8..dc6d5a14ed41 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -310,6 +310,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
@@ -410,6 +411,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
@@ -6575,6 +6577,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{<stdio.h>} @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
@@ -8290,6 +8302,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 ba516afc8af0..8531fb0296c9 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 } */
+}