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)
