From patchwork Sat Sep 9 15:31:12 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Thomas Schwinge X-Patchwork-Id: 1831959 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4RjcQg6N1Rz1yh5 for ; Sun, 10 Sep 2023 01:31:37 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 45C0D3858426 for ; Sat, 9 Sep 2023 15:31:34 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from esa1.mentor.iphmx.com (esa1.mentor.iphmx.com [68.232.129.153]) by sourceware.org (Postfix) with ESMTPS id 452DC3858D1E for ; Sat, 9 Sep 2023 15:31:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 452DC3858D1E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com X-CSE-ConnectionGUID: H8bzdA89QembBehRkWYDbg== X-CSE-MsgGUID: zv7ZqQ5zTIaSFmTQGiTDrw== X-IronPort-AV: E=Sophos;i="6.02,239,1688457600"; d="scan'208,223";a="18649122" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa1.mentor.iphmx.com with ESMTP; 09 Sep 2023 07:31:18 -0800 IronPort-SDR: wgEXXF7+yXy8Wdz0dRo2O4eTy/DrAFo8rR31vjRQ50tuiArw/wGsGkRXktZ7siW1oS6dMpNiig aR/nB+ZnctvQslRRCKGqx/G5oIXrtCf5h9cWVmjrDA3niTjOrK0l07OXfaM31tee+riA0YxAWt juVJaKBwWOMdZHdhSMY2LA1SUbk3lGAIgsVrtHX46Kvs8f6aNIgGL5r+47e+GYZqkkLQ67MeKz Y+nnpmnelDGv3W18lt5iZIVC2rgCmMPvfEhVyJQPH9WpvhDLxw0LqIpuLFYtTMIPNsCqhnCM94 4Co= From: Thomas Schwinge To: Subject: Fix false positive for -Walloc-size-larger-than, part II [PR79132] (was: [PATCH] Fix false positive for -Walloc-size-larger-than (PR, bootstrap/79132)) In-Reply-To: References: <993c5cf4-a937-904e-1044-4cb2db925459@suse.cz> User-Agent: Notmuch/0.29.3+94~g74c3f1b (https://notmuchmail.org) Emacs/28.2 (x86_64-pc-linux-gnu) Date: Sat, 9 Sep 2023 17:31:12 +0200 Message-ID: <87h6o3z7lr.fsf@euler.schwinge.homeip.net> MIME-Version: 1.0 X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-14.mgc.mentorg.com (139.181.222.14) To svr-ies-mbx-10.mgc.mentorg.com (139.181.222.10) X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, KAM_SHORT, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jakub Jelinek Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" Hi! On 2017-01-23T15:10:44-0700, Jeff Law wrote: > On 01/19/2017 04:46 AM, Martin Liška wrote: >> Following patch fixes asan bootstrap, as mentioned in the PR. >> >> Ready to be installed? >> * tree-ssa-reassoc.c (rewrite_expr_tree_parallel): Insert assert >> that would prevent us to call alloca with -1 as argument. > Maybe one day we'll be able to use the array index into STMTS to derive > a range for stmt_num. But we don't right now. > > Otherwise I don't see any way to derive a range for stmt_num that if > rewrite_expr_tree_parallel is not inlined into its caller. > > > OK for the trunk. Just a few years later, I've run into this one again -- with only a slight twist; probably latent all those years... OK to push the attached "Fix false positive for -Walloc-size-larger-than, part II [PR79132]"? Or, do we (incrementally?) want to formulate that "assume"-like trait yet differently? That is, should we gain some 'gcc_assume ([...])' in 'gcc/system.h'? CCing Jakub as author of commit 08b51baddc53d64aa4c5e7a81ef3c4bf320293be "c++, c: Implement C++23 P1774R8 - Portable assumptions [PR106654]": The following patch implements C++23 P1774R8 - Portable assumptions paper, by introducing support for [[assume (cond)]]; attribute for C++. In addition to that the patch adds [[gnu::assume (cond)]]; and __attribute__((assume (cond))); support to both C and C++. As described in C++23, the attribute argument is conditional-expression rather than the usual assignment-expression for attribute arguments, the condition is contextually converted to bool (for C truthvalue conversion is done on it) and is never evaluated at runtime. For C++ constant expression evaluation, I only check the simplest conditions for undefined behavior, because otherwise I'd need to undo changes to *ctx->global which happened during the evaluation (but I believe the spec allows that and we can further improve later). The patch uses a new internal function, .ASSUME, to hold the condition in the FEs. At gimplification time, if the condition is simple/without side-effects, it is gimplified as if (cond) ; else __builtin_unreachable (); and otherwise for now dropped on the floor. The intent is to incrementally outline the conditions into separate artificial functions and use .ASSUME further to tell the ranger and perhaps other optimization passes about the assumptions, as detailed in the PR. When implementing it, I found that assume entry hasn't been added to https://eel.is/c++draft/cpp.cond#6 Jonathan said he'll file a NB comment about it, this patch assumes it has been added into the table as 202207L when the paper has been voted in. With the attributes for both C/C++, I'd say we don't need to add __builtin_assume with similar purpose, especially when __builtin_assume in LLVM is just weird. It is strange for side-effects in function call's argument not to be evaluated, and LLVM in that case (annoyingly) warns and ignores the side-effects (but doesn't do then anything with it), if there are no side-effects, it will work like our if (!cond) __builtin_unreachable (); (Followed by further commits re "intent is to incrementally [...]".) The current 'gcc/doc/extend.texi', "Statement Attributes" for reference: @cindex @code{assume} statement attribute @item assume The @code{assume} attribute with a null statement serves as portable assumption. It should have a single argument, a conditional expression, which is not evaluated. If the argument would evaluate to true at the point where it appears, it has no effect, otherwise there is undefined behavior. This is a GNU variant of the ISO C++23 standard @code{assume} attribute, but it can be used in any version of both C and C++. @smallexample int foo (int x, int y) @{ __attribute__((assume(x == 42))); __attribute__((assume(++y == 43))); return x + y; @} @end smallexample @code{y} is not actually incremented and the compiler can but does not have to optimize it to just @code{return 42 + 42;}. (I've not actually verified that'd do the job here, but I'd be very surprised if not.) Grüße Thomas ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955 From 7dc7de834989d85cb1dbaf7b5a0917ba07319cfb Mon Sep 17 00:00:00 2001 From: Thomas Schwinge Date: Sat, 9 Sep 2023 16:49:16 +0200 Subject: [PATCH] Fix false positive for -Walloc-size-larger-than, part II [PR79132] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In a GCC bootstrap, I was running into: In file included from [...]/gcc/system.h:729, from [...]/gcc/tree-ssa-reassoc.cc:22: [...]/gcc/tree-ssa-reassoc.cc: In function ‘void rewrite_expr_tree_parallel(gassign*, int, bool, const vec&)’: [...]/gcc/../include/libiberty.h:733:36: error: argument to ‘alloca’ is too large [-Werror=alloca-larger-than=] 733 | # define alloca(x) __builtin_alloca(x) | ~~~~~~~~~~~~~~~~^~~ [...]/gcc/../include/libiberty.h:365:40: note: in expansion of macro ‘alloca’ 365 | #define XALLOCAVEC(T, N) ((T *) alloca (sizeof (T) * (N))) | ^~~~~~ [...]/gcc/tree-ssa-reassoc.cc:5518:20: note: in expansion of macro ‘XALLOCAVEC’ 5518 | gimple **stmts = XALLOCAVEC (gimple *, stmt_num); | ^~~~~~~~~~ [...]/gcc/../include/libiberty.h:733:36: note: limit is 9223372036854775807 bytes, but argument is 18446744073709551608 733 | # define alloca(x) __builtin_alloca(x) | ~~~~~~~~~~~~~~~~^~~ [...]/gcc/../include/libiberty.h:365:40: note: in expansion of macro ‘alloca’ 365 | #define XALLOCAVEC(T, N) ((T *) alloca (sizeof (T) * (N))) | ^~~~~~ [...]/gcc/tree-ssa-reassoc.cc:5518:20: note: in expansion of macro ‘XALLOCAVEC’ 5518 | gimple **stmts = XALLOCAVEC (gimple *, stmt_num); | ^~~~~~~~~~ '18446744073709551608' means '-8', which is 'sizeof (gimple *) * (-1)'. That's the very issue that PR79132 "False positive for -Walloc-size-larger-than= with -fsanitize=address aka. bootstrap-asan breakage" commit ad8040243acd2a909b61b5690f7dac9ae362c945 (Subversion r244857) "Fix false positive for -Walloc-size-larger-than (PR bootstrap/79132)" meant to address -- the detail being that '-fsanitize=address' nowadays doesn't seem necessary anymore to trigger this, and why I was running into this at all, is 'configure'ing with '--enable-checking=no', which per 'gcc/system.h' means: #define gcc_assert(EXPR) ((void)(0 && (EXPR))) ..., and thus the 'gcc_assert (op_num > 0);' is a no-op, eh... PR bootstrap/79132 gcc/ * tree-ssa-reassoc.cc (rewrite_expr_tree_parallel): Reformulate 'gcc_assert' into 'gcc_unreachable'. --- gcc/tree-ssa-reassoc.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gcc/tree-ssa-reassoc.cc b/gcc/tree-ssa-reassoc.cc index eda03bf98a6..df4ebaa8ac5 100644 --- a/gcc/tree-ssa-reassoc.cc +++ b/gcc/tree-ssa-reassoc.cc @@ -5513,7 +5513,8 @@ rewrite_expr_tree_parallel (gassign *stmt, int width, bool has_fma, enum tree_code opcode = gimple_assign_rhs_code (stmt); int op_num = ops.length (); int op_normal_num = op_num; - gcc_assert (op_num > 0); + if (op_num < 1) + gcc_unreachable (); int stmt_num = op_num - 1; gimple **stmts = XALLOCAVEC (gimple *, stmt_num); int i = 0, j = 0; -- 2.34.1