From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from paperclip.tbsaunde.org (tbsaunde.org [66.228.47.254]) by sourceware.org (Postfix) with ESMTP id DF404384F02A for ; Thu, 1 Jul 2021 10:16:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DF404384F02A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=tbsaunde.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tbsaunde.org Received: from rag (pool-108-24-42-131.cmdnnj.fios.verizon.net [108.24.42.131]) by paperclip.tbsaunde.org (Postfix) with ESMTPSA id 961D2C0A0; Thu, 1 Jul 2021 10:16:26 +0000 (UTC) Date: Thu, 1 Jul 2021 06:16:25 -0400 From: Trevor Saunders To: David Malcolm Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH 2/4] allow poisoning input_location in ranges it should not be used Message-ID: References: <20210630053529.26581-1-tbsaunde@tbsaunde.org> <20210630053529.26581-2-tbsaunde@tbsaunde.org> <907daf69d72fedce3dd9ee8a9dccc59d7d22a08a.camel@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <907daf69d72fedce3dd9ee8a9dccc59d7d22a08a.camel@redhat.com> X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, 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: Thu, 01 Jul 2021 10:16:28 -0000 On Wed, Jun 30, 2021 at 11:13:23AM -0400, David Malcolm wrote: > On Wed, 2021-06-30 at 01:35 -0400, Trevor Saunders wrote: > > This makes it possible to assert if input_location is used during the > > lifetime > > of a scope.  This will allow us to find places that currently use it > > within a > > function and its callees, or prevent adding uses within the lifetime > > of a > > function after all existing uses are removed. > > > > bootstrapped and regtested on x86_64-linux-gnu, ok? > > > > Trev > > [...snip...] > > > diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c > > index d58586f2526..3f68d1d79eb 100644 > > --- a/gcc/diagnostic.c > > +++ b/gcc/diagnostic.c > > @@ -1835,7 +1835,7 @@ internal_error (const char *gmsgid, ...) > >    auto_diagnostic_group d; > >    va_list ap; > >    va_start (ap, gmsgid); > > -  rich_location richloc (line_table, input_location); > > +  rich_location richloc (line_table, UNKNOWN_LOCATION); > >    diagnostic_impl (&richloc, NULL, -1, gmsgid, &ap, DK_ICE); > >    va_end (ap); > >   > > I actually make use of this in the analyzer: the analyzer sets > input_location to stmt->location when analyzing a given stmt - that > way, if the analyzer ICEs, the ICE is shown at the code construct that > crashed the analyzer. > > This behavior is useful to me, and would be lost with the proposed > patch. I made this change because otherwise if the compiler ICE's while access to input_location is blocked we end up infinitely recursing complaining we can't access it while trying to say where the last error was. I was nervous about the change before, and now I agree we need something else. > Is there a better way of doing what I'm doing? > > Is the long-term goal of the patch kit to reduce our reliance on global > variables? Are we ultimately still going to need a variable for "where > to show the ICE if gcc crashes"? (perhaps stashing it in the > diagnostic_context???) Yes, the goal is ultimately removal of global state, however I'm not really ure what the better approach to your problem is, after all even moving it to the diagnostic context is sort of a global state, and sort of dupplicates input_location. That said it is somewhat more constrained, so if it removes usage of input_location perhaps its worthwhile? Sorry I'm not yet sure what to propose here. Trev > > Hope this is constructive > Dave >