On Mon, 16 Jun 2014, Travis Friesen wrote:
> Hi Julia
>
> Thanks for your (extremely rapid) response. I'm trying your suggestion
> now, but it's led me to two minor issues.
>
> First, Coccinnelle seems to get confused by "(short unsigned)" casts (as
> opposed to the more typical "unsigned short" usage). It throws a
> "strange type1, maybe because of weird order: short unsigned")" error
> when I use the following rule:
>
> @@
> typedef uint16_t;
> @@
>
> -unsigned short
> +uint16_t
>
> My initial reaction is to hunt down the developer here who originally
> decided to write casts as "(short unsigned)" and break his legs, but
> thankfully it only occurs a handful of times in our code base, so it was
> easier to just manually fix each instance than threaten someone with
> bodily harm. I am going to check if this is considered valid C at all,
> but gcc does seem to accept it.
I'm a little surprised that Coccinelle complains about this when it is not
in the generated code. But putting them around in the right way seems
like a good option for the future of the code base.
> The second has to do with a macro expansion. The rule
>
> @@
> typedef int32_t;
> @@
>
> (
> -int
> +int32_t
> |
> -long
> +int32_t
> )
>
> Throws a " Fatal error: exception
> Failure("/home/trav/src/autopilot/Sim/Drone.c: 109: try to delete an expanded
> token: int")" on the following macro:
>
> #define round(x) (int)(((x) >= 0) ? (floor((x) + 0.5)) : (-floor(-(x) +
> 0.5)));
>
> Again, such occurrences are few and far between, and can be fixed
> manually. At least, in this file, they are few and far between. In case
> another file has more such occurrences, how would I solve this issue?
The problem is not the definition of the macro, but rather with its uses.
The code is really pretty strange. The expansion of the macro is written
as an expression, but it has a semicolon at the end. So I guess it is
supposed to be used as something like:
x = round(a)
rather than
x = round(a);
This doesn't seem like a very good design. Eg if (x < round(a)) ...
wouldn't compile at all. Would it be an option to remove the semicolon in
the macro definition, and add semicolons at the macro uses?
It might also be possible to add #define round(x) x; to a macro definition
file that is given with the argument --macro-file-builtins. Then,
hopefully, it will replace (in the AST) the uses of the macro by something
that doesn't contain a type.
> Also, as a NB (and I think this is mentioned elsewhere) but the use of
> certain macros like UNUSED() would cause coccinelle to skip full functions.
> By adding
>
> #define UNUSED(x) (x)
>
> To standard.h, my problems seemed to evaporate.
Yes. I can't anticipate all of the strange macros that may be used by all
software. It can be a good idea to first run spatch --parse-c dirname.
That will end by telling you the 10 tokens that it most commonly had
trouble parsing. Usually if you give definitions for these, or for some
token near these, then you will be able to parse most of your code, or at
least most of the code that matters for what you are trying to do.
julia
>
>
> Thanks again for your help. Once I have this cocci script completely squared
> away, I'll submit it back to the list, to save other people future work :)
>
> Travis
>
> -----Original Message-----
> From: Julia Lawall [mailto:[email protected]]
> Sent: June-16-14 11:10 AM
> To: Travis Friesen
> Cc: [email protected]
> Subject: Re: [Cocci] Type change errors
>
> > @ rule1 @
> >
> > identifier i1;
> >
> > @@
> >
> >
> >
> > (
> >
> > - unsigned int i1;
> >
> > + uint32_t i1;
>
> This is not a good idea, because it is overspecified. All you want to do is
> change the type. You don't need to specify that the type has to be in a
> variable declaration. Here is what I tried:
>
> @@
> typedef uint_32;
> @@
>
> -unsigned int
> +uint_32
>
> On the code:
>
> int main(unsigned int x) {
> unsigned int y,z;
> return (unsigned int)(y+z);
> }
>
> You may have problems with variable declarations that include multiple
> variable. I have been working on that quite a bit recently, and I don't
> remember what is the status of that in the current release. Maybe there will
> be a new release soon.
>
> > Catch the bulk of our 32-bit integer usage in declarative statements,
> > but it’s obviously going to miss a lot of stuff, including function
> > definitions and prototypes. My attempts to use more sophisticated
> > rules, such as
> >
> >
> >
> > @ rule25 @
> >
> > identifier i2;
> >
> > @@
> >
> >
> >
> > unsigned
> >
> > (
> >
> > - long i2;
> >
> > |
> >
> > - int i2;
> >
> > )
> >
> > + uint32_t i2;
>
> You can't do this. You can't put a + on the outside of a disjunction.
> The + code has to be repeated in both branches.
>
> > Or
> >
> >
> >
> > @ rule4 @
> >
> > identifier fn;
> >
> > identifier i2;
> >
> > @@
> >
> >
> >
> > fn(...
> >
> > - int i2
> >
> > + int32_t i2
> >
> > ...)
>
> Two problems here. First, the elements of a parameter list need to be
> separated by commas. This includes ... Second, a pattern has to be a
> complete term. The outer part of this looks like a function call (function
> name, open and close parentheses, etc), but then the types mean that it can't
> be that. To match a function definition, it needs a body, ie { ... }.
> Normally, if you change the header of a definition, it will change any
> prototypes that it finds at the same time. But if the prototype is in a
> header file, and it doesn't find the header file when it is working on the C
> file, the prototype won't get transformed.
>
> But anyway, if you just make a rule on types, and not on contexts of types,
> the issue becomes irrelevant.
>
> > Result in a mysterious (and unhelpful) “minus: parse error...” Even
> > trying to omit the semicolons does not work. The very basic:
>
> Normally when you get a parse error it should tell you the line number and
> offset in the line of the problem. But it may take some getting used to the
> grammar to know what is being pointed at.
>
> > @ rule3 @
> >
> > identifier i2;
> >
> > @@
> >
> >
> >
> > - int i2
> >
> > + int32_t i2
> >
> >
> >
> >
> >
> > Also throws a “minus parse error...”. But from what I understand, the
> > semicolon should not be a necessary component to Coccinelle syntax.
>
> The semicolon is necessary. The semicolon is not necessary on eg
>
> x = 12
>
> because in C x = 12 is an expression. x = 12; is a statement that consists
> of a single expression, which is x = 12. This is why you can write eg if (x
> = foo()) ... But int i2 is not a complete anything. It needs a semicolon.
>
> julia
> _______________________________________________
Cocci mailing list
[email protected]
https://systeme.lip6.fr/mailman/listinfo/cocci
