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 EE9D63858C1F; Tue, 12 Sep 2023 17:37:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org EE9D63858C1F 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=1694540223; cv=none; d=zohomail.com; s=zohoarc; b=Or1jYelNF1XGOSy4pZ4OutOnAvmi4gvCvb0urvOdHJuX4yV7DPJ0kqK69/pMLq/QOWO6N6pVDXigfQ6cB1T/z6Dk4nnj+8UGqxQ/oH6qMQWKJMDk4GrvpSoZg5RjN784nAGGsDgyM+Azw/kuPGQZODkwqq5ZvsXTg09XrjeXIDk= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1694540223; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:To; bh=smBPuMjKtfic/3jGPrt3p44164FZP2e0v6JepSU9i/0=; b=KIeIvUKD1sXKonLcqGAP45Jpu8lXh6W9whcyfFqzuanx2sEX/dQZ4bUuJ9PiZ8emuUuRkWgqo+GlQSqb4ezXJxuuJJN+CryjSZQ+1N3XE8iADU6eBs6M2bddcxkJwXYlisouqn5MXMRCYYTe9IivI9PpwUuNktKiGw0c8Chb38o= 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=1694540223; 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=smBPuMjKtfic/3jGPrt3p44164FZP2e0v6JepSU9i/0=; b=LaAWFnslkD4YVxNc+/fCEPuMnHWuqm4yMYuszmsfYiD9lHhjcm2m/O88AaI31GA9 zP62o3YGvQ2j/cDk9azUAqTgvvZYGrx+4+MvL3XCVCAPyO0MQfMHSJXaXEnCb6F/ThU YggpAK8/25cNxoaVIAeejmyEqHiQ9wUko6zB9sTA= Received: from [192.168.1.172] (38.87.11.6 [38.87.11.6]) by mx.zohomail.com with SMTPS id 1694540220986200.37204003855265; Tue, 12 Sep 2023 10:37:00 -0700 (PDT) Message-ID: <2e5a9a1f4714e7946b11f929ae2a7ae0786d9c4a.camel@zoho.com> Subject: Re: [PATCH] ggc, jit: forcibly clear GTY roots in jit From: Antoni Boucher To: David Malcolm Cc: gcc-patches@gcc.gnu.org, jit@gcc.gnu.org Date: Tue, 12 Sep 2023 13:36:59 -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: rr080112282786a6f61b95e2e8204709de0000116f62582146ca17f71e4f4d967d755b8f6ad0eae2737380efa0:zu080112262be1473dad32bab7c5aba815000045ece7af0f2073a84f5a4824e2f01b5bd1bd6db869dde764:rf08011231744918ee70d1d49289ba4d4a0000652c9e24d4d8725ddb87178a476015e0d0ae733408688f439c4539d1cdae43776681d3: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: In the mean time, here's a (Rust) reproducer for the issue: fn main() { for _ in 0..5 { let context =3D Context::default(); context.add_command_line_option("-flto"); context.set_optimization_level(OptimizationLevel::Aggressive); context.add_driver_option("-nostdlib"); let int_type =3D context.new_type::(); let function =3D context.new_function(None, FunctionType::Exported, int_type, &[], "main", false); let block =3D function.new_block("start"); let value =3D context.new_rvalue_from_int(int_type, 42); block.end_with_return(None, value); context.compile_to_file(OutputKind::Executable, "my_exe"); } } On Tue, 2023-09-12 at 12:00 -0400, Antoni Boucher via Jit wrote: > It seems to not be enough to fix the issue. > Let me find out what's missing from my patch. >=20 > 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 > > > 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 > > > 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_c= c_finalize): New decl. > > > =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 clearing the deletable gcc= _root_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_finali= ze): New decl. > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 * toplev.cc (toplev::final= ize): 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-base= d > > > 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 * r= ti->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 * r= ti->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 * r= ti->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 * r= ti->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 >=20