From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 2D7733858D37 for ; Thu, 1 Feb 2024 02:45:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2D7733858D37 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 2D7733858D37 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706755510; cv=none; b=sNGjfynd098jLiuyUphClw33rgUwZllx5F9JzJe8IsqX2pPVOX57W/llPmk07uneCoMHUSPj1oH5tsi1lmjwCVA8VyMIsMQ+Dg0tDmR9aR+RrWwUcYzUA+fxS0u0DK2pgIkw+2s3JoBDE0Qv6i1PUTXdyVMguwDQILGUNZDRad8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706755510; c=relaxed/simple; bh=Ptg7rWRTJcZdQyAo1Nv8eoPgt4+msdaQhilIQGIzC9E=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=YC/jWEYueqy1YaYJDmcnY8RHWzDAN/6PfU9OnCqU5kesLHaaHixoEEEnJ2E7+QFWoNH+j9eqRE+rkhScx0MR0Djt+afat6f8IKdZ279UpiaFGxINBYw43P62OTYpV6BVDKwyM06V2+b48iFVW/uf0/zRxZR0rvNhtUuBuJHy1O4= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1706755507; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=DaSMBqI8Rb4PwqHMI6q3nVoBzcDQf53DpJrZ7ar+AOs=; b=Njiq961jQKkHb1kYJKrfvBloIKC1xJJbbMOXk6ZQqVZkiKbrhUxYFV+mQFV0vV0aO/F20C UGcL21eZxpoh8a4xanjORNAZ/U6h6w1CKiJbdAUhQOJewS99wi/12Y4wQbRiOetSeL8dgh fJeWQvOeBcnFloWN1GCiJ7YVo4fi8lc= Received: from mail-qv1-f71.google.com (mail-qv1-f71.google.com [209.85.219.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-65-D0QevTfLP5yi7lBhUmFDmg-1; Wed, 31 Jan 2024 21:45:05 -0500 X-MC-Unique: D0QevTfLP5yi7lBhUmFDmg-1 Received: by mail-qv1-f71.google.com with SMTP id 6a1803df08f44-68c48383676so5031606d6.1 for ; Wed, 31 Jan 2024 18:45:05 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706755505; x=1707360305; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=DaSMBqI8Rb4PwqHMI6q3nVoBzcDQf53DpJrZ7ar+AOs=; b=wbdD2euZ2NOkaJVahAP9x7GH9tpOvA40hm30BgKa+AE8yp4bLzgVCOxgFmK7vxp+QS 0iZAIqyxVshi3Yjg6Br6ikHiuogEn1rPvhGxAIBOE+M0SH6zWA+YDFyImIrXDeOKPv/a UsorNLStR9Xmy+wo4PVu81IR2qQCfwAgvZLp2h0VUyoJlr9vLVG39HZIe3l1KaLEy/2s NNs4T0JY22FN3uPQRly1FS4Ux5q4tgkXMZ3o1jbdquaCLlCpv/ecHaHXlNMZiWi4Suzh GaIwYmplZB0Ql+PEozbTe5wNDk330g6JA822drwS6f8uCZP3MR2aL183bLgaRfUmeOGe pLvw== X-Gm-Message-State: AOJu0Yyfn8G0OYmBkfNdh4N1rZXNRNLUYvxasYFSU3EBOzbvf94uim1G eQRSamXu+TkeAEIOea6wqhySVs7vTNz29zRyy7iqznh+3pCE9hrUU2LFDmmPrce+dZkfw2gQFS9 J1ejXa90/q8t1/NetkPqMGQVwMpAcNb9+tfVNYMmTWDPRUtKEeguZmok= X-Received: by 2002:a05:6214:2267:b0:68c:6720:fe8d with SMTP id gs7-20020a056214226700b0068c6720fe8dmr4011662qvb.5.1706755505497; Wed, 31 Jan 2024 18:45:05 -0800 (PST) X-Google-Smtp-Source: AGHT+IGpO0Udl3VcaCWGOZnQLbtVcWRABsWnShCFVYMSatwjF+7H94QsS0T6qDLdMaHZo6Rkvl29cg== X-Received: by 2002:a05:6214:2267:b0:68c:6720:fe8d with SMTP id gs7-20020a056214226700b0068c6720fe8dmr4011651qvb.5.1706755505201; Wed, 31 Jan 2024 18:45:05 -0800 (PST) Received: from [192.168.1.146] (130-44-146-16.s12558.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.146.16]) by smtp.gmail.com with ESMTPSA id r11-20020a056214212b00b0068c4f42c817sm3626645qvc.131.2024.01.31.18.45.04 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 31 Jan 2024 18:45:04 -0800 (PST) Message-ID: Date: Wed, 31 Jan 2024 21:45:03 -0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] c-family: Fix ICE with large column number after restoring a PCH [PR105608] To: Lewis Hyatt Cc: gcc-patches@gcc.gnu.org References: <20231206015211.682650-1-lhyatt@gmail.com> <17e2d4b7-1dfe-4e0c-9fea-c5c115ffc66c@redhat.com> <1020fac4-663e-49bb-b4e3-0c6b7a7c0f59@redhat.com> From: Jason Merrill In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-6.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,KAM_SHORT,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,RCVD_IN_SORBS_WEB,SPF_HELO_NONE,SPF_NONE,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: On 1/31/24 21:09, Lewis Hyatt wrote: > On Wed, Jan 31, 2024 at 03:18:01PM -0500, Jason Merrill wrote: >> On 1/30/24 21:49, Lewis Hyatt wrote: >>> 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! >> >> Can we use the line of the #include, as the test expects, rather than the >> following line? > > Thanks, yes, that will work too, it just needs a few changes to > c-family/c-pch.cc to set the location there and then increment it > after. Patch which does that is attached. (This is a new one based on > master, not incremental to the prior patch.) The testcase does not require > any changes this way, and bootstrap + regtest looks good. OK, thanks. Jason