Re: [Cocci] Matching function prototypes and casts

[Julia Lawall]

> > For me it works.  Do you have the latest version of Coccinelle from
> > github?  I used the option --all-includes.
>
> Ah-ha! Thank you. :) I had --no-includes in my .cocci. :)

>From a performance point of view, you might want to do what you can with
--no-includes first.  It will then only be doing the --all-includes on
files that still contain init_timer.

> More insane corner cases:
>
> I have this function:
>
> static void
> hfcmulti_dbusy_timer(struct hfc_multi *hc)
> {
> }
>
> And this rule (which is using the working change_timer_function_usage
> rule to find identifiers):
>
> // callback(struct something *handle)
> @change_callback_handle_arg
>  depends on patch &&
> change_timer_function_usage &&
> !change_callback_handle_cast@
> identifier change_timer_function_usage._callback;
> identifier change_timer_function_usage._timer;
> type _handletype;
> identifier _handle;
> @@
>
> -static void _callback(_handletype *_handle)
> +static void _callback(struct timer_list *t)
>  {
> +   _handletype *_handle = TIMER_CONTAINER(_handle, t, _timer);
> ...
>  }
>
> It correctly produces:
>
> -static void
> -hfcmulti_dbusy_timer(struct hfc_multi *hc)
> +static void hfcmulti_dbusy_timer(struct timer_list *t)
>  {
> +   struct hfc_multi *hc = TIMER_CONTAINER(hc, t, timer);
>  }
>
> But since this was an empty function originally, I don't want to add
> just the variable declaration. I tried various ()-like things, but
> they didn't work:
>
> -static void _callback(_handletype *_handle)
> +static void _callback(struct timer_list *t)
>  {
> (
> +   _handletype *_handle = TIMER_CONTAINER(_handle, t, _timer);
> ...
> |
> )
>  }
>
> etc...

Really what you care about is whether _handle was used.  I think that you
will need two rules:

@exists@
@@

-static void _callback(_handletype *_handle)
+static void _callback(struct timer_list *t) {
+  _handletype *_handle = TIMER_CONTAINER(_handle, t, _timer);
  ...
  _handle
  ... when any
}

and

-static void _callback(_handletype *_handle)
+static void _callback(struct timer_list *t) { ...
}

to clean up the rest.

julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Matching function prototypes and casts

[Kees Cook]

On Wed, Aug 23, 2017 at 2:19 PM, Julia Lawall  wrote:
>
>
> On Wed, 23 Aug 2017, Kees Cook wrote:
>
>> I think I'm getting closer. Here are some specific examples that don't
>> seem to work:
>>
>> ---match_callback.cocci---
>> virtual patch
>>
>> @match_timer_function_usage
>>  depends on patch@
>> expression _E;
>> struct timer_list _e;
>> identifier _timer;
>> identifier _callback;
>> type _cast_func, _cast_data;
>> @@
>>
>> (
>> -setup_timer(&_E->_timer@_e, _callback, _E);
>> |
>> -setup_timer(&_E->_timer@_e, &_callback, _E);
>> |
>> -setup_timer(&_E->_timer@_e, _callback, (_cast_data)_E);
>> |
>> -setup_timer(&_E->_timer@_e, &_callback, (_cast_data)_E);
>> |
>> -setup_timer(&_E->_timer@_e, (_cast_func)_callback, _E);
>> |
>> -setup_timer(&_E->_timer@_e, (_cast_func)&_callback, _E);
>> |
>> -setup_timer(&_E->_timer@_e, (_cast_func)_callback, (_cast_data)_E);
>> |
>> -setup_timer(&_E->_timer@_e, (_cast_func)&_callback, (_cast_data)_E);
>> |
>> -_E->_timer@_e.function = _callback;
>> |
>> -_E->_timer@_e.function = &_callback;
>> |
>> -_E->_timer@_e.function = (_cast_func)_callback;
>> |
>> -_E->_timer@_e.function = (_cast_func)&_callback;
>> )
>> ---EOF---
>>
>> Doesn't match drivers/ide/ide-probe.c which has:
>>
>> setup_timer(>timer, _timer_expiry, (unsigned long)hwif);
>>
>> Even this doesn't:
>>
>> (
>> -setup_timer(&_E->_timer@_e, &_callback, (_cast_data)_E);
>> )
>>
>> Unless I remove the "@_e" part...? Am I using that wrong?
>
> For me it works.  Do you have the latest version of Coccinelle from
> github?  I used the option --all-includes.

Ah-ha! Thank you. :) I had --no-includes in my .cocci. :)

More insane corner cases:

I have this function:

static void
hfcmulti_dbusy_timer(struct hfc_multi *hc)
{
}

And this rule (which is using the working change_timer_function_usage
rule to find identifiers):

// callback(struct something *handle)
@change_callback_handle_arg
 depends on patch &&
change_timer_function_usage &&
!change_callback_handle_cast@
identifier change_timer_function_usage._callback;
identifier change_timer_function_usage._timer;
type _handletype;
identifier _handle;
@@

-static void _callback(_handletype *_handle)
+static void _callback(struct timer_list *t)
 {
+   _handletype *_handle = TIMER_CONTAINER(_handle, t, _timer);
...
 }

It correctly produces:

-static void
-hfcmulti_dbusy_timer(struct hfc_multi *hc)
+static void hfcmulti_dbusy_timer(struct timer_list *t)
 {
+   struct hfc_multi *hc = TIMER_CONTAINER(hc, t, timer);
 }

But since this was an empty function originally, I don't want to add
just the variable declaration. I tried various ()-like things, but
they didn't work:

-static void _callback(_handletype *_handle)
+static void _callback(struct timer_list *t)
 {
(
+   _handletype *_handle = TIMER_CONTAINER(_handle, t, _timer);
...
|
)
 }

etc...

-Kees

-- 
Kees Cook
Pixel Security
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Matching function prototypes and casts

[Kees Cook]

I think I'm getting closer. Here are some specific examples that don't
seem to work:

---match_callback.cocci---
virtual patch

@match_timer_function_usage
 depends on patch@
expression _E;
struct timer_list _e;
identifier _timer;
identifier _callback;
type _cast_func, _cast_data;
@@

(
-setup_timer(&_E->_timer@_e, _callback, _E);
|
-setup_timer(&_E->_timer@_e, &_callback, _E);
|
-setup_timer(&_E->_timer@_e, _callback, (_cast_data)_E);
|
-setup_timer(&_E->_timer@_e, &_callback, (_cast_data)_E);
|
-setup_timer(&_E->_timer@_e, (_cast_func)_callback, _E);
|
-setup_timer(&_E->_timer@_e, (_cast_func)&_callback, _E);
|
-setup_timer(&_E->_timer@_e, (_cast_func)_callback, (_cast_data)_E);
|
-setup_timer(&_E->_timer@_e, (_cast_func)&_callback, (_cast_data)_E);
|
-_E->_timer@_e.function = _callback;
|
-_E->_timer@_e.function = &_callback;
|
-_E->_timer@_e.function = (_cast_func)_callback;
|
-_E->_timer@_e.function = (_cast_func)&_callback;
)
---EOF---

Doesn't match drivers/ide/ide-probe.c which has:

setup_timer(>timer, _timer_expiry, (unsigned long)hwif);

Even this doesn't:

(
-setup_timer(&_E->_timer@_e, &_callback, (_cast_data)_E);
)

Unless I remove the "@_e" part...? Am I using that wrong?

-Kees
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [PATCH] coccinelle: Improve setup_timer.cocci matching

[Kees Cook]

On Wed, Aug 23, 2017 at 6:13 AM, Julia Lawall  wrote:
>
>
> On Tue, 22 Aug 2017, Kees Cook wrote:
>
>> This improves the patch mode of setup_timer.cocci. Several patterns were
>> missing:
>>  - assignments-before-init_timer() cases
>>  - limiting the .data case removal to the struct timer_list instance
>>  - handling calls by dereference (timer->field vs timer.field)
>>
>> Running this on the current kernel tree produces a large diff that I
>> would like to get applied as the first step in removing the .data
>> field from struct timer_list:
>>
>>  208 files changed, 367 insertions(+), 757 deletions(-)
>>
>> Signed-off-by: Kees Cook 
>> ---
>>  scripts/coccinelle/api/setup_timer.cocci | 129 
>> +--
>>  1 file changed, 105 insertions(+), 24 deletions(-)
>>
>> diff --git a/scripts/coccinelle/api/setup_timer.cocci 
>> b/scripts/coccinelle/api/setup_timer.cocci
>> index eb6bd9e4ab1a..bc6bd8f0b4bf 100644
>> --- a/scripts/coccinelle/api/setup_timer.cocci
>> +++ b/scripts/coccinelle/api/setup_timer.cocci
>> @@ -2,6 +2,7 @@
>>  /// and data fields
>>  // Confidence: High
>>  // Copyright: (C) 2016 Vaishali Thakkar, Oracle. GPLv2
>> +// Copyright: (C) 2017 Kees Cook, Google. GPLv2
>>  // Options: --no-includes --include-headers
>>  // Keywords: init_timer, setup_timer
>>
>> @@ -10,60 +11,123 @@ virtual context
>>  virtual org
>>  virtual report
>>
>> +// Match the common cases first to avoid Coccinelle parsing loops with
>> +// "... when" clauses.
>> +
>>  @match_immediate_function_data_after_init_timer
>>  depends on patch && !context && !org && !report@
>>  expression e, func, da;
>>  @@
>>
>> --init_timer ();
>> -+setup_timer (, func, da);
>> +-init_timer
>> ++setup_timer
>> + ( \(\|e\)
>> ++, func, da
>> + );
>> +(
>> +-\(e.function\|e->function\) = func;
>> +-\(e.data\|e->data\) = da;
>> +|
>> +-\(e.function\|e->function\) = func;
>> +-\(e.data\|e->data\) = da;
>
> Same thing twice; I think you want to invert the last two lines.
>
>> +)
>> +
>> +@match_immediate_function_data_before_init_timer
>> +depends on patch && !context && !org && !report@
>> +expression e, func, da;
>> +@@
>>
>>  (
>> +-\(e.function\|e->function\) = func;
>> +-\(e.data\|e->data\) = da;
>> +|
>> +-\(e.function\|e->function\) = func;
>> +-\(e.data\|e->data\) = da;
>
> Same as in the previous case.

Oops, thanks! AIUI, missing these cases must makes runtime slower.
I'll fix it up and resend.

-Kees

-- 
Kees Cook
Pixel Security
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] parsing of C code

[Julia Lawall]

On Wed, 23 Aug 2017, Derek M Jones wrote:

> Julia,
>
> > I don't think that is what it is.  The declarations are eg:
> >
> > static inline __printf(2, 3)
> > void _dev_info(const struct device *dev, const char *fmt, ...)
> > {}
> >
> > I guess that the whole first line is part of the declaration of _dev_info,
> > but Coccinelle can't cope with the __printf(2, 3).
>
> https://stackoverflow.com/questions/17825588/what-does-this-generic-function-do
>
> > > Skip to ; is a remarkably effective syntax error recovery strategy.
> >
> > The parser needs to restart at a top-level declaration.  It's a yacc-based
> > parser.  We can't recover within the parsing process.  Perhaps it would be
> > possible to remove what was betwen two ;s around the line and column with
> > the error, but it seems like there could be a risk of making things worse.
>
> If it's yacc based you can recover where ever you like.  Knowing how to
> do it is something of a black art.

Well, ocamlyacc, to be precise.

>
> Bison supports a technique that does not require wearing a pointy hat:
>
> stmt_list: error ';' |
>stmt_list error ';' ;
>
> where error represents the something-went-wrong token.

This does look like a nice feature.

julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] parsing of C code

[Derek M Jones]

Julia,


I don't think that is what it is.  The declarations are eg:

static inline __printf(2, 3)
void _dev_info(const struct device *dev, const char *fmt, ...)
{}

I guess that the whole first line is part of the declaration of _dev_info,
but Coccinelle can't cope with the __printf(2, 3).


https://stackoverflow.com/questions/17825588/what-does-this-generic-function-do


Skip to ; is a remarkably effective syntax error recovery strategy.


The parser needs to restart at a top-level declaration.  It's a yacc-based
parser.  We can't recover within the parsing process.  Perhaps it would be
possible to remove what was betwen two ;s around the line and column with
the error, but it seems like there could be a risk of making things worse.


If it's yacc based you can recover where ever you like.  Knowing how to
do it is something of a black art.

Bison supports a technique that does not require wearing a pointy hat:

stmt_list: error ';' |
   stmt_list error ';' ;

where error represents the something-went-wrong token.


--
Derek M. Jones   Software analysis
tel: +44 (0)1252 520667  blog:shape-of-code.coding-guidelines.com
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] parsing of C code

[Julia Lawall]

On Wed, 23 Aug 2017, Derek M Jones wrote:

> Julia,
>
> > The Linux kernel has some declarations like
> >
> > type fn(a1, a2) >
> > with no trailing ; followed by a function definition.  This gives a parse
> > error on a1.  In the previous version, Coccinelle was able to recover an
>
> The K style.  Perfectly legal C.

I don't think that is what it is.  The declarations are eg:

static inline __printf(2, 3)
void _dev_info(const struct device *dev, const char *fmt, ...)
{}

I guess that the whole first line is part of the declaration of _dev_info,
but Coccinelle can't cope with the __printf(2, 3).

>
> > Basically the recovery process is focused on finding a { in column 0 and
> > then a } in column 0, and then it goes on after that.  But it can go
> > backwards sometimes, because a parsing problem can cause a parsing attempt
> > to read too far, into the next function.
>
> Skip to ; is a remarkably effective syntax error recovery strategy.

The parser needs to restart at a top-level declaration.  It's a yacc-based
parser.  We can't recover within the parsing process.  Perhaps it would be
possible to remove what was betwen two ;s around the line and column with
the error, but it seems like there could be a risk of making things worse.

julia

>
>
> --
> Derek M. Jones   Software analysis
> tel: +44 (0)1252 520667  blog:shape-of-code.coding-guidelines.com
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

>> How should the source code search be improved further here?
> 
> If you like the results from the second case, what more do you want?

* There are also several questionable transformation suggestions generated
  besides the usable ones.

* Will answers belong also to the topic “Get the non-optional nest construct
  completely working for SmPL”?
  https://github.com/coccinelle/coccinelle/issues

Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

> You would need when any on the ...  Otherwise, it will not match anything
> (perhaps declarations). The s1 after is precludes matching any statement.
> Likewise in the next if.

The suggested variant does still not work in the way I would expect it.

@duplicated_code@
identifier work;
statement s1, s2;
type T;
@@
 T work(...)
 {
 ... when any
*if ((...) < 0)
*{
... when any
*   s1
*   s2
*}
 <+...
*if ((...) < 0)
*{
... when any
*   s1
*   s2
*}
 ...+>
 }


I got some promising test results from the following SmPL approach.

@duplicated_code@
identifier work;
statement s1, s2;
type T;
@@
 T work(...)
 {
 ... when any
*if ((...) < 0)
*{
... when any
*   s1
*   s2
*}
 ... when any
*if ((...) < 0)
*{
... when any
*   s1
*   s2
*}
 ... when any
 }


elfring@Sonne:~/Projekte/Linux/next-patched> XX=$(date) && spatch.opt --timeout 
12 --sp-file ~/Projekte/Coccinelle/janitor/show_same_statements3b.cocci --dir 
sound > 
~/Projekte/Bau/Linux/scripts/Coccinelle/tuning1/next/20170803/same_statements3b.diff
 2> 
~/Projekte/Bau/Linux/scripts/Coccinelle/tuning1/next/20170803/same_statements3b-errors.txt;
 YY=$(date) && echo "$XX | $YY"
Mi 23. Aug 17:25:54 CEST 2017 | Mi 23. Aug 17:29:15 CEST 2017


How should the source code search be improved further here?

Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

> I have tried another variant out for a source code analysis.

Does the following SmPL script variant make sense?

@duplicated_code@
identifier work;
statement s1, s2;
type T;
@@
 T work(...)
 {
 ... when any
*if ((...) < 0)
*{
...
*   s1
*   s2
*}
 <+...
*if ((...) < 0)
*{
...
*   s1
*   s2
*}
 ...+>
 }


Can this approach find interesting places in any source files?

Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

[Cocci] [PATCH] Coccinelle: setup_timer: improve messages from setup_timer

[Julia Lawall]

Allow messages about multiple timers.

Signed-off-by: Julia Lawall 

---
 scripts/coccinelle/api/setup_timer.cocci |   15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/scripts/coccinelle/api/setup_timer.cocci 
b/scripts/coccinelle/api/setup_timer.cocci
index eb6bd9e..b5ab031 100644
--- a/scripts/coccinelle/api/setup_timer.cocci
+++ b/scripts/coccinelle/api/setup_timer.cocci
@@ -104,11 +104,9 @@ position j0, j1, j2;
 )
 
 @match_function_and_data_after_init_timer_context
-depends on !patch &&
-!match_immediate_function_data_after_init_timer_context &&
- (context || org || report)@
+depends on !patch && (context || org || report)@
 expression a, b, e1, e2, e3, e4, e5;
-position j0, j1, j2;
+position j0 != match_immediate_function_data_after_init_timer_context.j0,j1,j2;
 @@
 
 * init_timer@j0 ();
@@ -124,13 +122,12 @@ position j0, j1, j2;
 * e1@j2.function = a;
 )
 
-@r3_context depends on !patch &&
-!match_immediate_function_data_after_init_timer_context &&
-!match_function_and_data_after_init_timer_context &&
- (context || org || report)@
+@r3_context depends on !patch && (context || org || report)@
 expression c, e6, e7;
 position r1.p;
-position j0, j1;
+position j0 !=
+  {match_immediate_function_data_after_init_timer_context.j0,
+   match_function_and_data_after_init_timer_context.j0}, j1;
 @@
 
 * init_timer@j0@p ();

___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [PATCH] coccinelle: Improve setup_timer.cocci matching

[Julia Lawall]

On Tue, 22 Aug 2017, Kees Cook wrote:

> This improves the patch mode of setup_timer.cocci. Several patterns were
> missing:
>  - assignments-before-init_timer() cases
>  - limiting the .data case removal to the struct timer_list instance
>  - handling calls by dereference (timer->field vs timer.field)
>
> Running this on the current kernel tree produces a large diff that I
> would like to get applied as the first step in removing the .data
> field from struct timer_list:
>
>  208 files changed, 367 insertions(+), 757 deletions(-)
>
> Signed-off-by: Kees Cook 
> ---
>  scripts/coccinelle/api/setup_timer.cocci | 129 
> +--
>  1 file changed, 105 insertions(+), 24 deletions(-)
>
> diff --git a/scripts/coccinelle/api/setup_timer.cocci 
> b/scripts/coccinelle/api/setup_timer.cocci
> index eb6bd9e4ab1a..bc6bd8f0b4bf 100644
> --- a/scripts/coccinelle/api/setup_timer.cocci
> +++ b/scripts/coccinelle/api/setup_timer.cocci
> @@ -2,6 +2,7 @@
>  /// and data fields
>  // Confidence: High
>  // Copyright: (C) 2016 Vaishali Thakkar, Oracle. GPLv2
> +// Copyright: (C) 2017 Kees Cook, Google. GPLv2
>  // Options: --no-includes --include-headers
>  // Keywords: init_timer, setup_timer
>
> @@ -10,60 +11,123 @@ virtual context
>  virtual org
>  virtual report
>
> +// Match the common cases first to avoid Coccinelle parsing loops with
> +// "... when" clauses.
> +
>  @match_immediate_function_data_after_init_timer
>  depends on patch && !context && !org && !report@
>  expression e, func, da;
>  @@
>
> --init_timer ();
> -+setup_timer (, func, da);
> +-init_timer
> ++setup_timer
> + ( \(\|e\)
> ++, func, da
> + );
> +(
> +-\(e.function\|e->function\) = func;
> +-\(e.data\|e->data\) = da;
> +|
> +-\(e.function\|e->function\) = func;
> +-\(e.data\|e->data\) = da;

Same thing twice; I think you want to invert the last two lines.

> +)
> +
> +@match_immediate_function_data_before_init_timer
> +depends on patch && !context && !org && !report@
> +expression e, func, da;
> +@@
>
>  (
> +-\(e.function\|e->function\) = func;
> +-\(e.data\|e->data\) = da;
> +|
> +-\(e.function\|e->function\) = func;
> +-\(e.data\|e->data\) = da;

Same as in the previous case.

julia


> +)
> +-init_timer
> ++setup_timer
> + ( \(\|e\)
> ++, func, da
> + );
> +
> +@match_function_and_data_after_init_timer
> +depends on patch && !context && !org && !report@
> +expression e, e2, e3, e4, e5, func, da;
> +@@
> +
> +-init_timer
> ++setup_timer
> + ( \(\|e\)
> ++, func, da
> + );
> + ... when != func = e2
> + when != da = e3
> +(
>  -e.function = func;
> +... when != da = e4
>  -e.data = da;
>  |
> +-e->function = func;
> +... when != da = e4
> +-e->data = da;
> +|
>  -e.data = da;
> +... when != func = e5
>  -e.function = func;
> +|
> +-e->data = da;
> +... when != func = e5
> +-e->function = func;
>  )
>
> -@match_function_and_data_after_init_timer
> +@match_function_and_data_before_init_timer
>  depends on patch && !context && !org && !report@
> -expression e1, e2, e3, e4, e5, a, b;
> +expression e, e2, e3, e4, e5, func, da;
>  @@
> -
> --init_timer ();
> -+setup_timer (, a, b);
> -
> -... when != a = e2
> -when != b = e3
>  (
> --e1.function = a;
> -... when != b = e4
> --e1.data = b;
> +-e.function = func;
> +... when != da = e4
> +-e.data = da;
>  |
> --e1.data = b;
> -... when != a = e5
> --e1.function = a;
> +-e->function = func;
> +... when != da = e4
> +-e->data = da;
> +|
> +-e.data = da;
> +... when != func = e5
> +-e.function = func;
> +|
> +-e->data = da;
> +... when != func = e5
> +-e->function = func;
>  )
> +... when != func = e2
> +when != da = e3
> +-init_timer
> ++setup_timer
> + ( \(\|e\)
> ++, func, da
> + );
>
>  @r1 exists@
> +expression t;
>  identifier f;
>  position p;
>  @@
>
>  f(...) { ... when any
> -  init_timer@p(...)
> +  init_timer@p(\(\|t\))
>... when any
>  }
>
>  @r2 exists@
> +expression r1.t;
>  identifier g != r1.f;
> -struct timer_list t;
>  expression e8;
>  @@
>
>  g(...) { ... when any
> -  t.data = e8
> +  \(t.data\|t->data\) = e8
>... when any
>  }
>
> @@ -77,14 +141,31 @@ p << r1.p;
>  cocci.include_match(False)
>
>  @r3 depends on patch && !context && !org && !report@
> -expression e6, e7, c;
> +expression r1.t, func, e7;
>  position r1.p;
>  @@
>
> --init_timer@p ();
> -+setup_timer (, c, 0UL);
> -... when != c = e7
> --e6.function = c;
> +(
> +-init_timer@p();
> ++setup_timer(, func, 0UL);
> +... when != func = e7
> +-t.function = func;
> +|
> +-t.function = func;
> +... when != func = e7
> +-init_timer@p();
> ++setup_timer(, func, 0UL);
> +|
> +-init_timer@p(t);
> ++setup_timer(t, func, 0UL);
> +... when != func = e7
> +-t->function = func;
> +|
> +-t->function = func;
> +... when != func = e7
> +-init_timer@p(t);
> ++setup_timer(t, func, 0UL);
> +)
>
>  // 
> 
>
> --
> 2.7.4
>
>
> --
> Kees Cook
> Pixel Security
>
___

Re: [Cocci] Matching function prototypes and casts

[Julia Lawall]

On Tue, 22 Aug 2017, Kees Cook wrote:

> Hi,
>
> I'm working on a large type conversion for struct timer_list, and
> it'll require changing all of the callback prototypes and changing
> logic at their start. For example, if I start with this:
>
>
> void primary_callback(unsigned long arg);
> void alternate_callback(struct some_handle *handle);
> ...
>
> void primary_callback(unsigned long arg)
> {
> struct some_handle *handle = (struct some_handle *)arg;
> ...
> }
>
> void alternate_callback(struct some_handle *handle)
> {
> ...
> }
>
> ...
> setup_timer(>timer_field, some_callback, (unsigned long)ptr);
> ...
> ptr->timer_field.function = (void (*)(unsigned long)alternate_callback;
>
>
> I'd like to end up with:
>
>
> void primary_callback(struct timer_list *t);
> void alternate_callback(struct timer_list *t);
> ...
>
> void primary_callback(struct timer_list *t)
> {
> struct some_handle *handle = container_of(t, struct some_handle,
> timer_field);
> ...
> }
>
> void alternate_callback(struct timer_list *t)
> {
> struct some_handle *handle = container_of(t, struct some_handle,
> timer_field);
> ...
> }
>
> ...
> setup_time(>timer_field, some_callback, >timer_field);
> ...
> ptr->timer_field.function = alternate_callback;
>
>
> I'm having trouble matching the callback arg type, and I'm having
> trouble identifying the callbacks. I think I can do things like this,
> but it's not quite working:
>
> @match_setup_timer@
> identifier _p;
> struct timer_list _timer_field;
> identifier _callback;
> type _cast;
> @@
>
> (
>  setup_timer(&_p->_timer_field, _setup_callback, _p);
> |
>  setup_timer(&_p->_timer_field, (_cast)_setup_callback, _p);

The code has setup_callback but the metavariable is just callback.

> )
>
> @match_function_assignment@
> identifier _p;
> struct timer_list _timer_field;

This is declaring a metavariable that matches an expression of type struct
timer_list.  You are trying to match a field name, so you just want an
identifier metavariable.  If you want to be sure that _p->_timer_field has
type struct timer_list, then you can make an expression metavariable of
that type, ie:

struct timer_list e;

and then put @e after _timer_field.  But maybe you don't even need
_p->_timer_field and can just use e?

> identifier _callback;
> type _cast;
> @@
>
> (
>  _p->_timer_field.function = _callback;
> |
>  _p->_timer_field.function = (_cast)_callback;
> )
>
> // callback(unsigned long arg)
> @change_callback_handle_cast
> depends on match_setup_timer || match_function_assignment@
> identifier ???._callback;
> struct timer_list ???._timer_field;
> type _origtype;
> identifier _origarg;
> type _handletype;
> identifier _handle;
> @@
>
> -void _callback(_origtype _origarg)
> +void _callback(struct timer_list *t)
>  {
> -_handletype *_handle = (_handletype *)_origarg;
> +_handletype *_handle = container_of(t, _handletype, _timer_field);
>
> // callback(struct something *handle)
> @change_callback_handle_arg
> depends on match_setup_timer || match_function_assignment@
> identifier ???._callback;

Is the concern here that you want to get the metavariable binding from two
different rules?

It might be possible to just have two metavariables inherited from
different places, and then have a disjunction in the function name
position.

Alternatively, maybe those two rules could be merged.

julia

> struct timer_list ???._timer_field;
> type _handletype;
> identifier _handle;
> @@
>
> -void _callback(_handletype *_handle)
> +void _callback(struct timer_list *t)
>  {
> +_origtype *_origarg = container_of(t, _origtype, _timer_field);
>
>
> Namely, I don't seem to be matching casts or function arg types, and I
> sometimes seem to be missing cases of struct timer_list. Also, I can't
> figure out how to have the change_callback_* rules depend on either
> matching match_setup_timer or match_function_assignment and having
> access to the matched fields...
>
> What would be the best way to deal with these?
>
> Thanks!
>
> -Kees
>
> --
> Kees Cook
> Pixel Security
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] parsing of C code

[Julia Lawall]

On Tue, 22 Aug 2017, Derek M Jones wrote:

> Julia,
>
> > 1.  More aggressive inclusion of header files, combined with caching of
> > header files.  Now if there is only one occurrence of a header file with a
>
> Caching can be dangerous because macros may be defined differently
> for different includes of the same header.  An option to switch off
> caching could come in handy.

I can add such an option.

Actually, I noticed that unfolding macros can sometimes hurt more than it
helps.  When Coccinelle decides to unfold macros it unfolds them for all
the rest of the code, but it only unfolds one level of macro.  Sometimes
the one-level unfolding is weirder than the original and the parser error
that triggered unfoding is not related to macros anyway.  So then the
whole rest of the code has a weird semi-unfolded macro in it for no
purpose, whe it could have compiled normally.  At least for the Linux
kernel, though, this seems like a rare occurrence.

julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] parsing of C code

[Julia Lawall]

On Tue, 22 Aug 2017, Derek M Jones wrote:

> Julia,
>
> > 1.  More aggressive inclusion of header files, combined with caching of
> > header files.  Now if there is only one occurrence of a header file with a
>
> Caching can be dangerous because macros may be defined differently
> for different includes of the same header.  An option to switch off
> caching could come in handy.

I believe that macros are only applied if they are defined in the same
file.  At this point, I'm not 100% certain of that.  But when I ran the
provided test script on the Linux kernel, I list very few functions as
compared to the current --recursive-includes implementation.  The lost
functions were due to the recovery strategy, not to macro issues.
The Linux kernel has some declarations like

type fn(a1, a2)

with no trailing ; followed by a function definition.  This gives a parse
error on a1.  In the previous version, Coccinelle was able to recover an
parse the function definition.  In the new version, I comment out the
arguments and then another error is encountered that is after the ).  This
causes the recovery process to skip over the subsequent function.  The
recovery code is rather subtle to avoid infinite loops, and so far my
attempts to improve it have led to more of a mess than anything else.

The problem could be solved by not trying to patch up errors in
parentheses when they occur at top level.  When they occur inside a
function, they should not impact the recovery process.

Basically the recovery process is focused on finding a { in column 0 and
then a } in column 0, and then it goes on after that.  But it can go
backwards sometimes, because a parsing problem can cause a parsing attempt
to read too far, into the next function.

>
> > given name in the provided include paths, it will take that one, even if
> > there is no obvious connection between the location of the .c file and the
> > location of the header file.  This compensates for the lact of parsing of
> > Makefiles to extract -I options.  More header files will likely now be
>
> There is no need to parse Makefiles.  Simply create a script, say coc99,
> and configure it as the compiler that make uses.  coc99 parses its
> arguments to extract whatever information cocci needs and passes
> everything to the 'real' C compiler as-is.

At least for the Linux kernel, you can't just run one make and get all the
files to be compiled.  Some files are indeed very hard to compile.  For a
more uniform project this could work, though.

> How is the O'Reilly book coming along ;-)

Still looking for an author :)

julia

>
> --
> Derek M. Jones   Software analysis
> tel: +44 (0)1252 520667  blog:shape-of-code.coding-guidelines.com
> ___
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci