Re: likely/unlikely usage validation

Dan Carpenter <[email protected]> Wed, 29 May 2024 17:32:58 +0300
Newsgroups org.kernel.vger.smatch
Message-ID <[email protected]> 
On Tue, May 28, 2024 at 12:37:23PM +0200, Mateusz Guzik wrote:
> I misplaced a closing ) in a patch using unlikely() and broke the
> comparison, see [1] for context.
> 
> The fix was:
> -               if (unlikely(abs(count + amount)) >= batch) {
> +               if (unlikely(abs(count + amount) >= batch)) {
> 
> Neither kernel build with W=1 nor C=1 (smatch) report the problem.
> 
> Given that this compiles fine and for many cases with the bogus usage
> it also happens to produce the correct result, I have to suspect there
> are spots in the kernel with the problem.
> 
> Either way, sounds like something smatch should be testing for.
> 
> [1] https://lore.kernel.org/oe-lkp/pywb7wcml44gzgidn7mtwwr23mybbilakckchk4777wfibtruj@n4yiwwpvglf7/T/#t

Sure!  I've created a check for that and I'll push it.

I think those bugs are rare, though.  The last time we had one was maybe
in 2018.

regards,
dan carpenter
check_unlikely_parens.c (text/x-csrc, 1.1 KB) 
/*
 * Copyright (C) 2018 Oracle.
 *
 * 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_slist.h"
#include "smatch_extra.h"

static int my_id;

static void match_likely(const char *fn, struct expression *expr, void *unused)
{
	struct expression *parent;

	parent = expr_get_parent_expr(expr);
	if (!parent || parent->type != EXPR_COMPARE)
		return;
	sm_warning("warn: check likely/unlikely parentheses");
}

void check_unlikely_parens(int id)
{
	my_id = id;

	add_function_hook("__builtin_expect", &match_likely, NULL);
}