headers
Julia Lawall | 1 Sep 2010 21:07
Picon
Favicon

Re: Question on Coccinelle

[cocci mailing list added in CC]

On Wed, 1 Sep 2010, Paul E. McKenney wrote:

> On Wed, Sep 01, 2010 at 09:44:23AM -0700, Paul E. McKenney wrote:
> > On Wed, Sep 01, 2010 at 08:06:18AM +0200, Julia Lawall wrote:
> 
> [ . . . ]
> 
> > Hmmmm...  I am getting hit by loops:
> > 
> > diff = 
> > --- drivers/net/cnic.c	2010-08-16 11:54:39.000000000 -0700
> >  <at>  <at>  -2101,13 +2101,10  <at>  <at>  static void service_kcqes(struct cnic_de
> >  			goto end;
> >  		}
> >  
> > -		rcu_read_lock();
> > -		ulp_ops = rcu_dereference(cp->ulp_ops[ulp_type]);
> >  		if (likely(ulp_ops)) {
> > -			ulp_ops->indicate_kcqes(cp->ulp_handle[ulp_type],
> >  						  cp->completed_kcq + i, j);
> >  		}
> > -		rcu_read_unlock();
> >  end:
> >  		num_cqes -= j;
> >  		i += j;
> > 
> > 
> > It took me a bit to figure out that the complaint was due to the fact
(Continue reading)

Permalink | Reply | 6 comments |
headers
Julia Lawall | 1 Sep 2010 21:53
Picon
Favicon

Re: Question on Coccinelle

> And so if I really had wanted to key off of the initial rcu_read_lock(),
> I could have done so as follows, correct?

I'm not sure what the goal of the following is.  Actually, I'm not even 
sure that regular expressions work for complex expressions like 
rcu_read_lock(), but even if they do, then in the second rule, rcurscs.r 
should be a metavariable that matches anything that rcu_read_lock() 
matches.  So the effect is no different than inlining rcu_read_lock() in 
the places where you use the r metavariable.

Maybe, you mean that r should reference a lock call at a specific position 
in the code.  If that is what you want, then you can use position 
variables, as in the following simpler examples:

 <at>  <at> 
position p;
 <at>  <at> 

*foo <at> p();
...
*bar();
... when != foo <at> p();
*xxx();

This matches the following code:

int main () {
  foo();
  bar();
  foo();
(Continue reading)

Permalink | Reply | 4 comments |
headers
Paul E. McKenney | 1 Sep 2010 21:33
Picon

Re: Question on Coccinelle

On Wed, Sep 01, 2010 at 09:07:07PM +0200, Julia Lawall wrote:
> [cocci mailing list added in CC]
> 
> On Wed, 1 Sep 2010, Paul E. McKenney wrote:
> 
> > On Wed, Sep 01, 2010 at 09:44:23AM -0700, Paul E. McKenney wrote:
> > > On Wed, Sep 01, 2010 at 08:06:18AM +0200, Julia Lawall wrote:
> > 
> > [ . . . ]
> > 
> > > Hmmmm...  I am getting hit by loops:
> > > 
> > > diff = 
> > > --- drivers/net/cnic.c	2010-08-16 11:54:39.000000000 -0700
> > >  <at>  <at>  -2101,13 +2101,10  <at>  <at>  static void service_kcqes(struct cnic_de
> > >  			goto end;
> > >  		}
> > >  
> > > -		rcu_read_lock();
> > > -		ulp_ops = rcu_dereference(cp->ulp_ops[ulp_type]);
> > >  		if (likely(ulp_ops)) {
> > > -			ulp_ops->indicate_kcqes(cp->ulp_handle[ulp_type],
> > >  						  cp->completed_kcq + i, j);
> > >  		}
> > > -		rcu_read_unlock();
> > >  end:
> > >  		num_cqes -= j;
> > >  		i += j;
> > > 
> > >
(Continue reading)

Permalink | Reply | 5 comments |
headers
Julia Lawall | 2 Sep 2010 07:25
Picon
Favicon

Re: Question on Coccinelle

> > 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
(Continue reading)

Permalink | Reply | 2 comments |
headers
Paul E. McKenney | 1 Sep 2010 23:46
Picon

Re: Question on Coccinelle

On Wed, Sep 01, 2010 at 09:53:23PM +0200, Julia Lawall wrote:
> > And so if I really had wanted to key off of the initial rcu_read_lock(),
> > I could have done so as follows, correct?
> 
> I'm not sure what the goal of the following is.  Actually, I'm not even 
> sure that regular expressions work for complex expressions like 
> rcu_read_lock(), but even if they do, then in the second rule, rcurscs.r 
> should be a metavariable that matches anything that rcu_read_lock() 
> matches.  So the effect is no different than inlining rcu_read_lock() in 
> the places where you use the r metavariable.
> 
> Maybe, you mean that r should reference a lock call at a specific position 
> in the code.  If that is what you want, then you can use position 
> variables, as in the following simpler examples:
> 
>  <at>  <at> 
> position p;
>  <at>  <at> 

Got it!

> *foo <at> p();
> ...
> *bar();
> ... when != foo <at> p();
> *xxx();
> 
> This matches the following code:
> 
> int main () {
(Continue reading)

Permalink | Reply | 3 comments |
headers
Julia Lawall | 2 Sep 2010 17:47
Picon
Favicon

Re: Question on Coccinelle

On Thu, 2 Sep 2010, Paul E. McKenney wrote:

> On Thu, Sep 02, 2010 at 07:25:02AM +0200, Julia Lawall wrote:
> > > > 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.
> 
> Ah, got it!
> 
> > 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...+>); ... }
(Continue reading)

Permalink | Reply | 0 comments |
headers
Paul E. McKenney | 2 Sep 2010 17:29
Picon

Re: Question on Coccinelle

On Thu, Sep 02, 2010 at 07:25:02AM +0200, Julia Lawall wrote:
> > > 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.

Ah, got it!

> 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
(Continue reading)

Permalink | Reply | 1 comment |
headers
Joe Perches | 9 Sep 2010 07:32

spatch defect: C90 initializers?

$ cat find_pci_name.cocci
 <at>  <at> 
struct pci_driver * I;
 <at>  <at> 

* I->name
$

The cocci script above does not find pci_driver->name references
that are set by c90 initializers like:

$ grep "struct pci_driver atl1_driver" drivers/net/atlx/atl1.c -A 8
drivers/net/atlx/atl1.c:static struct pci_driver atl1_driver = {
drivers/net/atlx/atl1.c-	.name = ATLX_DRIVER_NAME,
drivers/net/atlx/atl1.c-	.id_table = atl1_pci_tbl,
drivers/net/atlx/atl1.c-	.probe = atl1_probe,
drivers/net/atlx/atl1.c-	.remove = __devexit_p(atl1_remove),
drivers/net/atlx/atl1.c-	.suspend = atl1_suspend,
drivers/net/atlx/atl1.c-	.resume = atl1_resume,
drivers/net/atlx/atl1.c-	.shutdown = atl1_shutdown
drivers/net/atlx/atl1.c-};

$ cocci -sp_file find_pci_name.cocci drivers/net/atlx/atl1.c
init_defs_builtins: /usr/local/share/coccinelle/standard.h
HANDLING: drivers/net/atlx/atl1.c
$
Permalink | Reply | 5 comments |
headers
Julia Lawall | 9 Sep 2010 08:28
Picon
Favicon

Re: spatch defect: C90 initializers?

There is nothing that really tries to do this.  The semantic match 
specifies a pointer, and a top-level structure is a structure, not a 
pointer to a structure.  There is an isomorphism that converts a field 
reference to a pointer reference:

Expression
 <at>  fld_to_ptr  <at> 
type T;
pure T E;
pure T *E1;
identifier fld;
 <at>  <at> 

E.fld => E1->fld

There is also an isomorphism that converts a field initialization to a C90 
initialization:

TopLevel
 <at>  mkinit  <at> 
type T;
pure context T E;
identifier I;
identifier fld;
expression E1;
 <at>  <at> 

E.fld = E1; => T I = { .fld = E1, };

It could be possible to add the following isomorphism:
(Continue reading)

Permalink | Reply | 4 comments |
headers
Julia Lawall | 9 Sep 2010 09:46
Picon
Favicon

Re: spatch defect: C90 initializers?

On Wed, 8 Sep 2010, Joe Perches wrote:

> On Thu, 2010-09-09 at 08:28 +0200, Julia Lawall wrote:
> > you could also just add a second rule to your semantic match:
> > 
> >  <at>  <at> 
> > identifier I;
> >  <at>  <at> 
> > 
> > * struct pci_driver I { .name = ..., };
> 
> That doesn't seem to work.

My sloppy C.  There should of course be an = before the {.  But also for 
some reason it doesn't like a * on the ...  This is probably a parsing 
problem that I can fix.  The following works:

 <at>  <at> 
expression E;
 <at>  <at> 

* struct pci_driver I = { .name = E, };

> $ cat find_pci_name.cocci
>  <at>  <at> 
> identifier I;
>  <at>  <at> 
> 
> * struct pci_driver I { .name = ..., }
> $ cat foo.c
(Continue reading)

Permalink | Reply | 2 comments |

Gmane