From b2ae5dfaabb515879c46c23b2aa62bbdd328c646 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Wed, 12 Nov 2014 11:34:50 -0500
Subject: [PATCH] Add a RAII class for marking scopes that assume that the GC
 won't run

We make assumptions in the codebase about when the GC can run, and when
it can't (e.g. within numerous passes) but these aren't captured in a way
that's verifiable.

This patch introduces a new RAII-style class auto_assert_no_gc, for
marking regions of code where we assume that a GC can't happen, together
with an assert within ggc_collect to verify that we're not within such
a region.

This lets us both
(A) document code regions where a GC must not happen and
(B) verify in a checked build that these assumptions hold

The patch also adds an example of using the class, to the JIT.

It all compiles away in a release build.

Bootstrap&regrtest in progress.

OK for trunk assuming it passes?

gcc/ChangeLog:
	* ggc-page.c (auto_assert_no_gc::count): New variable.
	(ggc_collect): Assert that we're not within code regions that are
	assuming that the GC isn't going to run.
	* ggc.h (class auto_assert_no_gc): New class for RAII-style
	assertions that ggc_collect is not called.

gcc/jit/ChangeLog:
	* jit-playback.c (gcc::jit::playback::context::replay): Split out
	into...
	(gcc::jit::playback::context::replay_no_gc): ...new method, adding
	an instance of auto_assert_no_gc to mark that ggc_collect must not
	be called within here, and...
	(gcc::jit::playback::context::replay_could_gc): ...new method.
	* jit-playback.h (gcc::jit::playback::context::replay_no_gc): New
	method.
	(gcc::jit::playback::context::replay_could_gc): New method.
---
 gcc/ggc-page.c         | 12 ++++++++++++
 gcc/ggc.h              | 36 ++++++++++++++++++++++++++++++++++
 gcc/jit/jit-playback.c | 52 ++++++++++++++++++++++++++++++++++++++++----------
 gcc/jit/jit-playback.h |  3 +++
 4 files changed, 93 insertions(+), 10 deletions(-)

diff --git a/gcc/ggc-page.c b/gcc/ggc-page.c
index f55c4e9..2916192 100644
--- a/gcc/ggc-page.c
+++ b/gcc/ggc-page.c
@@ -2151,11 +2151,23 @@ validate_free_objects (void)
 #define validate_free_objects()
 #endif
 
+#ifdef ENABLE_CHECKING
+int auto_assert_no_gc::count = 0;
+#endif
+
 /* Top level mark-and-sweep routine.  */
 
 void
 ggc_collect (void)
 {
+  /* Ensure that we don't try to garbage-collect in a code region that
+     assumes the GC won't run (as guarded by instances of
+     class auto_assert_no_gc).
+
+     If this assertion fails, then ggc_collect was called in a
+     region that assumed no GC could occur.  */
+  gcc_checking_assert (auto_assert_no_gc::count == 0);
+
   /* Avoid frequent unnecessary work by skipping collection if the
      total allocations haven't expanded much since the last
      collection.  */
diff --git a/gcc/ggc.h b/gcc/ggc.h
index dc21520..1422208 100644
--- a/gcc/ggc.h
+++ b/gcc/ggc.h
@@ -363,4 +363,40 @@ gt_pch_nx (unsigned int)
 {
 }
 
+/* We don't attempt to track references to GC-allocated entities
+   that are on the stack, so the garbage collectior can only run at
+   specific times.
+
+   A RAII-style class for marking a scope that assumes GC doesn't happen,
+   so that you can put:
+
+      auto_assert_no_gc no_gc_here;
+
+   in a scope and have ggc_collect fail with an assertion if it's
+   called.  */
+
+class auto_assert_no_gc
+{
+ public:
+  auto_assert_no_gc ()
+  {
+#if defined ENABLE_CHECKING
+    count++;
 #endif
+  }
+
+  ~auto_assert_no_gc ()
+  {
+#if defined ENABLE_CHECKING
+    gcc_assert (count > 0);
+    count--;
+#endif
+  }
+
+ private:
+  static int count;
+  friend void ggc_collect (void);
+};
+
+
+#endif /* #ifndef GCC_GGC_H */
diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c
index 285a3ef..9db01d3 100644
--- a/gcc/jit/jit-playback.c
+++ b/gcc/jit/jit-playback.c
@@ -1715,14 +1715,34 @@ compile ()
 
 /* Top-level hook for playing back a recording context.
 
-   This plays back m_recording_ctxt, and, if no errors
-   occurred builds statement lists for and then postprocesses
-   every function in the result.  */
+   This is split into the part that assumes the GC can't run,
+   and the part where ggc_collect could be called.  */
 
 void
 playback::context::
 replay ()
 {
+  replay_no_gc ();
+
+  if (errors_occurred ())
+    return;
+
+  replay_could_gc ();
+}
+
+/* The first half of playback::context::replay, which assumes the GC can't
+   run.
+
+   This plays back m_recording_ctxt, and, if no errors
+   occurred builds statement lists for every function in the
+   result.  */
+
+void
+playback::context::
+replay_no_gc ()
+{
+  auto_assert_no_gc no_gc_here;
+
   /* Adapted from c-common.c:c_common_nodes_and_builtins.  */
   tree array_domain_type = build_index_type (size_int (200));
   m_char_array_type_node
@@ -1759,14 +1779,26 @@ replay ()
       FOR_EACH_VEC_ELT (m_functions, i, func)
 	func->build_stmt_list ();
 
-      /* No GC can have happened yet.  */
+    }
 
-      /* Postprocess the functions.  This could trigger GC.  */
-      FOR_EACH_VEC_ELT (m_functions, i, func)
-	{
-	  gcc_assert (func);
-	  func->postprocess ();
-	}
+  /* No GC can have happened yet.  */
+}
+
+/* The second half of playback::context::replay, in which ggc_collect can
+   be called.  It postprocesses every function.  */
+
+void
+playback::context::
+replay_could_gc ()
+{
+  int i;
+  function *func;
+
+  /* Postprocess the functions.  This could trigger GC.  */
+  FOR_EACH_VEC_ELT (m_functions, i, func)
+    {
+      gcc_assert (func);
+      func->postprocess ();
     }
 }
 
diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
index dcb19bf..63458bc 100644
--- a/gcc/jit/jit-playback.h
+++ b/gcc/jit/jit-playback.h
@@ -209,6 +209,9 @@ public:
   }
 
 private:
+  void replay_no_gc ();
+  void replay_could_gc ();
+
   void dump_generated_code ();
 
   rvalue *
-- 
1.8.5.3

