Re: [linux-next:master 2908/3314] lib/stackdepot.c:544 stack_print() warn: unsigned 'stack->size' is never less than zero.

[Dan Carpenter <dan.carpenter@linaro.org>]

[-- Attachment #1: Type: text/plain, Size: 1314 bytes --]

On Fri, May 19, 2023 at 10:21:37AM +0300, Dan Carpenter wrote:
> On Thu, May 18, 2023 at 01:32:47PM -0700, Andrew Morton wrote:
> > On Thu, 18 May 2023 23:44:10 +0800 kernel test robot <lkp@intel.com> wrote:
> > 
> > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> > > head:   798d276b39e984345d52b933a900a71fa0815928
> > > commit: e435b85a4aea7a82259105d5d8025655460052e1 [2908/3314] mm, page_owner: add page_owner_stacks file to print out only stacks and their counte
> > > config: x86_64-randconfig-m001
> > > compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
> > > 
> > > If you fix the issue, kindly add following tag where applicable
> > > | Reported-by: kernel test robot <lkp@intel.com>
> > > | Closes: https://lore.kernel.org/oe-kbuild-all/202305182345.LTMlWG84-lkp@intel.com/
> > > 
> > > smatch warnings:
> > > lib/stackdepot.c:544 stack_print() warn: unsigned 'stack->size' is never less than zero.
> > > 
> > 
> > Linus would complain about that warning.
> > 
> 
> Yeah.  I hadn't intended that warning to be printed.  It's supposed
> to check for if the check for negatives is paired with an upper bound
> check.  That Smatch check is original vintage from 2009 so it's due for
> a re-write.

Done.  I'll test it out a bit and push next week.

regards,
dan carpenter


[-- Attachment #2: check_unsigned_lt_zero.c --]
[-- Type: text/x-csrc, Size: 2264 bytes --]

/*
 * Copyright (C) 2009 Dan Carpenter.
 * Copyright 2023 Linaro Ltd.
 *
 * This program is free software; you can redistribute it and/or
 * modify it under the terms of the GNU General Public License
 * as published by the Free Software Foundation; either version 2
 * of the License, or (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program; if not, see http://www.gnu.org/copyleft/gpl.txt
 */

#include "smatch.h"
#include "smatch_extra.h"

static int my_id;

static bool is_upper_op(int op)
{
	switch (op) {
	case '>':
	case SPECIAL_GTE:
	case SPECIAL_UNSIGNED_GT:
	case SPECIAL_UNSIGNED_GTE:
		return true;
	}
	return false;
}

static bool is_upper(struct expression *var, struct expression *cond)
{
	if (cond->type == EXPR_COMPARE &&
	    is_upper_op(cond->op) &&
	    expr_equiv(var, cond->left))
		return true;
	return false;
}

static bool has_upper_bound(struct expression *expr)
{
	struct expression *var = strip_expr(expr->left);
	struct expression *parent, *prev;

	parent = expr;
	prev = expr;
	while ((parent = expr_get_parent_expr(parent))) {
		if (parent->type == EXPR_LOGICAL &&
		    parent->op == SPECIAL_LOGICAL_AND)
			break;
		if (parent->type == EXPR_LOGICAL &&
		    parent->op == SPECIAL_LOGICAL_OR) {
			if (prev == parent->left &&
			    is_upper(var, parent->right))
				return true;
			if (prev == parent->right &&
			    is_upper(var, parent->left))
				return true;
		}
		prev = parent;
	}

	return false;
}

static void match_condition(struct expression *expr)
{
	char *name;

	if (expr->type != EXPR_COMPARE ||
	    expr->op != SPECIAL_UNSIGNED_LT ||
	    expr->op != SPECIAL_UNSIGNED_LTE)
		return;

	if (!expr_is_zero(expr->right))
		return;

	if (has_upper_bound(expr))
		return;

	name = expr_to_str(expr->left);
	sm_warning("unsigned '%s' is never less than zero.", name);
	free_string(name);
}

void check_unsigned_lt_zero(int id)
{
	my_id = id;

	add_hook(&match_condition, CONDITION_HOOK);
}