This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] ipa-fnsummary.c: fix use-after-free crash (PR jit/82826)


PR jit/82826 reports a crash when running jit.dg/test-benchmark.c,
introduced by r254140
(aka "Extend ipa-pure-const pass to propagate malloc attribute.")

I see the crash on the 28th of 400 in-process iterations of the
compiler; on turning on GCC_JIT_BOOL_OPTION_SELFCHECK_GC, it shows up
on the 2nd iteration.

The root cause is that in one in-process invocation of the compiler we
have mismatching calls to ipa_fn_summary_alloc/ipa_free_fn_summary.

The sequence of calls is:

1st in-process iteration:
  (1): ipa_fn_summary_alloc
    (called indirectly by pass_local_fn_summary::execute)

  (2): ipa_free_fn_summary
    (called by pass_ipa_free_fn_summary::execute)

  (3): ipa_fn_summary_alloc

...where (3) is called thusly:

  (gdb) bt
  #0  ipa_fn_summary_alloc () at ../../src/gcc/ipa-fnsummary.c:533
  #1  0x00007ffff6616788 in ipa_fn_summary_generate () at ../../src/gcc/ipa-fnsummary.c:3184
  #2  0x00007ffff679ebe4 in execute_ipa_summary_passes (ipa_pass=0x656be0) at ../../src/gcc/passes.c:2200
  #3  0x00007ffff6359c2c in ipa_passes () at ../../src/gcc/cgraphunit.c:2448
  #4  0x00007ffff635a095 in symbol_table::compile (this=0x7fffef8ed100) at ../../src/gcc/cgraphunit.c:2558
  #5  0x00007ffff635a4e2 in symbol_table::finalize_compilation_unit (this=0x7fffef8ed100) at ../../src/gcc/cgraphunit.c:2716
  #6  0x00007ffff68ab25d in compile_file () at ../../src/gcc/toplev.c:481
  #7  0x00007ffff68ae3e1 in do_compile () at ../../src/gcc/toplev.c:2063
  #8  0x00007ffff68ae758 in toplev::main (this=0x7fffffffdf8e, argc=12, argv=0x61f6c8) at ../../src/gcc/toplev.c:2198
  (etc)

and so we have an "alloc" that's not matched by a free.

Hence the global "ipa_fn_summaries" is left non-NULL, a
GTY-marked object, with a pointer to the GC-heap-allocated
"symtab".  There's no GTY marking for the symtab reference
in the function_summary <T *>; it has GTY((user)), and the
hand-written functions don't visit the symtab.

On the next in-process iteration, the would-be alloc encounters this
conditional:

2396	  if (!ipa_fn_summaries)
2397	    ipa_fn_summary_alloc ();

and hence doesn't re-allocate.

The global "ipa_fn_summaries" thus contains a pointer to last iteration's
"symtab", leading to unpredictable bugs due to there being *two* symbol
tables (one for the previous iteration, one for the current iteration).
At some point the previous symtab will be collected from "under"
ipa_fn_summaries, leading to a use-after-free crash e.g. here in
function_summary<ipa_fn_summary*>::release:
67	    m_symtab->remove_cgraph_insertion_hook (m_symtab_insertion_hook);

due to m_symtab being GC-allocated but with nothing keeping it alive,
and thus being (hopefully) poisoned by GC, or perhaps with the memory
being reused for something else.

This isn't an issue for the regular compiler, but when libgccjit
reruns in-process, the mismatched alloc/cleanups can lead to use-after-free
after the GC has run.

This patch fixes the issue in the least invasive way I could, by explicitly
ensuring that the cleanup happens in toplev::finalize (the jit's hook for
doing such cleanups); merely fixing the GTY issue wouldn't fix the issue of
accidentally having two symtabs.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
OK for trunk?

gcc/ChangeLog:
	PR jit/82826
	* ipa-fnsummary.c (ipa_fnsummary_c_finalize): New function.
	* ipa-fnsummary.h (ipa_fnsummary_c_finalize): New decl.
	* toplev.c: Include "ipa-fnsummary.h".
	(toplev::finalize): Call ipa_fnsummary_c_finalize.
---
 gcc/ipa-fnsummary.c | 9 +++++++++
 gcc/ipa-fnsummary.h | 2 ++
 gcc/toplev.c        | 2 ++
 3 files changed, 13 insertions(+)

diff --git a/gcc/ipa-fnsummary.c b/gcc/ipa-fnsummary.c
index f71338e..20eec2a 100644
--- a/gcc/ipa-fnsummary.c
+++ b/gcc/ipa-fnsummary.c
@@ -3618,3 +3618,12 @@ make_pass_ipa_fn_summary (gcc::context *ctxt)
 {
   return new pass_ipa_fn_summary (ctxt);
 }
+
+/* Reset all state within ipa-fnsummary.c so that we can rerun the compiler
+   within the same process.  For use by toplev::finalize.  */
+
+void
+ipa_fnsummary_c_finalize (void)
+{
+  ipa_free_fn_summary ();
+}
diff --git a/gcc/ipa-fnsummary.h b/gcc/ipa-fnsummary.h
index a794bd0..b345bbc 100644
--- a/gcc/ipa-fnsummary.h
+++ b/gcc/ipa-fnsummary.h
@@ -266,4 +266,6 @@ void estimate_node_size_and_time (struct cgraph_node *node,
 				  vec<inline_param_summary>
 				  inline_param_summary);
 
+void ipa_fnsummary_c_finalize (void);
+
 #endif /* GCC_IPA_FNSUMMARY_H */
diff --git a/gcc/toplev.c b/gcc/toplev.c
index f68de3b..590ab58 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -83,6 +83,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "edit-context.h"
 #include "tree-pass.h"
 #include "dumpfile.h"
+#include "ipa-fnsummary.h"
 
 #if defined(DBX_DEBUGGING_INFO) || defined(XCOFF_DEBUGGING_INFO)
 #include "dbxout.h"
@@ -2238,6 +2239,7 @@ toplev::finalize (void)
 
   /* Needs to be called before cgraph_c_finalize since it uses symtab.  */
   ipa_reference_c_finalize ();
+  ipa_fnsummary_c_finalize ();
 
   cgraph_c_finalize ();
   cgraphunit_c_finalize ();
-- 
1.8.5.3


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]