On Sat, 5 Jun 2021, Jason A. Donenfeld wrote:
> Hi,
>
> I'm trying to write a spatch rule that has some inversion logic for a
> given struct member, but I'm struggling to express this in SmPL. I'm
> also a bit of a coccinelle novice, however. Specifically, I'm trying
> to express:
>
> For all structs, for each member `M` of type `struct rwlock`, if there
> are calls to `rwlock_wlock(&M)` and `rwlock_wunlock(&M)`, but there
> are no calls to `rwlock_rlock(&M)` or `rwlock_runlock(&M)`, then
> change `M`'s type from `struct rwlock` to `struct lock` and change
> calls to `rwlock_wlock(&M)` and `rwlock_wunlock(&M)` to
> `lock_lock(&M)` and `lock_unlock(&M)`, respectively.
>
> In other words, "if I'm not using the reader part of an rwlock, make
> it into a normal boring lock."
>
> I realize that tracking whether, in a call to f(&something->M),
> something is actually of the type in which rwlock was replaced is kind
> of hard (I think?). But I'd be willing to compromise on just assuming
> that something->M is always correct, because M always has
> unique-enough names. Or maybe that compromise isn't needed due to some
> sort of amazing coccinelle type inference, but anyway, I'm willing to
> compromise.
>
> The trickiest part seems to be, however, only doing this in the case
> where there aren't calls to `rwlock_rlock(&M)` and
> `rwlock_runlock(&M)`. I tried using a virtual rule and a depends
> clause, but those dependencies don't seem to work over a given
> instance of an identifier.
>
> I could probably hack my way toward that with a ridiculous sed
> expression, or do it procedurally after parsing the AST. But it seems
> like this would be a good case for where Coccinelle makes things
> easier, so I thought I'd commit to getting it done with spatch. Any
> pointers would be appreciated.
A possible semantic patch is below. Note that the first line with the
#spatch gives some implicit command line options. You may want to change
the number of cores. Note that the semantic patch will directly modify
your code, so don't run it on anything you dont' want to destroy...
There are two parts. The first part finds relevant structures and locking
functions, and stores which operations are used on which structures in
some hash tables. That part is run over the entire code base.
After the completion of the first part, the second part looks for
structure definitions and calls that can be changed.
Hopefully the code based doesn't have multiple definitions of the same
structure with different properties. If that is a concern, it could be
possible to collect the names of the relevant structure definitions in the
first phase as well, and complain about or ignore structure names that are
defined twice.
julia
#spatch -j 4 --recursive-includes --include-headers-for-types --include-headers
--in-place
virtual after_start
@initialize:ocaml@
@@
let has_write_table = Hashtbl.create 101
let has_read_table = Hashtbl.create 101
let ok i m =
let entry = (i,m) in
Hashtbl.mem has_write_table entry && not(Hashtbl.mem has_read_table entry)
@hasw depends on !after_start@
identifier i,m;
struct i x;
@@
(
rwlock_wlock(&x.m)
|
rwlock_wunlock(&x.m)
)
@script:ocaml@
i << hasw.i;
m << hasw.m;
@@
Hashtbl.replace has_write_table (i,m) ()
@hasr depends on !after_start@
identifier i,m;
struct i x;
@@
(
rwlock_rlock(&x.m)
|
rwlock_runlock(&x.m)
)
@script:ocaml@
i << hasr.i;
m << hasr.m;
@@
Hashtbl.replace has_read_table (i,m) ()
@finalize:ocaml depends on !after_start@
wt << merge.has_write_table;
rt << merge.has_read_table;
@@
let redo ts dst =
List.iter (Hashtbl.iter (fun k _ -> Hashtbl.add dst k ())) ts in
redo wt has_write_table;
redo rt has_read_table;
let it = new iteration() in
it#add_virtual_rule After_start;
it#register()
(* ----------------------------------------------------------- *)
@ty2 depends on after_start@
identifier i;
identifier m : script:ocaml(i) { ok i m };
@@
struct i {
...
- struct rwlock m;
+ struct lock m;
...
}
@depends on after_start disable fld_to_ptr@
identifier m;
identifier i : script:ocaml(m) { ok i m };
struct i x;
@@
- rwlock_wlock
+ lock_lock
(&x.m)
@depends on after_start disable fld_to_ptr@
identifier m;
identifier i : script:ocaml(m) { ok i m };
struct i x;
@@
- rwlock_wunlock
+ lock_unlock
(&x.m)
@depends on after_start disable fld_to_ptr, ptr_to_array@
identifier m;
identifier i : script:ocaml(m) { ok i m };
struct i *x;
@@
- rwlock_wlock
+ lock_lock
(&x->m)
@depends on after_start disable fld_to_ptr, ptr_to_array@
identifier m;
identifier i : script:ocaml(m) { ok i m };
struct i *x;
@@
- rwlock_wunlock
+ lock_unlock
(&x->m)
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
