From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from userp2120.oracle.com ([156.151.31.85]:48600 "EHLO userp2120.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728602AbgBKOqM (ORCPT ); Tue, 11 Feb 2020 09:46:12 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=date : from : to : cc : subject : message-id : references : mime-version : content-type : in-reply-to; s=corp-2020-01-29; bh=lnttKXLDf+wHBCgJ4+YZEARyPDATjLGVqyD1sitIGhM=; b=jSl3hgyXX12TBkmvXv8UNWmEAbxPPNsuBPH+tr9tSYdxpjRqVyIrkmQ48Ca8qH4uqnvD z/+h+lRVeL3kVB2wjFZTSjoz0dj6irmUYVXOwizM7NUdpuyZ/0zpVOvVDkwmqUxJWJI7 HO7fUprgWkqENdp/LHCEC5FrX8UVp6/KnWAuK2czSzT+gdCtBMYRCA6Pn5eTzl5/84m6 QTg9qU5oPYg9BXAu5HHyfaZyb01Jh4NMONKiGwn7LTl4HBCFU5YFTS0Gdk+YqTLIV68P hgUFEZPbdO4KgH72b9bkqotaCb1xIkxtRk9aP1UqvU4CLYT1Wq1ip62D+1r2RUa5xZnE VQ== Date: Tue, 11 Feb 2020 17:46:02 +0300 From: Dan Carpenter Subject: Re: smatch: misreported 'mask and shift to zero' Message-ID: <20200211144602.GH1778@kadam> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: smatch-owner@vger.kernel.org List-ID: To: Toomas Soome Cc: smatch@vger.kernel.org On Wed, Feb 05, 2020 at 08:52:16AM +0200, Toomas Soome wrote: > hi! > > The smatch check with illumos code has warning: > > /code/illumos-gate/usr/src/tools/proto/root_sparc-nd/opt/onbld/bin/sparc/smatch: /code/illumos-gate/usr/src/common/bignum/bignumimpl.c:1410 big_mul_add_vec() warn: mask and shift to zero > > The code itself is perfectly legal: > > http://src.illumos.org/source/xref/illumos-gate/usr/src/common/bignum/bignumimpl.c#1410 > The problem is that Smatch doesn't handle loops correctly. The test is looking for places which do: y = (y & 0xff) >> 8; ^^^^ ^ It only cares about these two values. The problem is that Smatch only parses loops one time. The canonical loops are handled so it knows that inside the loop i is in the range "0 to len - 1". But it doesn't know that cy1 is changed. Smatch is correct that "(cy1 >> (BIG_CHUNK_SIZE / 2))" is going to be zero on the first iteration but on the second iteration that's not true so it shouldn't print a warning. One way to fix this is to use the diff below. The difference is that get_value() only considers literals and get_implied_value() looks at variables as well. I think in real life the mask value is always a literal. regards, dan carpenter diff --git a/check_shift_to_zero.c b/check_shift_to_zero.c index 019b06fb..3552fe87 100644 --- a/check_shift_to_zero.c +++ b/check_shift_to_zero.c @@ -58,7 +58,7 @@ static void match_binop2(struct expression *expr) if (!get_implied_value(expr->right, &shift)) return; - if (!get_implied_value(left->right, &mask)) + if (!get_value(left->right, &mask)) return; if (mask.uvalue >> shift.uvalue)