> > This time there is a call to foo after the call to bar and before the call 
> > to xxx, and this call to foo is that same as the one that was matched 
> > before the call to bar.
> 
> OK, because we would have to cycle around the loop to match, and the
> "when" clause would disqualify the only reasonable match.

Yes.

> Thank you again!
> 
> And yet another question...
> 
> spatch does not seem to like the following:
> 
>       when != if (p) atomic_inc(&<+...p->f1...+>)

The branch of an if is a statement.  A function call that is a statement 
has a ; after it.  I see that the parse error is quite unhelpful, due to 
the way the yacc-like parsing works.

But here you assume that the if (p) will only have the atomic_inc.  
Perhaps the following would be better:

when != if (p) { ... atomic_inc(&<+...p->f1...+>); ... }

Note that all of the when code has to stay on the same line.  An 
isomorphism will allow this to match a case where there are no {}.

> The problem I am trying to solve is that it is OK to leak a pointer
> out of an RCU read-side critical section if you have done something
> to keep the underlying data structure from going away.  One way of
> doing this is to increment a reference count in the pointed-to structure,
> for example:
> 
>       rcu_read_lock();
>       p = rcu_dereference(gp);
>       if (p && something_bad_happened(p))
>               p = NULL
>       if (p)
>               atomic_inc(&p->refcount);
>       rcu_read_unlock();
> 
>       if (p)
>               do_something_with(p->a);
> 
> My current set of rules will flag the last line as a pointer leak,
> presumably because there is a code path that skips the atomic_inc().
> My first reaction was to just put the "if" statement in a when clause
> as above, but that gives a parse error.
> 
> I tried a number of variations, including a separate rule matching the
> "if" (which worked) and recording the "if"s position (which did not).
> For your amusement, one attempt follows.

I don't think when != @guardedref.q works at all, or if it does, then it 
is being interpreted as something other than what you intended.  When you 
inherit a metavariable, the name of the rule from which it is inherited 
only appears in the metavariable declaration.  So guardedref.q is a 
structure field reference.  And a position variable has to be attached to 
something.  In this case, it should be an identifier, because that is what 
the position was attached to in guardedref.  It can be any identifier at 
all, not necessarily an inherited one, because the position is enough to 
make it unique.

julia


 
> Thoughts?
> 
>                                                       Thanx, Paul
> 
> ------------------------------------------------------------------------
> 
> @guardedref@
> position q;
> identifier p;
> identifier f1;
> @@
>       if (p...@q) {
>               atomic_inc(&p->f1);
>       }
> 
> @rcupointerleak@
> identifier p;
> identifier f1, f2, f3;
> expression e1, e2, e3;
> expression c1;
> @@
> *     rcu_read_lock();
> 
>       ... when any
> 
> (
> *     p = rcu_dereference(e1);
> |
> *     p = rcu_dereference_raw(e1);
> |
> *     p = rcu_dereference_check(e1,c1);
> |
> *     p = rcu_dereference_index_check(e1,c1);
> |
> *     list_for_each_entry_rcu(p,e1,f3)
> |
> *     list_for_each_entry_continue_rcu(p,e1,f3)
> |
> *     hlist_for_each_entry_rcu(p,e1,f3)
> |
> *     hlist_for_each_entry_continue_rcu(p,e1,f3)
> )
> 
>       ... when != p = e2
>           when != atomic_inc_not_zero(&<+...p->f1...+>)
>           when != atomic_inc_not_zero(&<+...p[e2]...+>)
>           when != atomic_inc(&<+...p->f1...+>)
>           when != atomic_inc(&<+...p[e2]...+>)
>           when != try_module_get(p->owner)
>           when != @guardedref.q
> 
> *     rcu_read_unlock();
> 
>       ... when != p = e2
> 
> (
> *     *p
> |
> *     p->f2
> |
> *     p[e3]
> )
> 
_______________________________________________
Cocci mailing list
[email protected]
http://lists.diku.dk/mailman/listinfo/cocci
(Web access from inside DIKUs LAN only)

Reply via email to