[Cocci] problems with typecasts

[Carl-Daniel Hailfinger]

On 05.03.2009 16:24, Julia Lawall wrote:
> On Thu, 5 Mar 2009, Carl-Daniel Hailfinger wrote:
>   
>> I'm trying to convert the opensource flashrom program from direct
>> pointer and memory manipulation to helper functions.
>>
>> Current code looks like this:
>>
>> volatile uint8_t *bios = flash->virtual_memory;
>> uint8_t id;
>> *bios = 0x01;
>> *(bios + 0xbeef) = 0x02;
>> *(volatile uint8_t *)bios = 0x03;
>> *(volatile uint8_t *)(bios + 0xbeef) = 0x04;
>> id = *bios;
>> if (*bios & 0x80)
>>     do_something();
>>
>> The transformed code should look like this:
>>
>> volatile uint8_t *bios = flash->virtual_memory;
>> writeb(0x01, bios);
>> writeb(0x02, bios + 0xbeef);
>> writeb(0x03, bios);
>> writeb(0x04, bios + 0xbeef);
>> id = readb(bios);
>> if (readb(bios) & 0x80)
>>     do_something();
>>     
>
> Seems like a good use of Coccinelle :)
>   

Yes indeed. I used sed scripts before, but Coccinelle is a real
time-saver and the ability to check variable types helps catching cases
which are impossible to change with regular expressions.


>> The semantic patch does not work as intended, though:
>> - The typecasts removal from the second writeb() argmument does not look
>> specific enough, but seems to work fine anyway.
>>     
>
> If you want the cast to be more specific, it should be possible.  In the 
> version of Coccinelle you have, though, there might be a problem with 
> putting volatile in casts.  If you get a parse error, and if you have the 
> source code of Coccinelle, check whether the cast_expr entry in 
> parsing_cocci/parser_cocci_menhir.mly looks like the following:
>
> cast_expr(r,pe):
>     unary_expr(r,pe)                      { $1 }
>   | lp=TOPar t=ctype rp=TCPar e=cast_expr(r,pe)
>       { Ast0.wrap(Ast0.Cast (P.clt2mcode "(" lp, t,
> 			     P.clt2mcode ")" rp, e)) }
>
>   

Coccinelle 0.1.5 is what I'm using and it has the code you posted.

cast_expr(r,pe):
    unary_expr(r,pe)                      { $1 }
  | lp=TOPar t=ctype rp=TCPar e=cast_expr(r,pe)
      { Ast0.wrap(Ast0.Cast (P.clt2mcode "(" lp, t,
                             P.clt2mcode ")" rp, e)) }



>> - Reads outside an assignment are not matched (the if clause).
>>     
>
> This is because you have a eg *b = a;.  If you drop the semicolon, it 
> should work
>   

I tried this and failed.
@@
expression a;
volatile uint8_t *b;
@@
- a = *b
+ a = readb(b)

still does not match
if ((*bios & 0x80) == 0) {      // it's busy
    while ((*bios & 0x80) == 0) ;
}

That's probably because I also match the equal sign. I'd rather avoid
creating a separate semantic patch for every possible formula containing
a read access to *bios.
Is it possible to tell Coccinelle to just match on read accesses? Or
maybe a right-hand-side expression?


>> - Code inside #if 0 sections is not touched.
>>     
>
> Just use the option -noif0_passing
>   

Great, thanks. Must have overlooked that part of the man page.


>> And spatch spews the following warning message:
>> (ONCE) Type annotater:not handling GetRefLabel
>>     
>
> This is probably not a problem, unless you find that sometimes it is not 
> deducing that some variable has type uint8_t *
>   

Manual inspection revealed no problems, so I'll ignore that message.

Thank you!

Regards,
Carl-Daniel