public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r14-4003] ggc, jit: forcibly clear GTY roots in jit
@ 2023-09-14 20:28 David Malcolm
  0 siblings, 0 replies; only message in thread
From: David Malcolm @ 2023-09-14 20:28 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:eaa8e8541349df5ea326afa861c97b70ebc47f6b

commit r14-4003-geaa8e8541349df5ea326afa861c97b70ebc47f6b
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Thu Sep 14 16:28:44 2023 -0400

    ggc, jit: forcibly clear GTY roots in jit
    
    As part of Antoyo's work on supporting LTO in rustc_codegen_gcc, he
    noticed an ICE inside libgccjit when compiling certain rust files.
    
    Debugging libgccjit showed that outdated information from a previous
    in-memory compile was referring to ad-hoc locations in the previous
    compile's line_table.
    
    The issue turned out to be the function decls in internal_fn_fnspec_array
    from the previous compile keeping alive the symtab nodes for these
    functions, and from this finding other functions in the previous
    compile, walking their CFGs, and finding ad-hoc data pointers in an edge
    with a location_t using ad-hoc data from the previous line_table
    instance, and thus a use-after-free ICE attempting to use this ad-hoc
    data.
    
    Previously in toplev::finalize we've fixed global state "piecemeal" by
    calling out to individual source_name_cc_finalize functions.  However,
    it occurred to me that we have run-time information on where the
    GTY-marked pointers are.
    
    Hence this patch takes something of a "big hammer" approach by adding a
    new ggc_common_finalize that walks the GC roots, zeroing all of the
    pointers.  I stepped through this in the debugger and observed that, in
    particular, this correctly zeroes the internal_fn_fnspec_array at the end
    of a libgccjit compile.  Antoyo reports that this fixes the ICE for him.
    Doing so uncovered an ICE with libgccjit in dwarf2cfi.cc due to reuse of
    global variables from the previous compile, which this patch also fixes.
    
    I noticed that in ggc_mark_roots when clearing deletable roots we only
    clear the initial element in each gcc_root_tab_t.  This looks like a
    latent bug to me, which the patch fixes.  That said, there don't seem to
    be any deletable roots where the number of elements != 1.
    
    gcc/ChangeLog:
            * dwarf2cfi.cc (dwarf2cfi_cc_finalize): New.
            * dwarf2out.h (dwarf2cfi_cc_finalize): New decl.
            * ggc-common.cc (ggc_mark_roots): Multiply by rti->nelt when
            clearing the deletable gcc_root_tab_t.
            (ggc_common_finalize): New.
            * ggc.h (ggc_common_finalize): New decl.
            * toplev.cc (toplev::finalize): Call dwarf2cfi_cc_finalize and
            ggc_common_finalize.
    
    Signed-off-by: David Malcolm <dmalcolm@redhat.com>

Diff:
---
 gcc/dwarf2cfi.cc  |  9 +++++++++
 gcc/dwarf2out.h   |  1 +
 gcc/ggc-common.cc | 23 ++++++++++++++++++++++-
 gcc/ggc.h         |  2 ++
 gcc/toplev.cc     |  3 +++
 5 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/gcc/dwarf2cfi.cc b/gcc/dwarf2cfi.cc
index ddc728f4ad0..f1777c0a4cf 100644
--- a/gcc/dwarf2cfi.cc
+++ b/gcc/dwarf2cfi.cc
@@ -3822,4 +3822,13 @@ make_pass_dwarf2_frame (gcc::context *ctxt)
   return new pass_dwarf2_frame (ctxt);
 }
 
+void dwarf2cfi_cc_finalize ()
+{
+  add_cfi_insn = NULL;
+  add_cfi_vec = NULL;
+  cur_trace = NULL;
+  cur_row = NULL;
+  cur_cfa = NULL;
+}
+
 #include "gt-dwarf2cfi.h"
diff --git a/gcc/dwarf2out.h b/gcc/dwarf2out.h
index 870b56a6a37..61a996050ff 100644
--- a/gcc/dwarf2out.h
+++ b/gcc/dwarf2out.h
@@ -419,6 +419,7 @@ struct fixed_point_type_info
     } scale_factor;
 };
 
+void dwarf2cfi_cc_finalize (void);
 void dwarf2out_cc_finalize (void);
 
 /* Some DWARF internals are exposed for the needs of DWARF-based debug
diff --git a/gcc/ggc-common.cc b/gcc/ggc-common.cc
index bed7a9d4d02..95803fa95a1 100644
--- a/gcc/ggc-common.cc
+++ b/gcc/ggc-common.cc
@@ -86,7 +86,7 @@ ggc_mark_roots (void)
 
   for (rt = gt_ggc_deletable_rtab; *rt; rt++)
     for (rti = *rt; rti->base != NULL; rti++)
-      memset (rti->base, 0, rti->stride);
+      memset (rti->base, 0, rti->stride * rti->nelt);
 
   for (rt = gt_ggc_rtab; *rt; rt++)
     ggc_mark_root_tab (*rt);
@@ -1293,3 +1293,24 @@ report_heap_memory_use ()
 	     SIZE_AMOUNT (MALLINFO_FN ().arena));
 #endif
 }
+
+/* Forcibly clear all GTY roots.  */
+
+void
+ggc_common_finalize ()
+{
+  const struct ggc_root_tab *const *rt;
+  const_ggc_root_tab_t rti;
+
+  for (rt = gt_ggc_deletable_rtab; *rt; rt++)
+    for (rti = *rt; rti->base != NULL; rti++)
+      memset (rti->base, 0, rti->stride * rti->nelt);
+
+  for (rt = gt_ggc_rtab; *rt; rt++)
+    for (rti = *rt; rti->base != NULL; rti++)
+      memset (rti->base, 0, rti->stride * rti->nelt);
+
+  for (rt = gt_pch_scalar_rtab; *rt; rt++)
+    for (rti = *rt; rti->base != NULL; rti++)
+      memset (rti->base, 0, rti->stride * rti->nelt);
+}
diff --git a/gcc/ggc.h b/gcc/ggc.h
index 34108e2f006..3280314f848 100644
--- a/gcc/ggc.h
+++ b/gcc/ggc.h
@@ -368,4 +368,6 @@ inline void gt_ggc_mx (unsigned long int) { }
 inline void gt_ggc_mx (long long int) { }
 inline void gt_ggc_mx (unsigned long long int) { }
 
+extern void ggc_common_finalize ();
+
 #endif
diff --git a/gcc/toplev.cc b/gcc/toplev.cc
index 6c1a6f443c1..db62e3e995e 100644
--- a/gcc/toplev.cc
+++ b/gcc/toplev.cc
@@ -2336,6 +2336,7 @@ toplev::finalize (void)
   cgraph_cc_finalize ();
   cgraphunit_cc_finalize ();
   symtab_thunks_cc_finalize ();
+  dwarf2cfi_cc_finalize ();
   dwarf2out_cc_finalize ();
   gcse_cc_finalize ();
   ipa_cp_cc_finalize ();
@@ -2350,6 +2351,8 @@ toplev::finalize (void)
   save_decoded_options = NULL;
   save_decoded_options_count = 0;
 
+  ggc_common_finalize ();
+
   /* Clean up the context (and pass_manager etc). */
   delete g;
   g = NULL;

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2023-09-14 20:28 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-14 20:28 [gcc r14-4003] ggc, jit: forcibly clear GTY roots in jit David Malcolm

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).