From patchwork Mon Oct 17 17:30:15 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bernd Edlinger X-Patchwork-Id: 77785 Delivered-To: patch@linaro.org Received: by 10.140.97.247 with SMTP id m110csp500474qge; Mon, 17 Oct 2016 10:30:53 -0700 (PDT) X-Received: by 10.66.132.108 with SMTP id ot12mr32654309pab.157.1476725453678; Mon, 17 Oct 2016 10:30:53 -0700 (PDT) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id 80si31593995pfp.274.2016.10.17.10.30.53 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 17 Oct 2016 10:30:53 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-return-438815-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org; spf=pass (google.com: domain of gcc-patches-return-438815-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-438815-patch=linaro.org@gcc.gnu.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:references:in-reply-to :content-type:mime-version; q=dns; s=default; b=t5Xu4W5ASJLN/HXQ vzU+AC6vNXQpPYnBAqEpEhSLIwfD9blPDqNEEgOVn8Lj9pTcRKbgoTXUvKLXkndw We1vYBD1HiFE66Am7Tmo7VaFSZh5YV49YOcm/pEN2iks802mfLsp/NB2fiZCo8P+ 19v2X0oSY6Y9g1D0H6rJ4AnHOag= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:references:in-reply-to :content-type:mime-version; s=default; bh=F3lG0kw3iv9y+idc9Fh4yu /+V9w=; b=u1waWl59nD1i5HP5gEnvx94EXfyU0EU1t9Vn0+yLwuo8sRD2nSaIBs NM7RrgE3D6RxqYy03uwgzB8G7aqNsvRu8ww7hSPeXCOn1pVTtDzFqHZ4hJfqx5At 1AZj60zAkbGrV6LXRLrkcNnfOH1arVIJtYmvzGqDxVcVl+alGsELI= Received: (qmail 58236 invoked by alias); 17 Oct 2016 17:30:38 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 58139 invoked by uid 89); 17 Oct 2016 17:30:31 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=Hx-spam-relays-external:sk:COL004-, H*RU:sk:COL004-, HX-HELO:sk:COL004-, Hx-spam-relays-external:sk:col004- X-HELO: COL004-OMC4S6.hotmail.com Received: from col004-omc4s6.hotmail.com (HELO COL004-OMC4S6.hotmail.com) (65.55.34.208) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 17 Oct 2016 17:30:20 +0000 Received: from EUR01-HE1-obe.outbound.protection.outlook.com ([65.55.34.199]) by COL004-OMC4S6.hotmail.com over TLS secured channel with Microsoft SMTPSVC(7.5.7601.23008); Mon, 17 Oct 2016 10:30:19 -0700 Received: from VE1EUR01FT015.eop-EUR01.prod.protection.outlook.com (10.152.2.60) by VE1EUR01HT014.eop-EUR01.prod.protection.outlook.com (10.152.2.124) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.629.5; Mon, 17 Oct 2016 17:30:16 +0000 Received: from AM4PR0701MB2162.eurprd07.prod.outlook.com (10.152.2.54) by VE1EUR01FT015.mail.protection.outlook.com (10.152.2.237) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.669.7 via Frontend Transport; Mon, 17 Oct 2016 17:30:15 +0000 Received: from AM4PR0701MB2162.eurprd07.prod.outlook.com ([10.167.132.147]) by AM4PR0701MB2162.eurprd07.prod.outlook.com ([10.167.132.147]) with mapi id 15.01.0679.006; Mon, 17 Oct 2016 17:30:15 +0000 From: Bernd Edlinger To: Markus Trippelsdorf CC: Jason Merrill , Florian Weimer , "gcc-patches@gcc.gnu.org" , Jeff Law Subject: Re: [PATCH] Make -Wint-in-bool-context warn on suspicious shift ops Date: Mon, 17 Oct 2016 17:30:15 +0000 Message-ID: References: <87fuol77ri.fsf@mid.deneb.enyo.de> <20161017152304.GA303@x4> <20161017171124.GB303@x4> In-Reply-To: <20161017171124.GB303@x4> authentication-results: trippelsdorf.de; dkim=none (message not signed) header.d=none; trippelsdorf.de; dmarc=none action=none header.from=hotmail.de; x-ms-exchange-messagesentrepresentingtype: 1 x-eopattributedmessage: 0 x-microsoft-exchange-diagnostics: 1; VE1EUR01HT014; 6:swo1wPna5ZoTwKCOzZY5jWBRtp23Qnfddd7UZUrBIxSj+BXrfuUwhsuM+jbUyNwMkRiS1NVfJ5t21Ecbyj1hZK2VPnJPoYmv+YQ0IGHa9qqhN7PIp26eZwjaoXOS0mI4dND+55Qtwl5NrSiZ9FLwA4YASNc1vV/wSp80PJVhUhUb5e3+xGLAR7k/zCh2T456g/tjXV9bUVTMrrzZ45yAjwHqyONQvr8MnWBTuuTl7WPRahSZW1qjdT+awkfNvQfbe33aRIVk0fHvDp6DrzJblwdEp3136YIgrrjYrvHAuro=; 5:eJGcIxOzXyz6KxeWrfjfX5YpyVT1B/stDjcQncpeGCXoyL3kX6ktsl6HK68l42+akbkiUTHKi/1Zm2OopMtCv5lY0N4Be+zUwohRckO6f6B3rfF/FsWiyaGOD4DBiQpsgutyuUg8z6RXA76esC+9Pg==; 24:XBRXZAF9iyte7VUZ1KW94pWCQdpQJA4U4qTdi0X4mqghw9cDLUsw5DnVeroTGetV4V0OdJE6nRXb+YFKqh4ezegph7E775jnVZclsWJqjww=; 7:cBfCtFacewepQAwKKfn1mxgcxXy3TWzOxRj0MMoYzMwRjMiV8mZKfijpA6shYgm8t0xfPQnwyz+t8jLYqtmDmLm+8EFxtSaIb/4KHvy+BwgfFS+S2fz86UwsXGzLhjcHOAO7hx0k4HR5YAV9giCbhZBn31CU+ykBWkvGBYnSgJj/YkF36x832Pl300LXTvhZi5eIzVReHIrZ0zK+TF5J4hD+Obw03mAxNx9pKOeCOcfJ8YlEAn42QXSwfd+ZeGL1+CApJ0LSVE10IMtBr/4sm6idaeO0kFVS93vtrY1vdjK7jrpkTJIGvH86RDSNy3k5zde/mk26OuUDPGCICeouOw== x-forefront-antispam-report: EFV:NLI; SFV:NSPM; SFS:(10019020)(98900003); DIR:OUT; SFP:1102; SCL:1; SRVR:VE1EUR01HT014; H:AM4PR0701MB2162.eurprd07.prod.outlook.com; FPR:; SPF:None; LANG:en; x-ms-office365-filtering-correlation-id: 90845306-6653-4823-1df4-08d3f6b34149 x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(1601124038)(1603103081)(1601125047); SRVR:VE1EUR01HT014; x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(432015012)(102415321)(82015046); SRVR:VE1EUR01HT014; BCL:0; PCL:0; RULEID:; SRVR:VE1EUR01HT014; x-forefront-prvs: 0098BA6C6C spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Oct 2016 17:30:15.5170 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Internet X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-Transport-CrossTenantHeadersStamped: VE1EUR01HT014 On 10/17/16 19:11, Markus Trippelsdorf wrote: >>> I'm seeing this warning a lot in valid low level C code for unsigned >>> integers. And I must say it look bogus in this context. Some examples: > > (All these examples are from qemu trunk.) > >>> return ((a.high & 0x7fff) == 0x7fff) && (a.low<<1); >>> > > typedef struct { > uint64_t low; > uint16_t high; > } floatx80; > > static inline int floatx80_is_any_nan(floatx80 a) > { > return ((a.high & 0x7fff) == 0x7fff) && (a.low<<1); > } > >> With the shift op, the result depends on integer promotion rules, >> and if the value is signed, it can invoke undefined behavior. >> >> But if a.low is a unsigned short for instance, a warning would be >> more than justified here. > >>> if ( (uint32_t) ( aSig<<( shiftCount & 31 ) ) ) { >>> >> >> Yes interesting, aSig is signed int, right? > > No, it is uint32_t. > >> So if the << will overflow, the code is invoking undefined behavior. >> >> >>> && (uint64_t) (extractFloatx80Frac(a) << 1)) >>> >> >> What is the result type of extractFloatx80Frac() ? > > static inline uint64_t extractFloatx80Frac( floatx80 a ) > >> >>> if ((plen < KEYLENGTH) && (key << plen)) >>> >> >> This is from linux, yes, I have not seen that with the first >> version where the warning is only for signed shift ops. >> >> At first sight it looks really, like could it be that "key < plen" >> was meant? But yes, actually it works correctly as long >> as int is 32 bit, if int is 64 bits, that code would break >> immediately. > > u8 plen; > u32 key; > >> I think in the majority of code, where the author was aware of >> possible overflow issues and integer promotion rules, he will >> have used unsigned integer types, of sufficient precision. > > As I wrote above, all these warning were for unsigned integer types. > And all examples are perfectly valid code as far as I can see. > I would be fine with disabling the warning in cases where the shift is done in unsigned int. Note however, that the linux code is dependent on sizeof(int)<=sizeof(u32), but the warning would certainly be more helpful if it comes back at the day when int starts to be 64 bits wide. How about this untested patch, does it reduce the false positive rate for you? Thanks Bernd. 2016-10-17 Bernd Edlinger * c-common.c (c_common_truthvalue_conversion): Warn only for signed integer shift ops in boolean context. Index: gcc/c-family/c-common.c =================================================================== --- gcc/c-family/c-common.c (revision 241270) +++ gcc/c-family/c-common.c (working copy) @@ -3328,8 +3328,10 @@ TREE_OPERAND (expr, 0)); case LSHIFT_EXPR: - warning_at (EXPR_LOCATION (expr), OPT_Wint_in_bool_context, - "<< in boolean context, did you mean '<' ?"); + if (TREE_CODE (TREE_TYPE (expr)) == INTEGER_TYPE + && !TYPE_UNSIGNED (TREE_TYPE (expr))) + warning_at (EXPR_LOCATION (expr), OPT_Wint_in_bool_context, + "<< in boolean context, did you mean '<' ?"); break; case COND_EXPR: