From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 40967 invoked by alias); 3 Jun 2019 10:38:24 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 40959 invoked by uid 89); 3 Jun 2019 10:38:23 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-6.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=fold_build2 X-HELO: smtp.eu.adacore.com Received: from mel.act-europe.fr (HELO smtp.eu.adacore.com) (194.98.77.210) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 03 Jun 2019 10:38:22 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id 73D7F815A8; Mon, 3 Jun 2019 12:38:19 +0200 (CEST) Received: from smtp.eu.adacore.com ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id UZKbJiZkZtbV; Mon, 3 Jun 2019 12:38:19 +0200 (CEST) Received: from polaris.localnet (bon31-6-88-161-99-133.fbx.proxad.net [88.161.99.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.eu.adacore.com (Postfix) with ESMTPSA id 3D93C8152A; Mon, 3 Jun 2019 12:38:19 +0200 (CEST) From: Eric Botcazou To: Richard Biener Cc: gcc-patches@gcc.gnu.org Subject: Re: [patch] Fix segfault caused by spurious constant overflow Date: Mon, 03 Jun 2019 10:38:00 -0000 Message-ID: <11556037.CDzcRUGLMd@polaris> In-Reply-To: References: <5613133.Bo1V3R3Yke@polaris> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="nextPart1684312.EoW952bEcD" Content-Transfer-Encoding: 7Bit X-SW-Source: 2019-06/txt/msg00057.txt.bz2 This is a multi-part message in MIME format. --nextPart1684312.EoW952bEcD Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Content-length: 923 > Hmm, ISTR we had such mitigations in place (or have) elsewhere keying > on the most significant bit set instead of power-of-two. But your case > likely recurses and runs into the extract_multiv limiting to eventually > stop, even for (N + 4) * 8, right? If so shouldn't we prevent this > even for !TYPE_OVERFLOW_WRAPS? Also > > + && !(tree_fits_shwi_p (c) > + && exact_log2 (absu_hwi (tree_to_shwi (c))) > 0)) > > is better written as > > && exact_log2 (wi::to_wide (c)) > 0 It turns out that pow2p_hwi can be used instead and is cheaper, so I have changed both extract_muldiv_1 and fold_plusminus_mult_expr to using it. * fold-const.c (extract_muldiv_1) : Do not distribute a multiplication by a power-of-two value. (fold_plusminus_mult_expr): Use pow2p_hwi to detect a power-of-two value and turn the modulo operation into a masking operation. -- Eric Botcazou --nextPart1684312.EoW952bEcD Content-Disposition: attachment; filename="p.diff" Content-Transfer-Encoding: 7Bit Content-Type: text/x-patch; charset="UTF-8"; name="p.diff" Content-length: 2239 Index: fold-const.c =================================================================== --- fold-const.c (revision 271694) +++ fold-const.c (working copy) @@ -6475,8 +6475,12 @@ extract_muldiv_1 (tree t, tree c, enum t apply the distributive law to commute the multiply and addition if the multiplication of the constants doesn't overflow and overflow is defined. With undefined overflow - op0 * c might overflow, while (op0 + orig_op1) * c doesn't. */ - if (code == MULT_EXPR && TYPE_OVERFLOW_WRAPS (ctype)) + op0 * c might overflow, while (op0 + orig_op1) * c doesn't. + But fold_plusminus_mult_expr would factor back any power-of-two + value so do not distribute in the first place in this case. */ + if (code == MULT_EXPR + && TYPE_OVERFLOW_WRAPS (ctype) + && !(tree_fits_shwi_p (c) && pow2p_hwi (absu_hwi (tree_to_shwi (c))))) return fold_build2 (tcode, ctype, fold_build2 (code, ctype, fold_convert (ctype, op0), @@ -7124,14 +7128,13 @@ fold_plusminus_mult_expr (location_t loc /* No identical multiplicands; see if we can find a common power-of-two factor in non-power-of-two multiplies. This can help in multi-dimensional array access. */ - else if (tree_fits_shwi_p (arg01) - && tree_fits_shwi_p (arg11)) + else if (tree_fits_shwi_p (arg01) && tree_fits_shwi_p (arg11)) { - HOST_WIDE_INT int01, int11, tmp; + HOST_WIDE_INT int01 = tree_to_shwi (arg01); + HOST_WIDE_INT int11 = tree_to_shwi (arg11); + HOST_WIDE_INT tmp; bool swap = false; tree maybe_same; - int01 = tree_to_shwi (arg01); - int11 = tree_to_shwi (arg11); /* Move min of absolute values to int11. */ if (absu_hwi (int01) < absu_hwi (int11)) @@ -7144,7 +7147,10 @@ fold_plusminus_mult_expr (location_t loc else maybe_same = arg11; - if (exact_log2 (absu_hwi (int11)) > 0 && int01 % int11 == 0 + unsigned HOST_WIDE_INT factor = absu_hwi (int11); + if (factor > 1 + && pow2p_hwi (factor) + && (int01 & (factor - 1)) == 0 /* The remainder should not be a constant, otherwise we end up folding i * 4 + 2 to (i * 2 + 1) * 2 which has increased the number of multiplications necessary. */ --nextPart1684312.EoW952bEcD--