From patchwork Fri Jan 13 17:45:55 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 63163 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 5814B3857438 for ; Fri, 13 Jan 2023 17:46:31 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5814B3857438 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1673631991; bh=C7SHQkD+t6oB00QiM91fOff7qH5nY8fcuep8CCXwaH4=; h=Date:To:Cc:Subject:References:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=KoUf5ZtXt29OBwjDO/lOyQR5aJSZAq6IZV1dgMNu0LRdZ+u6CvhI+d1sNuuOK6gEh dWctWy7MSYxOZWUgPMwS4Sz3BiTEi7X+KIpSaMjfoGlduX/AR+4Lx4mlc8vZDri19m h5AXb+8oBvSp8RHTVLwfxCxX5VP0vUnXU38pOKmI= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org 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 6E88B3858C39 for ; Fri, 13 Jan 2023 17:46:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6E88B3858C39 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-65-QiQ8Zb3fOwaS5PKZOOQi4A-1; Fri, 13 Jan 2023 12:45:59 -0500 X-MC-Unique: QiQ8Zb3fOwaS5PKZOOQi4A-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 7B7EB801779 for ; Fri, 13 Jan 2023 17:45:59 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.223]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 3870140C2064; Fri, 13 Jan 2023 17:45:59 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 30DHjueH1363672 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Fri, 13 Jan 2023 18:45:57 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 30DHjuho1363671; Fri, 13 Jan 2023 18:45:56 +0100 Date: Fri, 13 Jan 2023 18:45:55 +0100 To: Jason Merrill Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] c, c++, v3: Avoid incorrect shortening of divisions [PR108365] Message-ID: References: <643ddc0e-2a76-c601-e7f6-8b6bb2b3974e@redhat.com> <5033e698-017c-59ae-1e72-edf13d722ef6@redhat.com> MIME-Version: 1.0 In-Reply-To: <5033e698-017c-59ae-1e72-edf13d722ef6@redhat.com> X-Scanned-By: MIMEDefang 3.1 on 10.11.54.1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-3.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, 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.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Jakub Jelinek via Gcc-patches From: Jakub Jelinek Reply-To: Jakub Jelinek Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" On Fri, Jan 13, 2023 at 11:58:06AM -0500, Jason Merrill wrote: > LGTM, though we might put that condition in c-common somewhere? So like this then? Just tested on the new testcases, full bootstrap/regtest queued? 2023-01-13 Jakub Jelinek PR c++/108365 * c-common.h (may_shorten_divmod): New static inline function. * c-typeck.cc (build_binary_op): Use may_shorten_divmod for integral division or modulo. * typeck.cc (cp_build_binary_op): Use may_shorten_divmod for integral division or modulo. * c-c++-common/pr108365.c: New test. * g++.dg/opt/pr108365.C: New test. * g++.dg/warn/pr108365.C: New test. Jakub --- gcc/c-family/c-common.h.jj 2022-11-14 13:35:34.195160199 +0100 +++ gcc/c-family/c-common.h 2023-01-13 18:35:08.130362228 +0100 @@ -918,6 +918,30 @@ extern tree convert_init (tree, tree); /* Subroutine of build_binary_op, used for certain operations. */ extern tree shorten_binary_op (tree result_type, tree op0, tree op1, bool bitwise); +/* Return true if division or modulo op0 / op1 or op0 % op1 may be shortened. + We can shorten only if we can guarantee that op0 is not signed integral + minimum or op1 is not -1, because e.g. (long long) INT_MIN / -1 is + well defined INT_MAX + 1LL if long long is wider than int, but INT_MIN / -1 + is UB. */ +static inline bool +may_shorten_divmod (tree op0, tree op1) +{ + tree type0 = TREE_TYPE (op0); + if (TYPE_UNSIGNED (type0)) + return true; + /* A cast from narrower unsigned won't be signed integral minimum, + but cast from same or wider precision unsigned could be. */ + if (TREE_CODE (op0) == NOP_EXPR + && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (op0, 0))) + && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0))) + && (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op0, 0))) + < TYPE_PRECISION (type0))) + return true; + if (TREE_CODE (op1) == INTEGER_CST && !integer_all_onesp (op1)) + return true; + return false; +} + /* Subroutine of build_binary_op, used for comparison operations. See if the operands have both been converted from subword integer types and, if so, perhaps change them both back to their original type. */ --- gcc/c/c-typeck.cc.jj 2023-01-13 11:11:45.368016437 +0100 +++ gcc/c/c-typeck.cc 2023-01-13 18:38:25.919538847 +0100 @@ -12431,9 +12431,7 @@ build_binary_op (location_t location, en undefined if the quotient can't be represented in the computation mode. We shorten only if unsigned or if dividing by something we know != -1. */ - shorten = (TYPE_UNSIGNED (TREE_TYPE (orig_op0)) - || (TREE_CODE (op1) == INTEGER_CST - && !integer_all_onesp (op1))); + shorten = may_shorten_divmod (op0, op1); common = 1; } break; @@ -12467,9 +12465,7 @@ build_binary_op (location_t location, en on some targets, since the modulo instruction is undefined if the quotient can't be represented in the computation mode. We shorten only if unsigned or if dividing by something we know != -1. */ - shorten = (TYPE_UNSIGNED (TREE_TYPE (orig_op0)) - || (TREE_CODE (op1) == INTEGER_CST - && !integer_all_onesp (op1))); + shorten = may_shorten_divmod (op0, op1); common = 1; } break; --- gcc/cp/typeck.cc.jj 2023-01-13 11:11:45.418015716 +0100 +++ gcc/cp/typeck.cc 2023-01-13 18:38:40.754327078 +0100 @@ -5455,10 +5455,7 @@ cp_build_binary_op (const op_location_t point, so we have to dig out the original type to find out if it was unsigned. */ tree stripped_op1 = tree_strip_any_location_wrapper (op1); - shorten = ((TREE_CODE (op0) == NOP_EXPR - && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0)))) - || (TREE_CODE (stripped_op1) == INTEGER_CST - && ! integer_all_onesp (stripped_op1))); + shorten = may_shorten_divmod (op0, stripped_op1); } common = 1; @@ -5491,10 +5488,7 @@ cp_build_binary_op (const op_location_t quotient can't be represented in the computation mode. We shorten only if unsigned or if dividing by something we know != -1. */ tree stripped_op1 = tree_strip_any_location_wrapper (op1); - shorten = ((TREE_CODE (op0) == NOP_EXPR - && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0)))) - || (TREE_CODE (stripped_op1) == INTEGER_CST - && ! integer_all_onesp (stripped_op1))); + shorten = may_shorten_divmod (op0, stripped_op1); common = 1; } break; --- gcc/testsuite/c-c++-common/pr108365.c.jj 2023-01-13 18:25:09.163911212 +0100 +++ gcc/testsuite/c-c++-common/pr108365.c 2023-01-13 18:25:09.163911212 +0100 @@ -0,0 +1,16 @@ +/* PR c++/108365 */ +/* { dg-do compile { target { { ilp32 || lp64 } || llp64 } } } */ +/* { dg-options "-O2 -fdump-tree-gimple" } */ +/* { dg-final { scan-tree-dump-not " \\\((int|unsigned short int|long long int|unsigned int)\\\) " "gimple" } } */ + +unsigned short +foo (unsigned short x, unsigned short y) +{ + return (unsigned) x / y; +} + +unsigned int +bar (unsigned int x, unsigned int y) +{ + return (long long) x / y; +} --- gcc/testsuite/g++.dg/opt/pr108365.C.jj 2023-01-13 18:25:09.163911212 +0100 +++ gcc/testsuite/g++.dg/opt/pr108365.C 2023-01-13 18:25:09.163911212 +0100 @@ -0,0 +1,13 @@ +// PR c++/108365 +// { dg-do run } + +char b = 1; + +int +main () +{ +#if __CHAR_BIT__ == 8 && __SIZEOF_SHORT__ == 2 && __SIZEOF_INT__ == 4 && __SIZEOF_LONG_LONG__ == 8 + while ((short) ((long long) (unsigned long long) (-__INT_MAX__ - 1) / (long long) (b ? -1 : 0))) + ; +#endif +} --- gcc/testsuite/g++.dg/warn/pr108365.C.jj 2023-01-13 18:25:09.163911212 +0100 +++ gcc/testsuite/g++.dg/warn/pr108365.C 2023-01-13 18:25:09.163911212 +0100 @@ -0,0 +1,5 @@ +// PR c++/108365 +// { dg-do compile { target { { { ilp32 || lp64 } || llp64 } && c++11 } } } + +constexpr char b = 1; +long t = (short) ((long long) (unsigned long long) (-__INT_MAX__ - 1) / (long long) (b ? -1 : 0)); // { dg-bogus "integer overflow in expression of type" }