From patchwork Wed Jan 11 17:58:49 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 42124 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4e01:0:0:0:0:0 with SMTP id p1csp3460794wrt; Wed, 11 Jan 2023 09:59:46 -0800 (PST) X-Google-Smtp-Source: AMrXdXtazccn4mMtBdSxl440UViSirGPVMlUsz9zQkyN07j5791xxlO8l9aPdSMm3BcDugFr2Igt X-Received: by 2002:aa7:d689:0:b0:46c:3f90:f5d9 with SMTP id d9-20020aa7d689000000b0046c3f90f5d9mr62317896edr.5.1673459986652; Wed, 11 Jan 2023 09:59:46 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1673459986; cv=none; d=google.com; s=arc-20160816; b=LudS/BX1LaK4ysPQiIpa9VmrQnIzh1N2HwlhMJyQRlSWqqEkHoGyx5FLYQOP3MQ4fo f+KYKiiELB4Ye4OeE8yRXQ1GPQ/HZA8o9AauWTaa0A63QAO5Oz66uoJIQPELISzLd0z/ RsQ/FpoAcosa0zHdSchtw7vBgdYlauDceRM0cfh0sKRax3N/0V7iiaJlCVCrf5/c4pd7 4+a0LYQBwmrB0vpwpHMTEK4ZcqOiqtFwGGg/HvAO7BHyAsQS2rMLvADI6PKHGTqvHyLR Z7JS9mBN+8gG4akBjQnvOFix4oey3C97V3BYITkQNS38s+1EGyt+AtSR6BWOBdrl5dTf ZajQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:from:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence :content-disposition:mime-version:message-id:subject:cc:to:date :dmarc-filter:delivered-to:dkim-signature:dkim-filter; bh=NqQgm3W1+qcPkK1rqP0anb4XQnKUpIgK2hpDy0M4MX0=; b=vyh15TvNrD1ZPwi2AXa028hWnunumtwRsjb2Cu2kvmblOkUXAIjb7wESRCPBe5bAR+ 0+Piw8vEOQpYc3mtKdwcXqzvUn/jb0WEtBvDZTpVOT64Ma1pF0Co0wEe621nqvkcP8ty maJbOD2VYZOfhvLlZn8qom4R6vTkjSYiJF5tRQK1FeY9gLGHKdWY7jDOzQAbtm3Kdrzy JMSuzjBxTCeoC03ledOQPcTLCwXcDUnudHj681k25cBwrc6g9j4Ejf617uHyA776UBrN 9/yjTIct0dl5929c6Fzvs41RFyNZVHvtVcEYPkK0+ApLbLcqW8WW37dh0qNa/MIv3P8Y 9kJQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=yZeW1Ppi; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from sourceware.org (server2.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id hv13-20020a17090760cd00b008244db32413si16442796ejc.445.2023.01.11.09.59.46 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 Jan 2023 09:59:46 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) client-ip=8.43.85.97; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=yZeW1Ppi; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B08CD3858C66 for ; Wed, 11 Jan 2023 17:59:45 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B08CD3858C66 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1673459985; bh=NqQgm3W1+qcPkK1rqP0anb4XQnKUpIgK2hpDy0M4MX0=; h=Date:To:Cc:Subject:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=yZeW1PpiOpMegUnpH1REpgh3UBZLmI6Izc5oL5MXrH/wZ78gtVZlto0E8RvV8pmZL Kz7r6V238BpXsjjGMzjh3jex8Ax1eaRIHj5sUBxXF8PtMjflvGp+0VWx9DdSLI6G0g 8+gR1asiOb2HO0bU4gyMWElypP6hMZiTcCpeFLSo= 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 6304B3858C83 for ; Wed, 11 Jan 2023 17:58:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6304B3858C83 Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-615-v14hsITyMeyJr5gSMJnjKg-1; Wed, 11 Jan 2023 12:58:53 -0500 X-MC-Unique: v14hsITyMeyJr5gSMJnjKg-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 892081C0A59B for ; Wed, 11 Jan 2023 17:58:53 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.223]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 4429E492C18; Wed, 11 Jan 2023 17:58:53 +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 30BHwosl3409959 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Wed, 11 Jan 2023 18:58:50 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 30BHwox93409958; Wed, 11 Jan 2023 18:58:50 +0100 Date: Wed, 11 Jan 2023 18:58:49 +0100 To: Jason Merrill Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] c++: Avoid incorrect shortening of divisions [PR108365] Message-ID: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-3.8 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+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1754749978743438634?= X-GMAIL-MSGID: =?utf-8?q?1754749978743438634?= Hi! The following testcase is miscompiled, because we shorten the division in a case where it should not be shortened. Divisions (and modulos) can be shortened if it is unsigned division/modulo, or if it is signed division/modulo where we can prove the dividend will not be the minimum signed value or divisor will not be -1, because e.g. on sizeof(long long)==sizeof(int)*2 && __INT_MAX__ == 0x7fffffff targets (-2147483647 - 1) / -1 is UB but (int) (-2147483648LL / -1LL) is not, it is -2147483648. The primary aim of both the C and C++ FE division/modulo shortening I assume was for the implicit integral promotions of {,signed,unsigned} {char,short} and because at this point we have no VRP information etc., the shortening is done if the integral promotion is from unsigned type for the divisor or if the dividend is an integer constant other than -1. This works fine for char/short -> int promotions when char/short have smaller precision than int - unsigned char -> int or unsigned short -> int will always be a positive int, so never the most negative. Now, the C FE checks whether orig_op0 is TYPE_UNSIGNED where op0 is either the same as orig_op0 or that promoted to int, I think that works fine, if it isn't promoted, either the division/modulo common type will have the same precision as op0 but then the division/modulo is unsigned and so without UB, or it will be done in wider precision (e.g. because op1 has wider precision), but then op0 can't be minimum signed value. Or it has been promoted to int, but in that case it was again from narrower type and so never minimum signed int. But the C++ FE was checking if op0 is a NOP_EXPR from TYPE_UNSIGNED. First of all, not sure if the operand of NOP_EXPR couldn't be non-integral type where TYPE_UNSIGNED wouldn't be meaningful, but more importantly, even if it is a cast from unsigned integral type, we only know it can't be minimum signed value if it is a widening cast, if it is same precision or narrowing cast, we know nothing. So, the following patch for the NOP_EXPR cases checks just in case that it is from integral type and more importantly checks it is a widening conversion, and then next to it also allows op0 to be just unsigned, promoted or not, as that is what the C FE will do for those cases too and I believe it must work - either the division/modulo common type will be that unsigned type, then we can shorten and don't need to worry about UB, or it will be some wider signed type but then it can't be most negative value of the wider type. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2023-01-11 Jakub Jelinek PR c++/108365 * typeck.cc (cp_build_binary_op): For integral division or modulo, shorten if type0 is unsigned, or op0 is cast from narrower unsigned integral type or stripped_op1 is INTEGER_CST other than -1. * g++.dg/opt/pr108365.C: New test. * g++.dg/warn/pr108365.C: New test. Jakub --- gcc/cp/typeck.cc.jj 2022-12-15 19:17:37.828072458 +0100 +++ gcc/cp/typeck.cc 2023-01-11 12:15:25.195284107 +0100 @@ -5455,8 +5455,15 @@ 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)))) + shorten = (TYPE_UNSIGNED (type0) + || (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))) || (TREE_CODE (stripped_op1) == INTEGER_CST && ! integer_all_onesp (stripped_op1))); } @@ -5491,8 +5498,12 @@ 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)))) + shorten = (TYPE_UNSIGNED (type0) + || (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))) || (TREE_CODE (stripped_op1) == INTEGER_CST && ! integer_all_onesp (stripped_op1))); common = 1; --- gcc/testsuite/g++.dg/opt/pr108365.C.jj 2023-01-11 12:19:03.322086288 +0100 +++ gcc/testsuite/g++.dg/opt/pr108365.C 2023-01-11 12:18:39.811430975 +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-11 12:32:55.952875172 +0100 +++ gcc/testsuite/g++.dg/warn/pr108365.C 2023-01-11 12:32:37.345148131 +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" }