From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.CeBiTec.Uni-Bielefeld.DE (smtp.CeBiTec.Uni-Bielefeld.DE [129.70.160.84]) by sourceware.org (Postfix) with ESMTPS id 235703858297 for ; Thu, 1 Feb 2024 12:24:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 235703858297 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=CeBiTec.Uni-Bielefeld.DE Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=cebitec.uni-bielefeld.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 235703858297 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=129.70.160.84 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706790270; cv=none; b=BQj2TgO3s9Cwb/Yjyl0sgIGVAoe5Cyzny0ydNxfrTb7FzVB/WR6voneG2EpOCfzw3T5Nj670Q/HA8Kt//qvfcTHPe/vQ2xGIM/rrdpxZH3KXGbNwL/WAv5cJYG1T7PZGzOgwA6FIeHyzpwCNzNs1G/KBDQOrVVnkbft4hz7xKoI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706790270; c=relaxed/simple; bh=6dRfIFndQgAva45lieJGmmxWCINOCv1qlhAWNfOJ3bY=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=A8AYlVSN4Br0/U0XNASXGmkB69iMOUcOH7M++bm4BO/SOCpLD6tzqiTZQN+s33C8gvTa5jBByizUAVUm6cqEtPn31Y3fvAyYPKSYLMLvD/DcJF87/xG7GguZ4gweEHqYxgFIO9RUI0mRUAkEOvxEW9gZ5zipAWwymR7G40XLqoc= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from localhost (localhost [127.0.0.1]) by smtp.CeBiTec.Uni-Bielefeld.DE (Postfix) with ESMTP id F3BC464061; Thu, 1 Feb 2024 13:24:26 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d= cebitec.uni-bielefeld.de; h=content-type:content-type :mime-version:user-agent:message-id:in-reply-to:date:date :references:subject:subject:from:from:received:received; s= 20200306; t=1706790262; bh=6dRfIFndQgAva45lieJGmmxWCINOCv1qlhAWN fOJ3bY=; b=eEZJL4+5j5QLjGyWwZeWvvBxyowDhOCjz2BT+RuGJ+Xej2cAJsCyZ 8SKyVQfm0zXLEVieW3nQbGsjQfYv97qmI3MfakWfgIidq/Kpkh+UclZPpvkAqYVF FC1PPxfXzQjHMaMIbTCEDUtQ1WcTNQjbBM8SwOZRnEcO2tC4+/3bU6ytBadlRSlJ DSsd/quJooKjZF7RkMKZfQb/8o5QNEtWJMH7290ZRiWcZ7NaA22SJVgTM2GI2nD5 WsO2uGM8lmhCyTIaT33Q55024cQN0UwSHVpBKOJJHzR4xjHXEwWFgi1A6vugUROT CWSOOZa09wOIpfyV4Uigefx0y7meqglYQ== X-Virus-Scanned: amavisd-new at cebitec.uni-bielefeld.de Received: from smtp.CeBiTec.Uni-Bielefeld.DE ([127.0.0.1]) by localhost (smtp.cebitec.uni-bielefeld.de [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id yrWcYuCiXtGG; Thu, 1 Feb 2024 13:24:22 +0100 (CET) Received: from manam.CeBiTec.Uni-Bielefeld.DE (p5085539e.dip0.t-ipconnect.de [80.133.83.158]) (Authenticated sender: ro) by smtp.CeBiTec.Uni-Bielefeld.DE (Postfix) with ESMTPSA id B0717641C4; Thu, 1 Feb 2024 13:24:22 +0100 (CET) From: Rainer Orth To: Lewis Hyatt Cc: Jason Merrill , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] c-family: Fix ICE with large column number after restoring a PCH [PR105608] References: <20231206015211.682650-1-lhyatt@gmail.com> <17e2d4b7-1dfe-4e0c-9fea-c5c115ffc66c@redhat.com> Date: Thu, 01 Feb 2024 13:24:22 +0100 In-Reply-To: (Lewis Hyatt's message of "Tue, 30 Jan 2024 21:49:25 -0500") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1.90 (usg-unix-v) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-3785.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,KAM_SHORT,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE 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 Lewis, > On Fri, Jan 26, 2024 at 04:16:54PM -0500, Jason Merrill wrote: >> On 12/5/23 20:52, Lewis Hyatt wrote: >> > Hello- >> > >> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105608 >> > >> > There are two related issues here really, a regression since GCC 11 where we >> > can ICE after restoring a PCH, and a deeper issue with bogus locations >> > assigned to macros that were defined prior to restoring a PCH. This patch >> > fixes the ICE regression with a simple change, and I think it's appropriate >> > for GCC 14 as well as backport to 11, 12, 13. The bad locations (wrong, but >> > not generally causing an ICE, and mostly affecting only the output of >> > -Wunused-macros) are not as problematic, and will be harder to fix. I could >> > take a stab at that for GCC 15. In the meantime the patch adds XFAILed >> > tests for the wrong locations (as well as passing tests for the regression >> > fix). Does it look OK please? Bootstrap + regtest all languages on x86-64 >> > Linux. Thanks! >> >> OK for trunk and branches, thanks! >> > > Thanks for the review! That is all taken care of. I have one more request if > you don't mind please... There have been some further comments on the PR > indicating that the new xfailed testcase I added is failing in an unexpected > way on at least one architecture. To recap, the idea here was that > > 1) libcpp needs new logic to be able to output correct locations for this > case. That will be some new code that is suitable for stage 1, not now. > > 2) In the meantime, we fixed things up enough to avoid an ICE that showed up > in GCC 11, and added an xfailed testcase to remind about #1. > > The problem is that, the reason that libcpp outputs the wrong locations, is > that it has always used a location from the old line_map instance to index > into the new line_map instance, and so the exact details of the wrong > locations it outputs depend on the state of those two line maps, which may > differ depending on system includes and things like that. So I was hoping to > make one further one-line change to libcpp, not yet to output correct > locations, but at least to output one which is the same always and doesn't > depend on random things. This would assign all restored macros to a > consistent location, one line following the #include that triggered the PCH > process. I think this probably shouldn't be backported but it would be nice > to get into GCC 14, while nothing critical, at least it would avoid the new > test failure that's being reported. But more generally, I think using a > location from a totally different line map is dangerous and could have worse > consequences that haven't been seen yet. Does it look OK please? Thanks! FWIW, I've tested this (the initial) version of this patch on sparc-sun-solaris2.11 (PASSes as before) and i386-pc-solaris2.11 (PASSes now unlike before). Thanks. Rainer -- ----------------------------------------------------------------------------- Rainer Orth, Center for Biotechnology, Bielefeld University