From 08b01551659ff2d9d49e13efb1adb79df41328a2 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Mon, 25 Mar 2019 14:39:18 -0400
Subject: [PATCH 141/161] FIXME: initial implementation of free of alloca
 warning

---
 gcc/analyzer/hardcoded-state-machine-malloc.cc | 65 ++++++++++++++++++++++++++
 gcc/analyzer/hardcoded-state-machine.h         |  2 +
 gcc/testsuite/gcc.dg/analyzer/malloc-1.c       | 14 ++++++
 gcc/testsuite/gcc.dg/analyzer/malloc-ipa-8.c   |  2 +-
 4 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/gcc/analyzer/hardcoded-state-machine-malloc.cc b/gcc/analyzer/hardcoded-state-machine-malloc.cc
index 3124eba..6b67845 100644
--- a/gcc/analyzer/hardcoded-state-machine-malloc.cc
+++ b/gcc/analyzer/hardcoded-state-machine-malloc.cc
@@ -219,6 +219,46 @@ public:
   }
 };
 
+class free_of_non_heap : public malloc_diagnostic
+{
+public:
+  free_of_non_heap (const malloc_state_machine &sm, tree arg)
+  : malloc_diagnostic (sm, arg)
+  {}
+
+  void emit (rich_location *rich_loc) FINAL OVERRIDE
+  {
+    auto_diagnostic_group d;
+    metadata m;
+    m.add_cwe (590); /* CWE-590: Free of Memory not on the Heap.  */
+    warning_at (rich_loc, m,
+		"free of %qE which points to memory not on the heap", m_arg);
+    // FIXME: ideally would distinguish the cases in the error message
+  }
+
+  label_text describe_critical_state_change
+    (tree expr,
+     state_machine::state_t old_state ATTRIBUTE_UNUSED,
+     state_machine::state_t new_state,
+     event_id_t event_id)
+    FINAL OVERRIDE
+  {
+    return label_text::borrow ("pointer is from here"); // TODO
+  }
+
+  /* FIXME.  */
+  label_text describe_final_event
+    (/*diagnostic_context *ctxt, */
+     tree expr ATTRIBUTE_UNUSED,
+     state_machine::state_t state ATTRIBUTE_UNUSED)
+  {
+    pretty_printer pp;
+    pp_printf (&pp, "call to %qs here", "free");
+    return label_text::take (xstrdup (pp_formatted_text (&pp)));
+    // TODO: using appropriate colorization etc
+  }
+};
+
 ////////////////////////////////////////////////////////////////////////////
 
 malloc_state_machine::malloc_state_machine (logger *logger)
@@ -230,6 +270,7 @@ malloc_state_machine::malloc_state_machine (logger *logger)
   m_null = add_state ("null");
   m_nonnull = add_state ("nonnull");
   m_free = add_state ("free");
+  m_non_heap = add_state ("non-heap");
   m_stop = add_state ("stop");
 }
 
@@ -257,6 +298,14 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt,
 	    }
 	}
 
+      if (is_named_call_p (call, "alloca", 1)
+	  || is_named_call_p (call, "__builtin_alloca", 1))
+	{
+	  tree lhs = gimple_call_lhs (call);
+	  if (lhs)
+	    sm_ctxt->on_transition (node, stmt, lhs, m_start, m_non_heap);
+	}
+
       if (is_named_call_p (call, "free", 1))
 	{
 	  tree arg = gimple_call_arg (call, 0);
@@ -275,6 +324,11 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt,
 	  sm_ctxt->warn_for_state (node, stmt, arg, m_free,
 				   new double_free (*this, arg));
 	  sm_ctxt->on_transition (node, stmt, arg, m_free, m_stop);
+
+	  /* non-heap -> stop, with warning.  */
+	  sm_ctxt->warn_for_state (node, stmt, arg, m_non_heap,
+				   new free_of_non_heap (*this, arg));
+	  sm_ctxt->on_transition (node, stmt, arg, m_non_heap, m_stop);
 	}
     }
 
@@ -291,6 +345,17 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt,
 	}
     }
 
+  if (const gassign *assign_stmt = dyn_cast <const gassign *> (stmt))
+    {
+      enum tree_code op = gimple_assign_rhs_code (assign_stmt);
+      if (op == ADDR_EXPR)
+	{
+	  tree lhs = gimple_assign_lhs (assign_stmt);
+	  if (lhs)
+	    sm_ctxt->on_transition (node, stmt, lhs, m_start, m_non_heap);
+	}
+    }
+
   // TODO: dereferences
   for (unsigned i = 0; i < gimple_num_ops (stmt); i++)
     {
diff --git a/gcc/analyzer/hardcoded-state-machine.h b/gcc/analyzer/hardcoded-state-machine.h
index 09f6ec1..f9b9c9d 100644
--- a/gcc/analyzer/hardcoded-state-machine.h
+++ b/gcc/analyzer/hardcoded-state-machine.h
@@ -240,6 +240,8 @@ public:
   state_t m_null;
   state_t m_nonnull;
   state_t m_free;
+  state_t m_non_heap; // FIXME: or should this be a different state machine?
+  // or do we need child values etc?
   state_t m_stop;
 };
 
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-1.c b/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
index 180c027..5aa93f7 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
@@ -251,3 +251,17 @@ int *test_23 (int n)
   return ptr;
 }
 
+int test_24 (void)
+{
+  void *ptr = alloca (sizeof (int)); /* { dg-message "pointer is from here" } */
+  free (ptr); /* { dg-warning "free of 'ptr' which points to memory not on the heap \\\[CWE-590\\\]" } */
+  /* TODO: more precise messages here.  */
+}
+
+int test_25 (void)
+{
+  char tmp[100];
+  void *p = tmp; /* { dg-message "pointer is from here" } */
+  free (p); /* { dg-warning "free of 'p' which points to memory not on the heap \\\[CWE-590\\\]" } */
+  /* TODO: more precise messages here.  */
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-8.c b/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-8.c
index 3bd9a98..2b7364c 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-8.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-8.c
@@ -16,7 +16,7 @@ void wrapped_free (void *ptr) /* { dg-warning "summary: add: 'ptr': 'start' -> '
 /* { dg-message "\\(10\\) entry to 'wrapped_free'" "" { target *-*-* } .-3 } */
 /* { dg-message "\\(17\\) entry to 'wrapped_free'" "" { target *-*-* } .-4 } */
 {
-  free (ptr); /* { dg-warning "double-free of 'ptr' \\\[CWE-415\\]" } */
+  free (ptr); /* { dg-warning "double-free of 'ptr' \\\[CWE-415\\\]" } */
   /* { dg-message "\\(11\\) first 'free' here" "" { target *-*-* } .-1 } */
   /* { dg-message "\\(12\\) first 'free' here" "" { target *-*-* } .-2 } */
   /* FIXME: unchecked -> free is correct, but start -> free should be pruned
-- 
1.8.5.3

