diff mbox series

[35/49] analyzer: new file: sm-file.cc

Message ID 1573867416-55618-36-git-send-email-dmalcolm@redhat.com
State New
Headers show
Series RFC: Add a static analysis framework to GCC | expand

Commit Message

David Malcolm Nov. 16, 2019, 1:23 a.m. UTC
This patch adds a state machine checker for stdio's FILE stream API.

gcc/ChangeLog:
	* analyzer/sm-file.cc: New file.
---
 gcc/analyzer/sm-file.cc | 338 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 338 insertions(+)
 create mode 100644 gcc/analyzer/sm-file.cc

Comments

Jeff Law Dec. 7, 2019, 3:15 p.m. UTC | #1
On Fri, 2019-11-15 at 20:23 -0500, David Malcolm wrote:
> This patch adds a state machine checker for stdio's FILE stream API.
> 
> gcc/ChangeLog:
> 	* analyzer/sm-file.cc: New file.
I note this seems somewhat incomplete -- which is fine given my
recommendation was to focus on the double-free analyzer.  The biggest
question is do we want to include this in the first iteration?  Perhaps
as an example that others can flesh out to capture the missing stuff
(like operations on released FD or file pointers?)

The similarities with double-free, use-after-free are significant.  But
I hesitate to suggest trying to generaize and merge them at this point.

jeff
diff mbox series

Patch

diff --git a/gcc/analyzer/sm-file.cc b/gcc/analyzer/sm-file.cc
new file mode 100644
index 0000000..4fc308c
--- /dev/null
+++ b/gcc/analyzer/sm-file.cc
@@ -0,0 +1,338 @@ 
+/* A state machine for detecting misuses of <stdio.h>'s FILE * API.
+   Copyright (C) 2019 Free Software Foundation, Inc.
+   Contributed by David Malcolm <dmalcolm@redhat.com>.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it
+under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#include "config.h"
+#include "gcc-plugin.h"
+#include "system.h"
+#include "coretypes.h"
+#include "tree.h"
+#include "gimple.h"
+#include "diagnostic-path.h"
+#include "diagnostic-metadata.h"
+#include "analyzer/analyzer.h"
+#include "analyzer/pending-diagnostic.h"
+#include "analyzer/sm.h"
+
+namespace {
+
+/* A state machine for detecting misuses of <stdio.h>'s FILE * API.  */
+
+class fileptr_state_machine : public state_machine
+{
+public:
+  fileptr_state_machine (logger *logger);
+
+  bool inherited_state_p () const FINAL OVERRIDE { return false; }
+
+  bool on_stmt (sm_context *sm_ctxt,
+		const supernode *node,
+		const gimple *stmt) const FINAL OVERRIDE;
+
+  void on_condition (sm_context *sm_ctxt,
+		     const supernode *node,
+		     const gimple *stmt,
+		     tree lhs,
+		     enum tree_code op,
+		     tree rhs) const FINAL OVERRIDE;
+
+  void on_leak (sm_context *sm_ctxt,
+		const supernode *node,
+		const gimple *stmt,
+		tree var,
+		state_machine::state_t state) const FINAL OVERRIDE;
+  bool can_purge_p (state_t s) const FINAL OVERRIDE;
+
+  /* Start state.  */
+  state_t m_start;
+
+  /* State for a FILE * returned from fopen that hasn't been checked for
+     NULL.
+     It could be an open stream, or could be NULL.  */
+  state_t m_unchecked;
+
+  /* State for a FILE * that's known to be NULL.  */
+  state_t m_null;
+
+  /* State for a FILE * that's known to be a non-NULL open stream.  */
+  state_t m_nonnull;
+
+  /* State for a FILE * that's had fclose called on it.  */
+  state_t m_closed;
+
+  /* Stop state, for a FILE * we don't want to track any more.  */
+  state_t m_stop;
+};
+
+////////////////////////////////////////////////////////////////////////////
+
+class file_diagnostic : public pending_diagnostic
+{
+public:
+  file_diagnostic (const fileptr_state_machine &sm, tree arg)
+  : m_sm (sm), m_arg (arg)
+  {}
+
+  bool subclass_equal_p (const pending_diagnostic &base_other) const OVERRIDE
+  {
+    return m_arg == ((const file_diagnostic &)base_other).m_arg;
+  }
+
+  label_text describe_state_change (const evdesc::state_change &change)
+    OVERRIDE
+  {
+    if (change.m_old_state == m_sm.m_start
+	&& change.m_new_state == m_sm.m_unchecked)
+      // TODO: verify that it's the fopen stmt, not a copy
+      return label_text::borrow ("opened here");
+    if (change.m_old_state == m_sm.m_unchecked
+	&& change.m_new_state == m_sm.m_nonnull)
+      return change.formatted_print ("assuming %qE is non-NULL",
+				     change.m_expr);
+    if (change.m_new_state == m_sm.m_null)
+      return change.formatted_print ("assuming %qE is NULL",
+				     change.m_expr);
+    return label_text ();
+  }
+
+protected:
+  const fileptr_state_machine &m_sm;
+  tree m_arg;
+};
+
+class double_fclose : public file_diagnostic
+{
+public:
+  double_fclose (const fileptr_state_machine &sm, tree arg)
+    : file_diagnostic (sm, arg)
+  {}
+
+  const char *get_kind () const FINAL OVERRIDE { return "double_fclose"; }
+
+  bool emit (rich_location *rich_loc) FINAL OVERRIDE
+  {
+    return warning_at (rich_loc, OPT_Wanalyzer_double_fclose,
+		       "double %<fclose%> of 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_first_fclose_event = change.m_event_id;
+	return change.formatted_print ("first %qs here", "fclose");
+      }
+    return file_diagnostic::describe_state_change (change);
+  }
+
+  label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE
+  {
+    if (m_first_fclose_event.known_p ())
+      return ev.formatted_print ("second %qs here; first %qs was at %@",
+				 "fclose", "fclose",
+				 &m_first_fclose_event);
+    return ev.formatted_print ("second %qs here", "fclose");
+  }
+
+private:
+  diagnostic_event_id_t m_first_fclose_event;
+};
+
+class file_leak : public file_diagnostic
+{
+public:
+  file_leak (const fileptr_state_machine &sm, tree arg)
+    : file_diagnostic (sm, arg)
+  {}
+
+  const char *get_kind () const FINAL OVERRIDE { return "file_leak"; }
+
+  bool emit (rich_location *rich_loc) FINAL OVERRIDE
+  {
+    diagnostic_metadata m;
+    /* CWE-775: "Missing Release of File Descriptor or Handle after
+       Effective Lifetime". */
+    m.add_cwe (775);
+    return warning_at (rich_loc, m, OPT_Wanalyzer_file_leak,
+		       "leak of FILE %qE",
+		       m_arg);
+  }
+
+  label_text describe_state_change (const evdesc::state_change &change)
+    FINAL OVERRIDE
+  {
+    if (change.m_new_state == m_sm.m_unchecked)
+      {
+	m_fopen_event = change.m_event_id;
+	return label_text::borrow ("opened here");
+      }
+    return file_diagnostic::describe_state_change (change);
+  }
+
+  label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE
+  {
+    if (m_fopen_event.known_p ())
+      return ev.formatted_print ("%qE leaks here; was opened at %@",
+				 ev.m_expr, &m_fopen_event);
+    else
+      return ev.formatted_print ("%qE leaks here", ev.m_expr);
+  }
+
+private:
+  diagnostic_event_id_t m_fopen_event;
+};
+
+////////////////////////////////////////////////////////////////////////////
+
+fileptr_state_machine::fileptr_state_machine (logger *logger)
+: state_machine ("file", logger)
+{
+  m_start = add_state ("start");
+  m_unchecked = add_state ("unchecked");
+  m_null = add_state ("null");
+  m_nonnull = add_state ("nonnull");
+  m_closed = add_state ("closed");
+  m_stop = add_state ("stop");
+}
+
+/* Implementation of state_machine::on_stmt vfunc for fileptr_state_machine.  */
+
+bool
+fileptr_state_machine::on_stmt (sm_context *sm_ctxt,
+				const supernode *node,
+				const gimple *stmt) const
+{
+  if (const gcall *call = dyn_cast <const gcall *> (stmt))
+    {
+      if (is_named_call_p (call, "fopen", 2))
+	{
+	  tree lhs = gimple_call_lhs (call);
+	  if (lhs)
+	    {
+	      lhs = sm_ctxt->get_readable_tree (lhs);
+	      sm_ctxt->on_transition (node, stmt, lhs, m_start, m_unchecked);
+	    }
+	  else
+	    {
+	      /* TODO: report leak.  */
+	    }
+	  return true;
+	}
+
+      if (is_named_call_p (call, "fclose", 1))
+	{
+	  tree arg = gimple_call_arg (call, 0);
+	  arg = sm_ctxt->get_readable_tree (arg);
+
+	  sm_ctxt->on_transition (node, stmt, arg, m_start, m_closed);
+
+	  // TODO: is it safe to call fclose (NULL) ?
+	  sm_ctxt->on_transition (node, stmt, arg, m_unchecked, m_closed);
+	  sm_ctxt->on_transition (node, stmt, arg, m_null, m_closed);
+
+	  sm_ctxt->on_transition (node, stmt , arg, m_nonnull, m_closed);
+
+	  sm_ctxt->warn_for_state (node, stmt, arg, m_closed,
+				   new double_fclose (*this, arg));
+	  sm_ctxt->on_transition (node, stmt, arg, m_closed, m_stop);
+	  return true;
+	}
+
+      // TODO: operations on closed file
+      // etc
+    }
+  return false;
+}
+
+/* Implementation of state_machine::on_condition vfunc for
+   fileptr_state_machine.
+   Potentially transition state 'unchecked' to 'nonnull' or to 'null'.  */
+
+void
+fileptr_state_machine::on_condition (sm_context *sm_ctxt,
+				     const supernode *node,
+				     const gimple *stmt,
+				     tree lhs,
+				     enum tree_code op,
+				     tree rhs) const
+{
+  if (!zerop (rhs))
+    return;
+
+  // TODO: has to be a FILE *, specifically
+  if (TREE_CODE (TREE_TYPE (lhs)) != POINTER_TYPE)
+    return;
+
+  // TODO: has to be a FILE *, specifically
+  if (TREE_CODE (TREE_TYPE (rhs)) != POINTER_TYPE)
+    return;
+
+  if (op == NE_EXPR)
+    {
+      log ("got 'ARG != 0' match");
+      sm_ctxt->on_transition (node, stmt,
+			      lhs, m_unchecked, m_nonnull);
+    }
+  else if (op == EQ_EXPR)
+    {
+      log ("got 'ARG == 0' match");
+      sm_ctxt->on_transition (node, stmt,
+			      lhs, m_unchecked, m_null);
+    }
+}
+
+/* Implementation of state_machine::on_leak vfunc for
+   fileptr_state_machine.
+   Complain about leaks of FILE * in state 'unchecked' and 'nonnull'.  */
+
+void
+fileptr_state_machine::on_leak (sm_context *sm_ctxt,
+				const supernode *node,
+				const gimple *stmt,
+				tree var,
+				state_machine::state_t state ATTRIBUTE_UNUSED)
+  const
+{
+  sm_ctxt->warn_for_state (node, stmt, var, m_unchecked,
+			   new file_leak (*this, var));
+  sm_ctxt->warn_for_state (node, stmt, var, m_nonnull,
+			   new file_leak (*this, var));
+}
+
+/* Implementation of state_machine::can_purge_p vfunc for fileptr_state_machine.
+   Don't allow purging of pointers in state 'unchecked' or 'nonnull'
+   (to avoid false leak reports).  */
+
+bool
+fileptr_state_machine::can_purge_p (state_t s) const
+{
+  return s != m_unchecked && s != m_nonnull;
+}
+
+} // anonymous namespace
+
+/* Internal interface to this file. */
+
+state_machine *
+make_fileptr_state_machine (logger *logger)
+{
+  return new fileptr_state_machine (logger);
+}