From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from sender4-pp-o91.zoho.com (sender4-pp-o91.zoho.com [136.143.188.91]) by sourceware.org (Postfix) with ESMTPS id F21763858D3C; Tue, 12 Sep 2023 16:00:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org F21763858D3C 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=1694534429; cv=none; d=zohomail.com; s=zohoarc; b=i1MxuCEv5Ncq4KYqhS/xy3qhxMUnvVS0sc4j1CwscJXr1yJ55sJy5n1e9VTu3Vz0TtnWz//aBy/Pq5tNiHjLNrMBM9LHLl0INfY0+zLm8rtFjbODBc8csLLo28WWKtIFlsr5K3PpXCjbHxGHSKkOBbMR0i6sT8GR/jCNJ8Gzwck= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1694534429; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:To; bh=Q+beyVgsCNr73OUJW2ig+JRkscKyodEF7T/ae6gZ1tY=; b=hw2tGkG8FA8UfeRi6flSFtP9UEXsqA4I9kzYgfb7Cu/aIB3FqLOvL9QOHzbfq73OaBlMJ9PAIPzeQIS5tcsC3dGpTsB7+CB1SWRtZsET1AVCSs7podEhpk8uI9S08Pb+Hc5JYAnb4Us+/wk5RIWBPV5F+pcA76fpqmYfUcVT7/I= 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=1694534429; 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=Q+beyVgsCNr73OUJW2ig+JRkscKyodEF7T/ae6gZ1tY=; b=Sje88rqh0XtZIggi9nTZF/WM7Hu8LeTcuV+YT7zga7ZiIYTBJEJPhsM/6f0xp6uO X1EcoaY2B54BaM1wYHLQJVqTN28nw4QKoS03xoUcsz0Zbf8fvMyom5QRJg1frp3EGMW /bX4+wgbYtPp2KPgn48MWQwT8UO/mhoyImx1D+0M= Received: from [192.168.1.172] (38.87.11.6 [38.87.11.6]) by mx.zohomail.com with SMTPS id 1694534428319114.23602587927871; Tue, 12 Sep 2023 09:00:28 -0700 (PDT) Message-ID: Subject: Re: [PATCH] ggc, jit: forcibly clear GTY roots in jit From: Antoni Boucher To: Richard Biener , David Malcolm Cc: gcc-patches@gcc.gnu.org, jit@gcc.gnu.org Date: Tue, 12 Sep 2023 12:00:26 -0400 In-Reply-To: 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: rr08011228259cca4ec3d6c0d1f4a185a6000078372671f36f93d3ad08bfbe60478b4b67f839e42d40c89938ad:zu08011226ccf57f0facccb654aed25b0c0000bd6cd4793d0ad8890e727ee4dabc47b330e4c3eade1a6d5a:rf0801123217fe5aeee8ce3a97c6b658bc00007141e41ecc1b997614b27fcf0b7cf3456f054c6ab7e121b62faf18417ddf9e858b4519d2:ZohoMail X-ZohoMailClient: External X-Spam-Status: No, score=-10.5 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: It seems to not be enough to fix the issue. Let me find out what's missing from my patch. On Tue, 2023-09-12 at 11:35 +0200, Richard Biener via Jit wrote: > On Wed, Sep 6, 2023 at 3:41=E2=80=AFPM David Malcolm via Gcc-patches > wrote: > >=20 > > 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 fo= r > > 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 > > seem 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 > OK. >=20 > Thanks, > Richard. >=20 > > Thanks > > Dave > >=20 > > gcc/ChangeLog: > > =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 * dwarf2out.h (dwarf2cfi_cc_= finalize): New decl. > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 * ggc-common.cc (ggc_mark_ro= ots): Multiply by rti->nelt > > when > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 clearing the deletable gcc_r= oot_tab_t. > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 (ggc_common_finalize): New. > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 * ggc.h (ggc_common_finalize= ): New decl. > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 * toplev.cc (toplev::finaliz= e): 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} > >=20 > > +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}; > >=20 > > +void dwarf2cfi_cc_finalize (void); > > =C2=A0void dwarf2out_cc_finalize (void); > >=20 > > =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) > >=20 > > =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); > >=20 > > =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) { } > >=20 > > +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; > >=20 > > +=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; > > -- > > 2.26.3 > >=20