From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x52c.google.com (mail-ed1-x52c.google.com [IPv6:2a00:1450:4864:20::52c]) by sourceware.org (Postfix) with ESMTPS id BB5F83853C02 for ; Wed, 28 Jul 2021 09:23:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BB5F83853C02 Received: by mail-ed1-x52c.google.com with SMTP id b7so2299328edu.3 for ; Wed, 28 Jul 2021 02:23:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Tf2I2pPya34rDSOIyQCxuq+8uLzf47dNULCWW0juzWM=; b=pJmgAhIkAL3+QHjCKyvlRzasKZVDKvqkkHXlqR71H6gTQJY82Gy3Hdvywtxojy0jW7 571JK5ojXLc6+KfduLs+6T36h46jx4VYYDFoAlJXL49fn429GNL8CF/tzYdpEGgytnZP /JhxasHvvx6E87vGR2lM6kfVkt3Ea8nXtf4GekJpc1GiULgmht36Ksf3xNG/yN07C062 6bJGA/0wFNsMtpptFkQt4Hr2v59idnFT7DLuchph0/veOQ5hpRxM5ft2GRvm5c9azGna FEX1foXYvo66NnDMNiotaLCiDj33KdTqj8165SMiwQe9dtO6xiacL8Sa58rlItI+g0fD hEnQ== X-Gm-Message-State: AOAM5323wczC6rQt5OxcYRwj6o8lcywFCcdF56Cizz8SNervyir5vQHf yfaKpk3U9oOaiCysszs+jtcOvwVEN0zdAXpB/Ro= X-Google-Smtp-Source: ABdhPJx0KgcCk2ngZlj6rWZAbDw+Tt7mt7WKfp+9GWRzd9l7SqXGqFaQkt7qVv7Rluhw4gGzFRLFGYKY+4KsXzBLcQ0= X-Received: by 2002:a50:8d4e:: with SMTP id t14mr33906256edt.138.1627464234596; Wed, 28 Jul 2021 02:23:54 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Richard Biener Date: Wed, 28 Jul 2021 11:23:43 +0200 Message-ID: Subject: Re: [PATCH] add access warning pass To: Martin Sebor Cc: gcc-patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, 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: Wed, 28 Jul 2021 09:23:57 -0000 On Fri, Jul 16, 2021 at 12:42 AM Martin Sebor via Gcc-patches wrote: > > A number of access warnings as well as their supporting > infrastructure (compute_objsize et al.) are implemented in > builtins.{c,h} where they (mostly) operate on trees and run > just before RTL expansion. > > This setup may have made sense initially when the warnings were > very simple and didn't perform any CFG analysis, but it's becoming > a liability. The code has grown both in size and in complexity, > might need to examine the CFG to improve detection, and in some > cases might achieve a better S/R ratio if run earlier. Running > the warning code on trees is also slower because it doesn't > benefit from the SSA_NAME caching provided by the pointer_query > class. Finally, having the code there is also an impediment to > maintainability as warnings and builtin expansion are unrelated > to each other and contributors to one area shouldn't need to wade > through unrelated code (similar for patch reviewers). > > The attached change introduces a new warning pass and a couple of > new source and headers and, as the first step, moves the warning > code from builtins.{c,h} there. To keep the initial changes as > simple as possible the pass only runs a subset of existing > warnings: -Wfree-nonheap-object, -Wmismatched-dealloc, and > -Wmismatched-new-delete. The others (-Wstringop-overflow and > -Wstringop-overread) still run on the tree representation and > are still invoked from builtins.c or elsewhere. > > The changes have no functional impact either on codegen or on > warnings. I tested them on x86_64-linux. > > As the next step I plan to change the -Wstringop-overflow and > -Wstringop-overread code to run on the GIMPLE IL in the new pass > instead of on trees in builtins.c. That's the maybe_warn_rdwr_sizes thing? + gimple *stmt = gsi_stmt (si); + if (!is_gimple_call (stmt)) + continue; + + check (as_a (stmt)); if (gcall *call = dyn_cast (gsi_stmt (si))) check (call); might be more C++-ish. The patch looks OK - I skimmed it as mostly moving things around plus adding a new pass. Thanks, Richard. > Martin > > PS The builtins.c diff produced by git diff was much bigger than > the changes justify. It seems that the code removal somehow > confused it. To make review easy I replaced it with a plain > unified diff of builtins.c that doesn't suffer from the problem.