From c42df8fc0283c81849ebb5dabd1ac20eacda9fd7 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Thu, 17 Sep 2020 13:38:31 -0400
Subject: [PATCH 47/47] FIXME: more on plugin experiment

---
 .../gcc.dg/plugin/analyzer_gil_plugin.c       | 48 +++++++++++---
 gcc/testsuite/gcc.dg/plugin/gil-1.c           | 63 ++++++++++++++++++-
 2 files changed, 98 insertions(+), 13 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/plugin/analyzer_gil_plugin.c b/gcc/testsuite/gcc.dg/plugin/analyzer_gil_plugin.c
index 616bb30cfbd..c6032d6c297 100644
--- a/gcc/testsuite/gcc.dg/plugin/analyzer_gil_plugin.c
+++ b/gcc/testsuite/gcc.dg/plugin/analyzer_gil_plugin.c
@@ -86,6 +86,8 @@ public:
 
   bool can_purge_p (state_t s) const FINAL OVERRIDE;
 
+  location_t fixup_location (location_t loc) const FINAL OVERRIDE;
+
   void check_for_pyobject_usage_without_gil (sm_context *sm_ctxt,
 					     const supernode *node,
 					     const gimple *stmt,
@@ -218,8 +220,8 @@ class fncall_without_gil : public gil_diagnostic
 class pyobject_usage_without_gil : public gil_diagnostic
 {
  public:
-  pyobject_usage_without_gil (const gil_state_machine &sm)
-  : gil_diagnostic (sm)
+  pyobject_usage_without_gil (const gil_state_machine &sm, tree expr)
+  : gil_diagnostic (sm), m_expr (expr)
   {}
 
   const char *get_kind () const FINAL OVERRIDE
@@ -229,18 +231,24 @@ class pyobject_usage_without_gil : public gil_diagnostic
 
   bool subclass_equal_p (const pending_diagnostic &base_other) const OVERRIDE
   {
-    return true; // TODO
+    return same_tree_p (m_expr,
+			((const pyobject_usage_without_gil&)base_other).m_expr);
   }
 
   bool emit (rich_location *rich_loc) FINAL OVERRIDE
   {
-    // TODO
     auto_diagnostic_group d;
     return warning_at (rich_loc, 0, // FIXME
-		       "FIXME: pyobject_usage_without_gil");
+		       "use of PyObject %qE without the GIL", m_expr);
+  }
+
+  label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE
+  {
+    return ev.formatted_print ("PyObject %qE used here without the GIL", m_expr);
   }
 
  private:
+  tree m_expr;
 };
 
 /* gil_state_machine's ctor.  */
@@ -336,8 +344,9 @@ gil_state_machine::on_stmt (sm_context *sm_ctxt,
 			"PyEval_SaveThread");
 	      if (global_state == m_released_gil)
 		sm_ctxt->set_global_state (m_start);
-	      else
+	      else if (global_state == m_start)
 		{
+		  // FIXME: can this happen?
 		  sm_ctxt->warn (node, stmt, NULL_TREE,
 				 new double_restore_thread (*this, call));
 		  sm_ctxt->set_global_state (m_stop);
@@ -389,6 +398,27 @@ gil_state_machine::can_purge_p (state_t s ATTRIBUTE_UNUSED) const
   return true;
 }
 
+location_t
+gil_state_machine::fixup_location (location_t loc) const
+{
+  if (linemap_location_from_macro_expansion_p (line_table, loc))
+    // FIXME: only for some macros?
+    // FIXME: make this part of the diagnostic_manager logic?
+    loc = linemap_resolve_location (line_table, loc,
+				    LRK_MACRO_EXPANSION_POINT, NULL);
+  return loc;
+}
+
+static bool
+type_based_on_pyobject_p (tree type)
+{
+  // FIXME: also check for "subclasses"
+  if (TREE_CODE (type) == RECORD_TYPE)
+    //&& FIXME ("PyObject"))
+    return true;
+  return false;
+}
+
 void
 gil_state_machine::check_for_pyobject_usage_without_gil (sm_context *sm_ctxt,
 							 const supernode *node,
@@ -396,12 +426,10 @@ gil_state_machine::check_for_pyobject_usage_without_gil (sm_context *sm_ctxt,
 							 tree op) const
 {
   tree type = TREE_TYPE (op);
-  if (TREE_CODE (type) == RECORD_TYPE)
-    //&& FIXME ("PyObject"))
-    // FIXME: "subclasses"
+  if (type_based_on_pyobject_p (type))
     {
       sm_ctxt->warn (node, stmt, NULL_TREE,
-		     new pyobject_usage_without_gil (*this));
+		     new pyobject_usage_without_gil (*this, op));
       sm_ctxt->set_global_state (m_stop);
     }
   //inform (input_location, "got usage of PyObject"); // TODO
diff --git a/gcc/testsuite/gcc.dg/plugin/gil-1.c b/gcc/testsuite/gcc.dg/plugin/gil-1.c
index 0d25a612432..77351543eba 100644
--- a/gcc/testsuite/gcc.dg/plugin/gil-1.c
+++ b/gcc/testsuite/gcc.dg/plugin/gil-1.c
@@ -37,15 +37,24 @@ typedef struct _object {
     int ob_refcnt;
 } PyObject;
 
-static inline void Py_INCREF(PyObject *op)
+#define _PyObject_CAST(op) ((PyObject*)(op))
+
+extern void _Py_Dealloc(PyObject *);
+
+static inline void _Py_INCREF(PyObject *op)
 {
   op->ob_refcnt++;
 }
-static inline void Py_DECREF(PyObject *op)
+static inline void _Py_DECREF(PyObject *op)
 {
-  op->ob_refcnt--;
+  if (--op->ob_refcnt == 0) {
+    _Py_Dealloc(op);
+  }
 }
 
+#define Py_INCREF(op) _Py_INCREF(_PyObject_CAST(op))
+#define Py_DECREF(op) _Py_DECREF(_PyObject_CAST(op))
+
 void test_1 (void)
 {
   Py_BEGIN_ALLOW_THREADS
@@ -57,6 +66,7 @@ void test_2 (PyObject *obj)
   Py_BEGIN_ALLOW_THREADS
 
   Py_INCREF (obj); // FIXME!
+  Py_DECREF (obj); // FIXME!
 
   Py_END_ALLOW_THREADS
 }
@@ -70,7 +80,54 @@ void test_2 (PyObject *obj)
 void test_3 (PyObject *obj)
 {
   Py_BEGIN_ALLOW_THREADS
+
+  Py_BEGIN_ALLOW_THREADS
+  Py_END_ALLOW_THREADS
+
+  Py_END_ALLOW_THREADS
+}
+
+void test_4 (PyObject *obj)
+{
+  /* These aren't nested, so should be OK.  */
   Py_BEGIN_ALLOW_THREADS
   Py_END_ALLOW_THREADS
+
+  Py_BEGIN_ALLOW_THREADS
   Py_END_ALLOW_THREADS
 }
+
+/* Interprocedural example of erroneously nested usage.  */
+
+static void  __attribute__((noinline))
+called_by_test_5 (void)
+{
+  Py_BEGIN_ALLOW_THREADS
+  Py_END_ALLOW_THREADS
+}
+
+void test_5 (PyObject *obj)
+{
+  Py_BEGIN_ALLOW_THREADS
+  called_by_test_5 ();
+  Py_END_ALLOW_THREADS
+}
+
+/* Interprocedural example of bogusly using a PyObject outside of GIL.  */
+
+static void  __attribute__((noinline))
+called_by_test_6 (PyObject *obj)
+{
+  Py_INCREF (obj); // FIXME!
+  Py_DECREF (obj); // FIXME!
+}
+
+void test_6 (PyObject *obj)
+{
+  Py_BEGIN_ALLOW_THREADS
+  called_by_test_6 (obj);
+  Py_END_ALLOW_THREADS
+}
+
+/* TODO: use the macro usage location, not the macro def location
+   May require a hook for this.  */
-- 
2.26.2

