Patchwork [jit] Fix global state init_p within ipa_init

login
register
mail settings
Submitter David Malcolm
Date March 10, 2014, 8:55 p.m.
Message ID <1394484912-27797-1-git-send-email-dmalcolm@redhat.com>
Download mbox | patch
Permalink /patch/328818/
State New
Headers show

Comments

David Malcolm - March 10, 2014, 8:55 p.m.
Committed to branch dmalcolm/jit:

Attempting to repeatedly compile with optimizations on within one
process would, under some circumstances, lead to a segfault the second
time in, here:

 #0  splay_tree_delete (sp=0x0) at ../../src/libiberty/splay-tree.c:353
 #1  0x00007ffff77f104f in propagate () at ../../src/gcc/ipa-reference.c:878
 #2  0x00007ffff77f1d23 in (anonymous namespace)::pass_ipa_reference::execute
	(this=0x72f5d0) at ../../src/gcc/ipa-reference.c:1191
 #3  0x00007ffff6fc5237 in execute_one_pass (pass=0x72f5d0) at
	../../src/gcc/passes.c:2216
 #4  0x00007ffff6fc6044 in execute_ipa_pass_list (pass=0x72f5d0) at
	../../src/gcc/passes.c:2580
 #5  0x00007ffff6c93d5f in ipa_passes () at ../../src/gcc/cgraphunit.c:2027

The crash occurred here:
 #1  0x00007ffff77f104f in propagate () at ../../src/gcc/ipa-reference.c:878
 878	    splay_tree_delete (reference_vars_to_consider);
 (gdb) p reference_vars_to_consider
 $10 = (splay_tree) 0x0

This tended to occur when working with functions intended to be inlined.

Debuggging indicated that the root-cause was the "init_p" flag within
ipa_init: it is static, presumably to allow idempotent initialization.
However, there is also cleanup, so that the 2nd time in, ipa-reference.c
is working with post-clean up state, failing to rebuild its data structures
(e.g. a NULL for "reference_vars_to_consider").

Hence this patches changes ipa initialization from being idempotent within
the lifetime of the process to being idempotent within one invocation of
toplev_main, by moving it from static in function scope to *file* scope,
adding a reset of the flag to toplev_finalize.

[That said, I *think* ipa_init is only called at most once per invocation
of toplev_main.]

Doing so allows the optimizer to inline functions in repeated invocations
of the compiler within one process.
[...provided debuginfo generation is disabled.  In this latter case, with
GCC_JIT_BOOL_OPTION_DEBUGINFO, the equivalent of -g, I'm currently running
into what I take to be a separate segfault in dwarf2out.c the second time
in.  I haven't tracked this other one down yet.]

gcc/
	* ipa-reference.c (ipa_init): Move static bool init_p from here
	to...
	(ipa_init_p): New file-scope variable, so that it can be reset
	when repeatedly invoking the compiler within one process by...
	(ipa_reference_c_finalize): New function.
	* ipa-reference.h (ipa_reference_c_finalize): New.
	* toplev.c (toplev_finalize): Invoke new function
	ipa_reference_c_finalize.
---
 gcc/ChangeLog.jit   | 11 +++++++++++
 gcc/ipa-reference.c | 13 +++++++++----
 gcc/ipa-reference.h |  1 +
 gcc/toplev.c        |  2 ++
 4 files changed, 23 insertions(+), 4 deletions(-)

Patch

diff --git a/gcc/ChangeLog.jit b/gcc/ChangeLog.jit
index 555fc76..0e35180 100644
--- a/gcc/ChangeLog.jit
+++ b/gcc/ChangeLog.jit
@@ -1,3 +1,14 @@ 
+2014-03-10  David Malcolm  <dmalcolm@redhat.com>
+
+	* ipa-reference.c (ipa_init): Move static bool init_p from here
+	to...
+	(ipa_init_p): New file-scope variable, so that it can be reset
+	when repeatedly invoking the compiler within one process by...
+	(ipa_reference_c_finalize): New function.
+	* ipa-reference.h (ipa_reference_c_finalize): New.
+	* toplev.c (toplev_finalize): Invoke new function
+	ipa_reference_c_finalize.
+
 2014-01-24  David Malcolm  <dmalcolm@redhat.com>
 
 	* timevar.def: Replace TV_CLIENT_CALLBACK with TV_JIT_REPLAY.
diff --git a/gcc/ipa-reference.c b/gcc/ipa-reference.c
index b3e137e..2a6671f 100644
--- a/gcc/ipa-reference.c
+++ b/gcc/ipa-reference.c
@@ -403,17 +403,17 @@  propagate_bits (ipa_reference_global_vars_info_t x_global, struct cgraph_node *x
     }
 }
 
+static bool ipa_init_p = false;
+
 /* The init routine for analyzing global static variable usage.  See
    comments at top for description.  */
 static void
 ipa_init (void)
 {
-  static bool init_p = false;
-
-  if (init_p)
+  if (ipa_init_p)
     return;
 
-  init_p = true;
+  ipa_init_p = true;
 
   if (dump_file)
     reference_vars_to_consider = splay_tree_new (splay_tree_compare_ints, 0, 0);
@@ -1199,3 +1199,8 @@  make_pass_ipa_reference (gcc::context *ctxt)
 {
   return new pass_ipa_reference (ctxt);
 }
+
+void ipa_reference_c_finalize (void)
+{
+  ipa_init_p = false;
+}
diff --git a/gcc/ipa-reference.h b/gcc/ipa-reference.h
index 317ebb0..fbcf6fa 100644
--- a/gcc/ipa-reference.h
+++ b/gcc/ipa-reference.h
@@ -26,6 +26,7 @@  along with GCC; see the file COPYING3.  If not see
 /* In ipa-reference.c  */
 bitmap ipa_reference_get_not_read_global (struct cgraph_node *fn);
 bitmap ipa_reference_get_not_written_global (struct cgraph_node *fn);
+void ipa_reference_c_finalize (void);
 
 #endif  /* GCC_IPA_REFERENCE_H  */
 
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 2418f7b..9de1c2d 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -78,6 +78,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "context.h"
 #include "pass_manager.h"
 #include "dwarf2out.h"
+#include "ipa-reference.h"
 
 #if defined(DBX_DEBUGGING_INFO) || defined(XCOFF_DEBUGGING_INFO)
 #include "dbxout.h"
@@ -2000,6 +2001,7 @@  void toplev_finalize (void)
   cgraphunit_c_finalize ();
   dwarf2out_c_finalize ();
   ipa_c_finalize ();
+  ipa_reference_c_finalize ();
   predict_c_finalize ();
   symtab_c_finalize ();
   varpool_c_finalize ();