From patchwork Wed Nov 23 10:37:09 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 61023 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 AA91C3853D50 for ; Wed, 23 Nov 2022 10:37:45 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AA91C3853D50 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1669199865; bh=SAcDaZZq0qsZCGhnmgkwpOdcrpz2u4fcJJUJEBZRp1Q=; 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=JP0MgerF68LwUyzxCixprwpEV9RPIiQMNZ2Sv+tdKlxlCEgRpCWUioJCpXnLpSsZZ hkztpz0yXwi10U0VRHONZLjkBCFftm0PURod+Zh8jyCDNs9knZGbPjzZGuEyNFyAHg qNSQ5NZIImtZQGLb21BBFCs+yQnYe+zSLdW2Kq0w= 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 D39513858D32 for ; Wed, 23 Nov 2022 10:37:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D39513858D32 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-213-Z2DsiOLxNxiIuIhnX5xziA-1; Wed, 23 Nov 2022 05:37:15 -0500 X-MC-Unique: Z2DsiOLxNxiIuIhnX5xziA-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 8682785A588; Wed, 23 Nov 2022 10:37:15 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.194.202]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 18A2A1121314; Wed, 23 Nov 2022 10:37:14 +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 2ANAbASD2532446 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Wed, 23 Nov 2022 11:37:10 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 2ANAb9Zl2532445; Wed, 23 Nov 2022 11:37:09 +0100 Date: Wed, 23 Nov 2022 11:37:09 +0100 To: Jason Merrill , "Joseph S. Myers" , Marek Polacek Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] c-family: Incremental fix for -Wsign-compare BIT_NOT_EXPR handling [PR107465] Message-ID: References: MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 3.1 on 10.11.54.3 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" Hi! There can be too many extensions and seems I didn't get everything right in the previously posted patch. The following incremental patch ought to fix that. The code can deal with quite a few sign/zero extensions at various spots and it is important to deal with all of them right. On the argument that contains BIT_NOT_EXPR we have: MSB bits#4 bits#3 BIT_NOT_EXPR bits#2 bits#1 LSB where bits#1 is one or more bits (TYPE_PRECISION (TREE_TYPE (arg0)) at the end of the function) we don't know anything about, for the purposes of this warning it is VARYING that is inverted with BIT_NOT_EXPR to some other VARYING bits; bits#2 is one or more bits (TYPE_PRECISION (TREE_TYPE (op0)) - TYPE_PRECISION (TREE_TYPE (arg0)) at the end of the function) which are known to be 0 before the BIT_NOT_EXPR and 1 after it. bits#3 is zero or more bits from the TYPE_PRECISION (TREE_TYPE (op0)) at the end of function to the TYPE_PRECISION (TREE_TYPE (op0)) at the end of the function to TYPE_PRECISION (TREE_TYPE (op0)) at the start of the function, which are either zero extension or sign extension. And bits#4 is zero or more bits from the TYPE_PRECISION (TREE_TYPE (op0)) at the start of the function to TYPE_PRECISION (result_type), which again can be zero or sign extension. Now, vanilla trunk as well as the previously posted patch mishandles the case where bits#3 are sign extended (as bits#2 are known to be all set, that means bits#3 are all set too) but bits#4 are zero extended and are thus all 0. The patch fixes it by tracking the lowest bit which is known to be clear above the known to be set bits (if any, otherwise it is precision of result_type). Ok for trunk if it passes bootstrap/regtest? 2022-11-23 Jakub Jelinek PR c/107465 * c-warn.cc (warn_for_sign_compare): Don't warn for unset bits above innermost zero extension of BIT_NOT_EXPR result. * c-c++-common/Wsign-compare-2.c (f18): New test. Jakub --- gcc/c-family/c-warn.cc.jj 2022-11-23 10:04:53.000000000 +0100 +++ gcc/c-family/c-warn.cc 2022-11-23 11:19:38.928113842 +0100 @@ -2344,13 +2344,33 @@ warn_for_sign_compare (location_t locati have all bits set that are set in the ~ operand when it is extended. */ + /* bits0 is the bit index of op0 extended to result_type, which will + be always 0 and so all bits above it. If there is a BIT_NOT_EXPR + in that operand possibly sign or zero extended to op0 and then + possibly further sign or zero extended to result_type, bits0 will + be the precision of result type if all the extensions involved + if any are sign extensions, and will be the place of the innermost + zero extension otherwise. We warn only if BIT_NOT_EXPR's operand is + zero extended from some even smaller precision, in that case after + BIT_NOT_EXPR some bits below bits0 will be guaranteed to be set. + Similarly for bits1. */ + int bits0 = TYPE_PRECISION (result_type); + if (TYPE_UNSIGNED (TREE_TYPE (op0))) + bits0 = TYPE_PRECISION (TREE_TYPE (op0)); tree arg0 = c_common_get_narrower (op0, &unsignedp0); if (TYPE_PRECISION (TREE_TYPE (arg0)) == TYPE_PRECISION (TREE_TYPE (op0))) unsignedp0 = TYPE_UNSIGNED (TREE_TYPE (op0)); + else if (unsignedp0) + bits0 = TYPE_PRECISION (TREE_TYPE (arg0)); op0 = arg0; + int bits1 = TYPE_PRECISION (result_type); + if (TYPE_UNSIGNED (TREE_TYPE (op1))) + bits1 = TYPE_PRECISION (TREE_TYPE (op1)); tree arg1 = c_common_get_narrower (op1, &unsignedp1); if (TYPE_PRECISION (TREE_TYPE (arg1)) == TYPE_PRECISION (TREE_TYPE (op1))) unsignedp1 = TYPE_UNSIGNED (TREE_TYPE (op1)); + else if (unsignedp1) + bits1 = TYPE_PRECISION (TREE_TYPE (arg1)); op1 = arg1; if ((TREE_CODE (op0) == BIT_NOT_EXPR) @@ -2360,6 +2380,7 @@ warn_for_sign_compare (location_t locati { std::swap (op0, op1); std::swap (unsignedp0, unsignedp1); + std::swap (bits0, bits1); } int unsignedp; @@ -2378,16 +2399,8 @@ warn_for_sign_compare (location_t locati && bits < HOST_BITS_PER_WIDE_INT) { HOST_WIDE_INT mask = HOST_WIDE_INT_M1U << bits; - if (unsignedp0) - { - bits = TYPE_PRECISION (TREE_TYPE (op0)); - if (bits < TYPE_PRECISION (result_type) - && bits < HOST_BITS_PER_WIDE_INT) - mask &= ~(HOST_WIDE_INT_M1U << bits); - } - bits = TYPE_PRECISION (result_type); - if (bits < HOST_BITS_PER_WIDE_INT) - mask &= ~(HOST_WIDE_INT_M1U << bits); + if (bits0 < HOST_BITS_PER_WIDE_INT) + mask &= ~(HOST_WIDE_INT_M1U << bits0); if ((mask & constant) != mask) { if (constant == 0) @@ -2405,24 +2418,7 @@ warn_for_sign_compare (location_t locati < TYPE_PRECISION (TREE_TYPE (op0))) && unsignedp && unsignedp1 - /* If unsignedp0, the BIT_NOT_EXPR result is - zero extended, so say if op0 is unsigned char - variable, BIT_NOT_EXPR is unsigned short and - result type int and op0 has value 0x55, the - int value will be 0xffaa, or for op0 0xaa it - will be 0xff55. In these cases, warn if - op1 is unsigned and narrower than unsigned short. - While if unsignedp0 is false, the BIT_NOT_EXPR - result is sign extended and because of the - above TYPE_PRECISION comparison we know the - MSB of BIT_NOT_EXPR is set (perhaps with some - further bits below it). The sign extension will - then ensure all bits above BIT_NOT_EXPR up to - result_type's precision are set. */ - && (TYPE_PRECISION (TREE_TYPE (op1)) - < TYPE_PRECISION (unsignedp0 - ? TREE_TYPE (op0) - : result_type))) + && TYPE_PRECISION (TREE_TYPE (op1)) < bits0) warning_at (location, OPT_Wsign_compare, "comparison of promoted bitwise complement " "of an unsigned value with unsigned"); --- gcc/testsuite/c-c++-common/Wsign-compare-2.c.jj 2022-11-23 10:04:53.000000000 +0100 +++ gcc/testsuite/c-c++-common/Wsign-compare-2.c 2022-11-23 11:05:10.497862219 +0100 @@ -104,3 +104,9 @@ f17 (unsigned char x, unsigned short y) { return (unsigned short) (~(unsigned short) x) == y; /* { dg-bogus "comparison of promoted bitwise complement of an unsigned value with unsigned" } */ } + +int +f18 (unsigned char x) +{ + return (unsigned int) (short) (~(unsigned short) x) == 0xffffff05ULL; /* { dg-bogus "comparison of promoted bitwise complement of an unsigned value with constant" } */ +}