From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from sender4-pp-o90.zoho.com (sender4-pp-o90.zoho.com [136.143.188.90]) by sourceware.org (Postfix) with ESMTPS id 75F063858401; Wed, 6 Sep 2023 13:57:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 75F063858401 Authentication-Results: sourceware.org; dmarc=pass (p=reject dis=none) header.from=zoho.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=zoho.com ARC-Seal: i=1; a=rsa-sha256; t=1694008649; cv=none; d=zohomail.com; s=zohoarc; b=M9OU/Y8Ejby6DsodbvlIFtBO57OD65/afZdqKxdhXBcGCMnRp8IHaw+Jzbbk6aggyOcncp4ZvRTEW7Hz1cCAAd6XpM+aStGoc5IqhGx31h3vYbUndkzBBSDVYnZwJ0yYmxzbC+fM3Bac3FMyb9MA7nF2D+m93uHrEWkrTsqKbCM= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1694008649; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:To; bh=YGXUlOA8+BUEhn8ouNoLNN9cY7gMnN0NLgziwohgEqE=; b=nd4ibYa+crHzwgsl3bxFSgslZYCxvTY+5WTiE5msSlg9NX2g35/yVjylo0LsfeIqDEdgsqX8gB7j3+5Q1T9cXtYMVYUQKcRwCbAl1lO6uzEmhJ+n8//66IUNoZ9YYbUhcM+7+0kzy9n2kBEQWHmc92duRJ3jFlDCA29KSW6+CX8= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass header.i=zoho.com; spf=pass smtp.mailfrom=bouanto@zoho.com; dmarc=pass header.from= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1694008649; s=zm2022; d=zoho.com; i=bouanto@zoho.com; h=Message-ID:Subject:Subject:From:From:To:To:Cc:Cc:Date:Date:In-Reply-To:References:Content-Type:Content-Transfer-Encoding:MIME-Version:Feedback-ID:Message-Id:Reply-To; bh=YGXUlOA8+BUEhn8ouNoLNN9cY7gMnN0NLgziwohgEqE=; b=F+cHHArTHirbcIIFeldn0k0S+Au04am7QblzS9rJB4H3TvZdwkrNkLbJ5JFI6AC0 0F47HGM2OxM4wakrZoksKsElaXxSKWf/A1e9jthMJEFVvH3xwhwiTEPoRvoZ95tDvtu tuN48PtR2+6t2Rx07kvgjrVYtQ3EZOwxFwSxUjAY= Received: from [192.168.1.172] (38.87.11.6 [38.87.11.6]) by mx.zohomail.com with SMTPS id 169400864717789.4349239319414; Wed, 6 Sep 2023 06:57:27 -0700 (PDT) Message-ID: <4fcf60744d0574ca9916cde02305a16535c31c45.camel@zoho.com> Subject: Re: [PATCH] ggc, jit: forcibly clear GTY roots in jit From: Antoni Boucher To: David Malcolm , gcc-patches@gcc.gnu.org, jit@gcc.gnu.org Cc: antoyo@gcc.gnu.org Date: Wed, 06 Sep 2023 09:57:25 -0400 In-Reply-To: <20230906134001.681629-1-dmalcolm@redhat.com> References: <20230906134001.681629-1-dmalcolm@redhat.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.48.4 MIME-Version: 1.0 Feedback-ID: rr08011228ff81d526b54f40db223494b9000094c72a8a3fe1bad99513c161666557346ca8e0d1220e301cdd78:zu080112269a985bf7c7912f54e707c3c800005030c787aea6a5c96d1e3362b3e769bab5403b799283a2d6:rf08011231f8941682cec599d6083f85f400000751f7beaa260d23fd4823295d7a16f28ac6055caa73359907861d8b5d2e1e00dd3c25:ZohoMail X-ZohoMailClient: External X-Spam-Status: No, score=-10.4 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi. I'll do another test to make sure this is enough since I tested with a few more finalize functions. Thanks a lot for finding this! On Wed, 2023-09-06 at 09:40 -0400, David Malcolm via Jit wrote: > As part of Antoyo's work on supporting LTO in rustc_codegen_gcc, he > noticed an ICE inside libgccjit when compiling certain rust files. >=20 > 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. >=20 > 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. >=20 > Previously in toplev::finalize we've fixed global state "piecemeal" > by > calling out to individual source_name_cc_finalize functions.=C2=A0 > However, > it occurred to me that we have run-time information on where the > GTY-marked pointers are. >=20 > 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.=C2=A0 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.=C2=A0 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. >=20 > I noticed that in ggc_mark_roots when clearing deletable roots we > only > clear the initial element in each gcc_root_tab_t.=C2=A0 This looks like a > latent bug to me, which the patch fixes.=C2=A0 That said, there don't see= m > to > be any deletable roots where the number of elements !=3D 1. >=20 > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. >=20 > OK for trunk? >=20 > Thanks > Dave >=20 > gcc/ChangeLog: > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* dwarf2cfi.cc (dwarf2cfi= _cc_finalize): New. > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* dwarf2out.h (dwarf2cfi_= cc_finalize): New decl. > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* ggc-common.cc (ggc_mark= _roots): Multiply by rti->nelt when > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0clearing the deletable gc= c_root_tab_t. > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0(ggc_common_finalize): Ne= w. > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* ggc.h (ggc_common_final= ize): New decl. > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* toplev.cc (toplev::fina= lize): Call dwarf2cfi_cc_finalize > and > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ggc_common_finalize. > --- > =C2=A0gcc/dwarf2cfi.cc=C2=A0 |=C2=A0 9 +++++++++ > =C2=A0gcc/dwarf2out.h=C2=A0=C2=A0 |=C2=A0 1 + > =C2=A0gcc/ggc-common.cc | 23 ++++++++++++++++++++++- > =C2=A0gcc/ggc.h=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0 2= ++ > =C2=A0gcc/toplev.cc=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0 3 +++ > =C2=A05 files changed, 37 insertions(+), 1 deletion(-) >=20 > diff --git a/gcc/dwarf2cfi.cc b/gcc/dwarf2cfi.cc > index ddc728f4ad00..f1777c0a4cf1 100644 > --- a/gcc/dwarf2cfi.cc > +++ b/gcc/dwarf2cfi.cc > @@ -3822,4 +3822,13 @@ make_pass_dwarf2_frame (gcc::context *ctxt) > =C2=A0=C2=A0 return new pass_dwarf2_frame (ctxt); > =C2=A0} > =C2=A0 > +void dwarf2cfi_cc_finalize () > +{ > +=C2=A0 add_cfi_insn =3D NULL; > +=C2=A0 add_cfi_vec =3D NULL; > +=C2=A0 cur_trace =3D NULL; > +=C2=A0 cur_row =3D NULL; > +=C2=A0 cur_cfa =3D NULL; > +} > + > =C2=A0#include "gt-dwarf2cfi.h" > diff --git a/gcc/dwarf2out.h b/gcc/dwarf2out.h > index 870b56a6a372..61a996050ff9 100644 > --- a/gcc/dwarf2out.h > +++ b/gcc/dwarf2out.h > @@ -419,6 +419,7 @@ struct fixed_point_type_info > =C2=A0=C2=A0=C2=A0=C2=A0 } scale_factor; > =C2=A0}; > =C2=A0 > +void dwarf2cfi_cc_finalize (void); > =C2=A0void dwarf2out_cc_finalize (void); > =C2=A0 > =C2=A0/* 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 bed7a9d4d021..95803fa95a17 100644 > --- a/gcc/ggc-common.cc > +++ b/gcc/ggc-common.cc > @@ -86,7 +86,7 @@ ggc_mark_roots (void) > =C2=A0 > =C2=A0=C2=A0 for (rt =3D gt_ggc_deletable_rtab; *rt; rt++) > =C2=A0=C2=A0=C2=A0=C2=A0 for (rti =3D *rt; rti->base !=3D NULL; rti++) > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 memset (rti->base, 0, rti->stride); > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 memset (rti->base, 0, rti->stride * rti->= nelt); > =C2=A0 > =C2=A0=C2=A0 for (rt =3D gt_ggc_rtab; *rt; rt++) > =C2=A0=C2=A0=C2=A0=C2=A0 ggc_mark_root_tab (*rt); > @@ -1293,3 +1293,24 @@ report_heap_memory_use () > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 = SIZE_AMOUNT (MALLINFO_FN ().arena)); > =C2=A0#endif > =C2=A0} > + > +/* Forcibly clear all GTY roots.=C2=A0 */ > + > +void > +ggc_common_finalize () > +{ > +=C2=A0 const struct ggc_root_tab *const *rt; > +=C2=A0 const_ggc_root_tab_t rti; > + > +=C2=A0 for (rt =3D gt_ggc_deletable_rtab; *rt; rt++) > +=C2=A0=C2=A0=C2=A0 for (rti =3D *rt; rti->base !=3D NULL; rti++) > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 memset (rti->base, 0, rti->stride * rti->= nelt); > + > +=C2=A0 for (rt =3D gt_ggc_rtab; *rt; rt++) > +=C2=A0=C2=A0=C2=A0 for (rti =3D *rt; rti->base !=3D NULL; rti++) > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 memset (rti->base, 0, rti->stride * rti->= nelt); > + > +=C2=A0 for (rt =3D gt_pch_scalar_rtab; *rt; rt++) > +=C2=A0=C2=A0=C2=A0 for (rti =3D *rt; rti->base !=3D NULL; rti++) > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 memset (rti->base, 0, rti->stride * rti->= nelt); > +} > diff --git a/gcc/ggc.h b/gcc/ggc.h > index 34108e2f0061..3280314f8481 100644 > --- a/gcc/ggc.h > +++ b/gcc/ggc.h > @@ -368,4 +368,6 @@ inline void gt_ggc_mx (unsigned long int) { } > =C2=A0inline void gt_ggc_mx (long long int) { } > =C2=A0inline void gt_ggc_mx (unsigned long long int) { } > =C2=A0 > +extern void ggc_common_finalize (); > + > =C2=A0#endif > diff --git a/gcc/toplev.cc b/gcc/toplev.cc > index 6c1a6f443c16..db62e3e995ec 100644 > --- a/gcc/toplev.cc > +++ b/gcc/toplev.cc > @@ -2336,6 +2336,7 @@ toplev::finalize (void) > =C2=A0=C2=A0 cgraph_cc_finalize (); > =C2=A0=C2=A0 cgraphunit_cc_finalize (); > =C2=A0=C2=A0 symtab_thunks_cc_finalize (); > +=C2=A0 dwarf2cfi_cc_finalize (); > =C2=A0=C2=A0 dwarf2out_cc_finalize (); > =C2=A0=C2=A0 gcse_cc_finalize (); > =C2=A0=C2=A0 ipa_cp_cc_finalize (); > @@ -2350,6 +2351,8 @@ toplev::finalize (void) > =C2=A0=C2=A0 save_decoded_options =3D NULL; > =C2=A0=C2=A0 save_decoded_options_count =3D 0; > =C2=A0 > +=C2=A0 ggc_common_finalize (); > + > =C2=A0=C2=A0 /* Clean up the context (and pass_manager etc). */ > =C2=A0=C2=A0 delete g; > =C2=A0=C2=A0 g =3D NULL;