From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1673631961; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references; bh=C7SHQkD+t6oB00QiM91fOff7qH5nY8fcuep8CCXwaH4=; b=dXJfWjl0YwXqnwxLx2wqa6V5FlZ3O11EM3RJRYHN0nnTwTS6YKc5JDT/nkxUsxp+53sZt0 FFjMMRwpFTERDQKefp3koq7DkKzeb33i7ND68LEEfD3t/o7cppa33xCeRrL9S1VmDw2jLu ZANtKEDlBEAo5fvjKgzCSHdntanll/I= 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 From: Jakub Jelinek To: Jason Merrill Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] c, c++, v3: Avoid incorrect shortening of divisions [PR108365] Message-ID: Reply-To: Jakub Jelinek 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-Type: text/plain; charset=us-ascii 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 List-Id: 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. --- 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" } Jakub