Re: smatch: misreported 'mask and shift to zero'

[Dan Carpenter <dan.carpenter@oracle.com>]

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)