From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id ED747385803D for ; Tue, 18 Jan 2022 08:36:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org ED747385803D Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 74CED21116; Tue, 18 Jan 2022 08:36:55 +0000 (UTC) Received: from murzim.suse.de (murzim.suse.de [10.160.4.192]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 58ACFA3B81; Tue, 18 Jan 2022 08:36:55 +0000 (UTC) Date: Tue, 18 Jan 2022 09:36:55 +0100 (CET) From: Richard Biener To: Martin Sebor cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] middle-end/101292 - invalid memory access with warning control In-Reply-To: <7a70a23c-e084-7fab-d532-0c7d38a71142@gmail.com> Message-ID: References: <7pq3onsr-r66r-s43o-7p4-9n5o5672q1q@fhfr.qr> <7a70a23c-e084-7fab-d532-0c7d38a71142@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 18 Jan 2022 08:37:02 -0000 On Mon, 17 Jan 2022, Martin Sebor wrote: > On 1/17/22 07:32, Richard Biener via Gcc-patches wrote: > > The warning control falls into the C++ trap of using a reference > > to old hashtable contents for a put operation which can end up > > re-allocating that before reading from the old freed referenced to > > source. Fixed by introducing a temporary. > > I think a better place to fix this and avoid the gotcha once and > for all is in the GCC hash_map: C++ containers are expected to > handle the insertion of own elements gracefully. I don't think that's reasonably possible if you consider T *a = map.get (X); T *b = map.get (Y); map.put (Z, *a); map.put (W, *b); the only way to "fix" it would be to change the API to not return by reference for get, remove get_or_insert (or change its API to also require passing the new value). Note the above shows that making 'put' take the value by value instead of by reference doesn't work either. IMHO the issue is that C++ doesn't make it obvious that 'put' gets a pointer to the old element (stupid references). Richard. > Martin > > > > > Bootstrap & regtest running on x86_64-unknown-linux-gnu. > > > > 2022-01-17 Richard Biener > > > > PR middle-end/101292 > > * diagnostic-spec.c (copy_warning): Make sure to not > > reference old hashtable content on possible resize. > > * warning-control.cc (copy_warning): Likewise. > > --- > > gcc/diagnostic-spec.c | 5 ++++- > > gcc/warning-control.cc | 3 ++- > > 2 files changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/gcc/diagnostic-spec.c b/gcc/diagnostic-spec.c > > index a8af229d677..4341ccfaae9 100644 > > --- a/gcc/diagnostic-spec.c > > +++ b/gcc/diagnostic-spec.c > > @@ -195,7 +195,10 @@ copy_warning (location_t to, location_t from) > > else > > { > > if (from_spec) > > - nowarn_map->put (to, *from_spec); > > + { > > + nowarn_spec_t tem = *from_spec; > > + nowarn_map->put (to, tem); > > + } > > else > > nowarn_map->remove (to); > > } > > diff --git a/gcc/warning-control.cc b/gcc/warning-control.cc > > index f9808bf4392..fa39ecab421 100644 > > --- a/gcc/warning-control.cc > > +++ b/gcc/warning-control.cc > > @@ -206,7 +206,8 @@ void copy_warning (ToType to, FromType from) > > gcc_assert (supp); > > > > gcc_checking_assert (nowarn_map); > > - nowarn_map->put (to_loc, *from_spec); > > + nowarn_spec_t tem = *from_spec; > > + nowarn_map->put (to_loc, tem); > > } > > else > > { > > -- Richard Biener SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)