On Fri, 11 Mar 2011, ygrek wrote:
> Hello,
>
> I am trying to write coccinelle patches for ocaml bindings API to
> automatically detect and fix
> common error cases.
> I have the following questions so far :
>
> 1)
> What is the best way to handle name #define's, i.e. caml/compatibility.h
> defines a bunch of names
> of the form :
> #define alloc_small caml_alloc_small
> and I want to use single name in cocci files, not both, for matching (any
> name can be present in .c files).
> Ideally I would like to reuse this compatibility.h without copying the
> definitions, but apparently this doesn't work, not
> even copying the definitions into local include or pasting directly into C
> source.
>
> $ cat test.cocci
> @@ expression SIZE, TAG; @@
> * caml_alloc_small(SIZE,TAG)
>
> $ cat test.h
> #define alloc_small caml_alloc_small
>
> $ cat test.c
>
> #include "test.h"
>
> void test()
> {
> value v = caml_alloc_small(3,0);
> value v2 = alloc_small(3,0);
> }
>
> Running spatch -all_includes -macro_file test.h -sp_file test.cocci test.c
> matches only one line, not both.
> IIUC this is because macro defines are used only for the .c code, but not for
> the code in the
> patches itself - correct? I figured out I can use isomorphisms to achieve
> this goal :
>
> $ cat caml.iso
> Expression
> @@ @@
> alloc_small <=> caml_alloc_small
>
> and @ using "caml.iso" @ in test.cocci does the trick, but I guess it is not
> quite the intended usage and
> I do not like to rewrite all #define's as isomorphisms..
This is the only option that comes to mind. The problem is that (to the
best of my knowledge) coccinelle only uses the definitions of macros if it
can't parse the code otherwise, and in your case it has no problem parsing
the code. Perhaps there is a way to generate the isomorphisms
automatically? You could do it using Coccinelle actually, since you can
match on #define I1 I2 and then write some python or ocaml code to print
out the isomorphism definition.
It would also be possible to match the #define in your SmPL rule, but this
would require anticipating each case where that could occur. You could
write, eg:
@d@
identifier I;
@@
#define I caml_alloc_small
@@
identifier d.I;
@@
(
- caml_alloc_small(...);
|
- I(...);
)
to get rid of a call to caml_alloc_small or whatever it has been renamed
as. You don't have to give compatibility.h as a -macro_file argument.
Just use -all_includes and -I with the path at which that file occurs.
> 2)
> First patch is to check assignments to allocated blocks. Blocks can be
> allocated as "structured" or "raw".
> This is detected by the tag paramater passed to caml_alloc. Ideally I would
> like to "evaluate" tag parameter
> (taking into consideration ocaml #define's) to a constant and compare it to
> maximum "structured" tag = No_scan_tag = 251,
> but this is not possible I guess? After some trial and error I came up with
> the following patch :
>
> $ cat caml.cocci
>
> @raw_block@
> local idexpression v;
> expression SIZE;
> identifier C ~=
> "^\(No_scan_tag\|Abstract_tag\|String_tag\|Double_tag\|Double_array_tag\|Custom_tag\)$"
> ;
> @@
> v = caml_alloc(SIZE,C)
I don't think there is any need to use a regular expression here, since as
far as I can see you just want to match a set of fixed names. In the
second argument to caml_alloc you can put:
\(No_scan_tag\|Abstract_tag\|String_tag\|Double_tag\|...\)
(where ... is the rest of your names, not the SmPL ...)
If you do this, the SmPL compiler has more information and can better
optimize your rule.
local idexpression v means that v is restricted to be a local variable.
If you just want it to be a variable, but don't care about it being a
local one, you can drop the local. If you don't even care about it being
a variable, eg you would like to allow a->b, then you can just put
expression.
Constant is for constants, such as 27. But it also considers an
identifier that is all capital letters (possibly containing numbers) as a
constant as well, because the names gives to #define in Linux usually have
this form.
> @@
> expression FIELD, VAL;
> local idexpression raw_block.v;
> @@
>
> - Store_field(v,FIELD,VAL)
> + Field(v,FIELD) = VAL
>
> @struct_block@
> local idexpression v;
> expression SIZE;
> identifier C !~=
> "^\(No_scan_tag\|Abstract_tag\|String_tag\|Double_tag\|Double_array_tag\|Custom_tag\)$"
> ;
> constant TAG;
> @@
> v = caml_alloc(SIZE,\(C\|TAG\))
>
> @@
> expression FIELD, VAL;
> local idexpression struct_block.v;
> @@
>
> - Field(v,FIELD) = VAL
> + Store_field(v,FIELD,VAL)
>
> $ cat caml.h
>
> #define CAMLprim
> #define CAMLextern
> #define CAMLunused
>
> #define CAMLlocal1(x) value x
>
> and it correctly detects all /* FIX */ lines in the following code :
>
> $ cat test.c
>
> void q()
> {
> value exceptionData;
>
> exceptionData = caml_alloc(3, 0);
>
> Field(exceptionData, 0) = Val_int(code); /* FIX */
> Field(exceptionData, 1) = Val_int(code); /* FIX */
> Field(exceptionData, 2) = copy_string(errorString); /* FIX */
> }
>
> void q()
> {
> // value exceptionData = caml_alloc(2,1);
> Field(exceptionData, 0) = Val_int(code); /* OK */
>
> value x = caml_alloc(3,No_scan_tag);
> Field(x,0) = 2; /* OK */
> Store_field(x,0,2); /* FIX */
> }
>
> It was not obvious what kinds of metavariables to use - i.e. what is the
> exact meaning of
> constant vs identifier vs local idexpression, etc? Is is ok to match e.g.
> No_scan_tag as identifier
> while it is really a #define? Any hints to improve this patch?
An identifier is the name of a structure field, a #define, a function, or
a variable. Is is the name of something rather than an expression that
has a value. But it is a bit confusing, because an identifier can be used
in the position of an expression as well.
> 3)
> Do you think overall coccinelle is a correct tool for such a task?
If there are a lot of actual calculations that have to be performed, then
perhaps not. But if you are mostly working at the syntactic structure,
then that is fine.
julia
_______________________________________________
Cocci mailing list
[email protected]
http://lists.diku.dk/mailman/listinfo/cocci
(Web access from inside DIKUs LAN only)
