From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id BC999385780A for ; Thu, 3 Dec 2020 16:56:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org BC999385780A Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 2B20F11D4; Thu, 3 Dec 2020 08:56:54 -0800 (PST) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.126]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 739C23F575; Thu, 3 Dec 2020 08:56:53 -0800 (PST) From: Richard Sandiford To: Richard Biener via Gcc-patches Mail-Followup-To: Richard Biener via Gcc-patches , Qing Zhao , Richard Biener , kees Cook , richard.sandiford@arm.com Cc: Qing Zhao , Richard Biener , kees Cook Subject: Re: How to traverse all the local variables that declared in the current routine? References: <217BE64F-A623-4453-B45B-D38B66B71B72@ORACLE.COM> <15EA64C7-D75F-4CE1-92C8-6940186A512A@ORACLE.COM> <7618DD31-87EC-4003-B1DD-E318E5369A71@ORACLE.COM> <3D01F6C7-2E67-4081-BD5F-4EDEC0227627@ORACLE.COM> <8A595141-563A-4CDC-AA09-BAB76E5113AA@ORACLE.COM> <83E30551-CAD3-4310-9E74-453874334CB2@gmail.com> Date: Thu, 03 Dec 2020 16:56:51 +0000 In-Reply-To: <83E30551-CAD3-4310-9E74-453874334CB2@gmail.com> (Richard Biener via Gcc-patches's message of "Thu, 03 Dec 2020 17:36:43 +0100") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-6.5 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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, 03 Dec 2020 16:56:56 -0000 Richard Biener via Gcc-patches writes: > On December 3, 2020 5:07:28 PM GMT+01:00, Qing Zhao wrote: >> >> >>> On Dec 3, 2020, at 2:45 AM, Richard Biener >> wrote: >>>=20 >>> On Wed, Dec 2, 2020 at 4:36 PM Qing Zhao >> wrote: >>>>=20 >>>>=20 >>>>=20 >>>> On Dec 2, 2020, at 2:45 AM, Richard Biener >> wrote: >>>>=20 >>>> On Tue, Dec 1, 2020 at 8:49 PM Qing Zhao >>wrote: >>>>=20 >>>>=20 >>>> Hi, Richard, >>>>=20 >>>> Could you please comment on the following approach: >>>>=20 >>>> Instead of adding the zero-initializer quite late at the pass >>=E2=80=9Cpass_expand=E2=80=9D, we can add it as early as during gimplific= ation. >>>> However, we will mark these new added zero-initializers as >>=E2=80=9Cartificial=E2=80=9D. And passing this =E2=80=9Cartificial=E2=80= =9D information to >>>> =E2=80=9Cpass_early_warn_uninitialized=E2=80=9D and =E2=80=9Cpass_late= _warn_uninitialized=E2=80=9D, >>in these two uninitialized variable analysis passes, >>>> (i.e., in tree-sea-uninit.c) We will update the checking on >>=E2=80=9Cssa_undefined_value_p=E2=80=9D to consider =E2=80=9Cartificial= =E2=80=9D zero-initializers. >>>> (i.e, if the def_stmt is marked with =E2=80=9Cartificial=E2=80=9D, the= n it=E2=80=99s a >>undefined value). >>>>=20 >>>> With such approach, we should be able to address all those >>conflicts. >>>>=20 >>>> Do you see any obvious issue with this approach? >>>>=20 >>>>=20 >>>> Yes, DSE will happily elide an explicit zero-init following the >>>> artificial one leading to false uninit diagnostics. >>>>=20 >>>>=20 >>>> Indeed. This is a big issue. And other optimizations might also be >>impacted by the new zero-init, resulting changed behavior >>>> of uninitialized analysis in the later stage. >>>=20 >>> I don't see how the issue can be resolved, you can't get both, uninit >>> warnings and no uninitialized memory. >>> People can compile twice, once without -fzero-init to get uninit >>> warnings and once with -fzero-init to get >>> the extra "security". >> >>So, for GCC, you think that it=E2=80=99s okay to get rid of the following >>requirement: >> >>C. The implementation needs to keep the current static warning on >>uninitialized >>variables untouched in order to avoid "forking the language=E2=80=9D. >> >>Then, we can add explanation in the user documentation of the new >>-fzero-init and also=20 >>that of the -Wuninitialized to inform users that -fzero-init will >>change the behavior of -Wuninitialized. >>In order to get the warnings, -fzero-init should not be added at the >>same time? >> >>With this requirement being eliminated, implementation will be much >>easier.=20 >> >>We can add the new initialization during simplification phase. Then >>this new option will work >>for all languages. Is this reasonable? > > I think that's reasonable indeed. Eventually doing the init after the ear= ly uninit pass is possible as well. Sorry to be awkward, but I kind-of disagree. IIRC, clang was able to give uninit warnings while implementing the initialisation as expected, so I think this is a GCC restriction rather than a fundamental incompatibility. I don't think it's reasonable to expect people to read the documentation of -ffoo for Clang and separately read the documentation of -ffoo for GCC. They'll at best read the documentation for one and (rightly) expect the other compiler to behave in a compatible way. I'm also not sure people would build twice in practice. I remember the issue of forking the language was discussed at length on the Clang dev list at the time (but I haven't gone back and re-read the thread, so I'm relying on memory here). Not forking the language was an important goal/requirement of the option and I don't think we should drop it when implementing the option in GCC. IMO, if we want to define a dialect of C/C++ in which uninitialised uses are always well defined rather than UB, we should do that as a separate option. If we're implementing the Clang options, we should continue to treat uninitialised uses as UB that triggers the same warnings as if the option wasn't passed. So TBH I'd rather not add the option until it can be implemented in a way that is compatible with Clang. Thanks, Richard