Powered by Linux
new locking check — Semantic Matching Tool

new locking check

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ross was asking about where the mailing list was, and how to write a
different kind of locking check where we want to ensure that a given
lock is held when we access a struct member.

I've CC'd the list so the first part of the question is answered.  :)
The second part of the question is more difficult.

The check_locking.c file does cross function analysis.  When a function
is called it passes the list of locks held.  If all the callers are
holding a lock then the state is &locked.  If only some of the callers
are holding a lock then it's &half_locked.

You need to rebuild the cross function database a bunch of times to
populate the database.  Each time you rebuild it, then it uses the data
that it has and adds one more level to the call tree.  Generally I tell
people that rebuilding the DB five times makes it fully populated.

So then you want to implement the new heuristic:
1) Add a hook for dereferences.  Which is actually a PREOP_EXPR and
   expr->op == *.  It's an ancient ugliness that EXPR_DEREF is not what
   you want.  I need to clean that up.

	add_hook(&match_dereferences, DEREF_HOOK);

2) Get the struct member name, and the lock that we want:

static void match_dereferences(struct expression *expr)
{
	char *member_name, *expected_lock;

	if (expr->type != EXPR_PREOP)
		return;

	if (__in_fake_assign || __in_fake_parameter_assign)
		return;

	member_name = get_member_name(expr->unop);
	if (!member_name)
		return;
	expected_lock = get_expected_lock(member_name);
	if (!expect_lock)
		return;

	...

3) Search through all locked states and check if our lock is held:

static bool lock_is_held(const char *lock)
{
	struct sm_state *sm;
	static int lock_id;

	if (!lock_id)
		lock_id = id_from_name("check_locking");

	FOR_EACH_MY_SM(lock_id, __get_cur_stree(), sm) {
		if (strcmp(sm->state->name, "locked") != 0 &&
		    strcmp(sm->state->name, "half_locked") != 0)
			continue;
		if (strstr(sm->name, lock))
			return true;
	} END_FOR_EACH_SM(sm);

	return false;
}

4) Print the warning:

	if (!lock_is_held(expected_lock))
		sm_warning("struct member '%s' used without '%s'", member_name, expected_lock);

There are a few problems:

The first problem is that whenever you write an ambitious Smatch check
there are always issues.  You can't predict in advance what the issues
will be.  I'm always help out though, so write test case or whatever
and send to the list.

The other problem is that the code likely has special cases like
probe where we know the structure hasn't been exported yet so it
won't race.  Or we know that we're holding a different lock so we don't
need to take the other regular one.

One idea is that instead of printing a warning, you add that to a list
of suspicous exprssions paired with the lock.  Then if anyone takes the
lock later in the function, print the warning at that point.

	add_check_tracker("check_locking", &locking_hook);

static void locking_hook(int owner, const char *name, struct symbol *sym, struct smatch_state *state)
{
        struct state_list *slist = NULL;
        struct sm_state *sm;

        if (strcmp(state->name, "locked") != 0) {
                clear_suspicious_pairs(name);
                return;
        }

	print_all_warnings(name);
        clear_suspicious_pairs(name);
}

regards,
dan carpenter



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux