[Cocci] Software analysis with SmPL around unchecked function calls

[Cocci] Software analysis with SmPL around unchecked function calls

[Markus Elfring]

Hello,

I would like to try another source code analysis approach out with
the software combination “Coccinelle 1.0.8-00004-g842075f7”.

@display@
expression x;
statement is, es;
@@
(
*x = kmemdup(...);
|if (...)
*x = kmemdup(...);
)
(if (!x) is
|if (...) is else es
|
 ... when any
     when != x
)


This SmPL small script can point an update candidate out like
the function “imx_pd_bind” as expected.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/imx/parallel-display.c?id=43b815c6a8e7dbccb5b8bd9c4b099c24bc22d135#n197
https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/gpu/drm/imx/parallel-display.c#L197

Unfortunately, I find also some false positives then at other places.

Example:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/cpufreq/sfi-cpufreq.c?id=8a8c600de5dc1d9a7f4b83269fddc80ebd3dd045#n23
https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/cpufreq/sfi-cpufreq.c#L23

…
@@ -37,7 +37,6 @@ static int sfi_parse_freq(struct sfi_tab
 	pentry = (struct sfi_freq_table_entry *)sb->pentry;
 	totallen = num_freq_table_entries * sizeof(*pentry);

-	sfi_cpufreq_array = kmemdup(pentry, totallen, GFP_KERNEL);
 	if (!sfi_cpufreq_array)
 		return -ENOMEM;
…


Would you like to clarify this situation for the semantic patch language?

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

Re: [Cocci] Software analysis with SmPL around unchecked function calls

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 1843 bytes --]



On Thu, 10 Oct 2019, Markus Elfring wrote:

> Hello,
>
> I would like to try another source code analysis approach out with
> the software combination “Coccinelle 1.0.8-00004-g842075f7”.
>
> @display@
> expression x;
> statement is, es;
> @@
> (
> *x = kmemdup(...);
> |if (...)
> *x = kmemdup(...);
> )
> (if (!x) is
> |if (...) is else es
> |
>  ... when any
>      when != x
> )


It's not clear what you want to match.  The above rule matches all kmemdup
calls that are followed by the the indicated code.  Do you want the
following?

Why not just

x = kmemdup(...);
... when != x

julia

>
>
> This SmPL small script can point an update candidate out like
> the function “imx_pd_bind” as expected.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/imx/parallel-display.c?id=43b815c6a8e7dbccb5b8bd9c4b099c24bc22d135#n197
> https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/gpu/drm/imx/parallel-display.c#L197
>
> Unfortunately, I find also some false positives then at other places.
>
> Example:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/cpufreq/sfi-cpufreq.c?id=8a8c600de5dc1d9a7f4b83269fddc80ebd3dd045#n23
> https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/cpufreq/sfi-cpufreq.c#L23
>
> …
> @@ -37,7 +37,6 @@ static int sfi_parse_freq(struct sfi_tab
>  	pentry = (struct sfi_freq_table_entry *)sb->pentry;
>  	totallen = num_freq_table_entries * sizeof(*pentry);
>
> -	sfi_cpufreq_array = kmemdup(pentry, totallen, GFP_KERNEL);
>  	if (!sfi_cpufreq_array)
>  		return -ENOMEM;
> …
>
>
> Would you like to clarify this situation for the semantic patch language?
>
> Regards,
> Markus
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

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

Re: [Cocci] Software analysis with SmPL around unchecked function calls

[Markus Elfring]

> Why not just
>
> x = kmemdup(...);
> ... when != x

I find this SmPL code exclusion specification too generic for the use case.
I would like to detect that a corresponding null pointer check would be missing
(before the data can be used for further data processing).

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

Re: [Cocci] Software analysis with SmPL around unchecked function calls

[Julia Lawall]

On Thu, 10 Oct 2019, Markus Elfring wrote:

> > Why not just
> >
> > x = kmemdup(...);
> > ... when != x
>
> I find this SmPL code exclusion specification too generic for the use case.
> I would like to detect that a corresponding null pointer check would be missing
> (before the data can be used for further data processing).

* x = kmemdup(...);
  ... when != x
(
  x->f
|
  f(...,<+...x...+>,...)
)

If this gives a parse error, put ... when any at the very end.

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

Re: [Cocci] Software analysis with SmPL around unchecked function calls

[Markus Elfring]

>> I would like to detect that a corresponding null pointer check would be missing
>> (before the data can be used for further data processing).
>
> * x = kmemdup(...);
>   ... when != x
> (
>   x->f
> |
>   f(...,<+...x...+>,...)
> )

This SmPL search approach does not work as expected for
the mentioned source file “drivers/gpu/drm/imx/parallel-display.c”.

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

Re: [Cocci] Software analysis with SmPL around unchecked function calls

[Markus Elfring]

>>>> I would like to detect that a corresponding null pointer check would be missing
>>>> (before the data can be used for further data processing).
>>>
>>> * x = kmemdup(...);
>>>   ... when != x
>>> (
>>>   x->f
>>> |
>>>   f(...,<+...x...+>,...)
>>> )
>>
>> This SmPL search approach does not work as expected for
>> the mentioned source file “drivers/gpu/drm/imx/parallel-display.c”.
>
> I have better things to do than to run your tests.  If you have a problem,
> say what the problem is an show the code that shows a problem.

A questionable source code place is just not pointed out
by this analysis approach.
No error message is displayed on my test system.

So I would appreciate to find a better solution together.

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

Re: [Cocci] Software analysis with SmPL around unchecked function calls

[Markus Elfring]

>> I would like to detect that a corresponding null pointer check would be missing
>> (before the data can be used for further data processing).
>
> * x = kmemdup(...);
>   ... when != x
> (
>   x->f
> |
>   f(...,<+...x...+>,...)
> )

This analysis approach looks promising in principle. I needed another moment
to become aware that it indicates pointer usage requirements
which can occasionally not be met (like in the mentioned function “imx_pd_bind”).
If the expression “x” would be split into accesses to data structure members,
the data processing can probably result in another desirable information display.

The pointer usages vary in affected function implementations (as usual).

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

Re: [Cocci] Software analysis with SmPL around unchecked function calls

[Markus Elfring]

> And what is the questionable source code place?

I find implementation details occasionally questionable where checks for variables
which provide a stored function return value are missing.
The possibility remains that also your search pattern suggestion will point
update candidates out at other places than the implementation of the mentioned
function “imx_pd_bind”.

How do you think about the following SmPL script variant?

@display@
expression x, y;
identifier f, md;
@@
*(x)->md = kmemdup(...);
 ... when != (x)->md
(((x)->md)->f
|f(..., <+... x ...+>, ...)
|(y = x)
)


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

Re: [Cocci] Software analysis with SmPL around unchecked function calls

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 956 bytes --]



On Fri, 11 Oct 2019, Markus Elfring wrote:

> > And what is the questionable source code place?
>
> I find implementation details occasionally questionable where checks for variables
> which provide a stored function return value are missing.
> The possibility remains that also your search pattern suggestion will point
> update candidates out at other places than the implementation of the mentioned
> function “imx_pd_bind”.

Blah, blah, blah.  So many words.  So little information.

What is the name of the file and the line number at which you get a result
that you find inacceptable with my rule?

If you can't answer that simple question, I'm not going to discuss this
further.

julia

>
> How do you think about the following SmPL script variant?
>
> @display@
> expression x, y;
> identifier f, md;
> @@
> *(x)->md = kmemdup(...);
>  ... when != (x)->md
> (((x)->md)->f
> |f(..., <+... x ...+>, ...)
> |(y = x)
> )
>
>
> Regards,
> Markus
>

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

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

Re: [Cocci] Software analysis with SmPL around unchecked function calls

[Markus Elfring]

>> The possibility remains that also your search pattern suggestion will point
>> update candidates out at other places than the implementation of the mentioned
>> function “imx_pd_bind”.
>
> So many words.  So little information.

This can also occasionally happen if the search approach is simpler
than it would be required for specific source code places.

The discussion context should be resolvable together with previous messages.

Repetition:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/imx/parallel-display.c?id=43b815c6a8e7dbccb5b8bd9c4b099c24bc22d135#n197
https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/gpu/drm/imx/parallel-display.c#L197


> What is the name of the file and the line number at which you get a result
> that you find inacceptable with my rule?

Another example:
Now I wonder about a patch hunk like the following which is generated
from the discussed search pattern suggestion.

@display@
expression x;
identifier f;
@@
* x = kmemdup(...);
  ... when != x
(
  x->f
|
  f(...,<+...x...+>,...)
)


elfring@Sonne:~/Projekte/Linux/next-patched> spatch ~/Projekte/Coccinelle/janitor/show_unchecked_kmemdup3.cocci net/sunrpc/auth_gss/auth_gss.c
…
@@ -146,7 +146,6 @@ simple_get_netobj(const void *p, const v
 	q = (const void *)((const char *)p + len);
 	if (unlikely(q > end || q < p))
 		return ERR_PTR(-EFAULT);
-	dest->data = kmemdup(p, len, GFP_NOFS);
 	if (unlikely(dest->data == NULL))
 		return ERR_PTR(-ENOMEM);
 	dest->len = len;


https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/sunrpc/auth_gss/auth_gss.c?id=9e208aa06c2109b45eec6be049a8e47034748c20#n149
https://elixir.bootlin.com/linux/v5.4-rc2/source/net/sunrpc/auth_gss/auth_gss.c#L149


Will the corresponding clarification help software developers (besides me)?

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

Re: [Cocci] Software analysis with SmPL around unchecked function calls

[Markus Elfring]

>> @display@
>> expression x;
>> identifier f;
>
> You can put f != {likely,unlikely} here.

I would appreciate to achieve a better understanding how these likeliness
annotations can influence the shown source code search approach.


> Maybe there will be some false positives when x->f is in a condition
> that previously checked that x is not NULL.

Such information can become more interesting.


> Does this happen a lot?

My view is incomplete.


> If the answer to either question is no, does the problem really matter?
> If it does really matter,

I hope that the probability for false positives (because of evolving
source code searches) can be considerably reduced.


> then it is possible to solve it, by adding a previous rule that
> marks such safe dereferences with a position variable.  But I don't know
> whether it is worth it.

I am curious how corresponding software development efforts will evolve.

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

Re: [Cocci] Software analysis with SmPL around unchecked function calls

[Markus Elfring]

>> @display@
>> expression x;
>> identifier f;
>
> You can put f != {likely,unlikely} here.

Now I have got a related impression. It seems that such a search pattern extension
affects also our unfinished clarification for the desired handling of
when constraints by SmPL ellipses.
How will this open issue evolve further?

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

Re: [Cocci] Software analysis with SmPL around unchecked pointer function calls

[Markus Elfring]

> Why not just

@display@
expression x, y;
@@
*y = (x)(...);
 ... when != y


It seems that such a source code search approach has got a high probability
for false positives, doesn't it?
Thus I am trying again to restrict it in reasonable ways.

Which would be a simple way to find only calls of functions
which have got a pointer return type?

Do any function signatures need to be taken into account
if you would not like to store a long function name list copy?

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

Re: [Cocci] Software analysis with SmPL around unchecked pointer function calls

[Julia Lawall]

On Fri, 18 Oct 2019, Markus Elfring wrote:

> > Why not just
>
> @display@
> expression x, y;
> @@
> *y = (x)(...);
>  ... when != y
>
>
> It seems that such a source code search approach has got a high probability
> for false positives, doesn't it?
> Thus I am trying again to restrict it in reasonable ways.
>
> Which would be a simple way to find only calls of functions
> which have got a pointer return type?

expression *y;

julia

> Do any function signatures need to be taken into account
> if you would not like to store a long function name list copy?
>
> Regards,
> Markus
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Software analysis with SmPL around unchecked pointer function calls

[Markus Elfring]

>> Which would be a simple way to find only calls of functions
>> which have got a pointer return type?
>
> expression *y;

I have got understanding difficulties with this SmPL variable specification.
I (as a C programmer) would tend to interpret an asterisk before
an expression as a pointer dereference (and not as a call with the desired
function return type).
Do I get an inappropriate impression here?

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

Re: [Cocci] Software analysis with SmPL around unchecked pointer function calls

[Julia Lawall]

On Fri, 18 Oct 2019, Markus Elfring wrote:

> >> Which would be a simple way to find only calls of functions
> >> which have got a pointer return type?
> >
> > expression *y;
>
> I have got understanding difficulties with this SmPL variable specification.
> I (as a C programmer) would tend to interpret an asterisk before
> an expression as a pointer dereference (and not as a call with the desired
> function return type).
> Do I get an inappropriate impression here?

Yes.

In int *i; there is no pointer dereference.

Y should match a pointer-typed expression.

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

Re: [Cocci] Software analysis with SmPL around unchecked pointer function calls

[Markus Elfring]

> In int *i; there is no pointer dereference.

Thanks for this clarification.

The semantic patch language syntax needs a different interpretation
of the desired meaning.
How do you think about to add the mentioned detail to the SmPL manual?


> Y should match a pointer-typed expression.

This view is appropriate.

But I would like to point an other data type distinction out
for the called function.
Thus I imagine that the following SmPL script variant would be incomplete.

@display@
expression* x, y;
@@
*y = (x)(...);
 ... when != y


The metavariable “x” can be restricted to a pointer expression.
But does such specification ensure also that the function pointer is connected
with a pointer return type?

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

Re: [Cocci] Software analysis with SmPL around unchecked pointer function calls

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 819 bytes --]



On Fri, 18 Oct 2019, Markus Elfring wrote:

> > In int *i; there is no pointer dereference.
>
> Thanks for this clarification.
>
> The semantic patch language syntax needs a different interpretation
> of the desired meaning.
> How do you think about to add the mentioned detail to the SmPL manual?
>
>
> > Y should match a pointer-typed expression.
>
> This view is appropriate.
>
> But I would like to point an other data type distinction out
> for the called function.
> Thus I imagine that the following SmPL script variant would be incomplete.
>
> @display@
> expression* x, y;
> @@
> *y = (x)(...);
>  ... when != y
>
>
> The metavariable “x” can be restricted to a pointer expression.
> But does such specification ensure also that the function pointer is connected
> with a pointer return type?

No.

julia

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

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

Re: [Cocci] Software analysis with SmPL around unchecked pointer function calls

[Markus Elfring]

>> The metavariable “x” can be restricted to a pointer expression.
>> But does such specification ensure also that the function pointer is connected
>> with a pointer return type?
>
> No.

Thanks for another clarification.

Will the distinction be improved for the safe usage of function pointers
also together with the semantic patch language?

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

Re: [Cocci] Software analysis with SmPL around unchecked pointer function calls

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 559 bytes --]



On Fri, 18 Oct 2019, Markus Elfring wrote:

> >> The metavariable “x” can be restricted to a pointer expression.
> >> But does such specification ensure also that the function pointer is connected
> >> with a pointer return type?
> >
> > No.
>
> Thanks for another clarification.
>
> Will the distinction be improved for the safe usage of function pointers
> also together with the semantic patch language?

I don't see any reason why declaring x as expression *x; should imply
anything about the type of the value returned by a function pointer.

julia

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

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

Re: [Cocci] Software analysis with SmPL around unchecked pointer function calls

[Markus Elfring]

>> Will the distinction be improved for the safe usage of function pointers
>> also together with the semantic patch language?
>
> I don't see any reason why declaring x as expression *x; should imply
> anything about the type of the value returned by a function pointer.

Function pointers have got also the property of a return type, don't they?

I would like to restrict the kind of called functions at the mentioned place
as precise as possible.

By the way:
Would we occasionally like to exclude the possibility for variable
assignments with incompatible pointer types?

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

Re: [Cocci] Software analysis with SmPL around unchecked pointer function calls

[Julia Lawall]

On Fri, 18 Oct 2019, Markus Elfring wrote:

> >> Will the distinction be improved for the safe usage of function pointers
> >> also together with the semantic patch language?
> >
> > I don't see any reason why declaring x as expression *x; should imply
> > anything about the type of the value returned by a function pointer.
>
> Function pointers have got also the property of a return type, don't they?

Function pointers certainly have a return type, but the return type could
be int.

julia

>
> I would like to restrict the kind of called functions at the mentioned place
> as precise as possible.
>
> By the way:
> Would we occasionally like to exclude the possibility for variable
> assignments with incompatible pointer types?
>
> Regards,
> Markus
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Software analysis with SmPL around unchecked pointer function calls

[Markus Elfring]

> Function pointers certainly have a return type,

This is fine as usual.


> but the return type could be int.

This can also happen.

But I would like to specify the restriction for a known pointer variant
at this place.

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

Re: [Cocci] Software analysis with SmPL around unchecked pointer function calls

[Markus Elfring]

> expression *y;

This specific information is interesting.

It was mentioned that further software development concerns
can occur around the possible application of function pointers.
Now I would like to point another analysis concern out.

The following SmPL search approach does not present the source code place

@display@
expression* x, y;
@@
*y = (x)(...);
 ... when != y


which can found by an other script variant for further considerations.

@display@
expression x;
@@
*x = device_link_add(...);
 ... when != x


https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/iommu/rockchip-iommu.c?id=c4b9850b3676869ac0def5885d781d17f64b3a86#n1057

elfring@Sonne:~/Projekte/Linux/next-patched> spatch drivers/iommu/rockchip-iommu.c ~/Projekte/Coccinelle/janitor/show_unchecked_device_link_add1.cocci
…
@@ -1072,8 +1072,6 @@ static int rk_iommu_add_device(struct de
 	iommu_group_put(group);

 	iommu_device_link(&iommu->iommu, dev);
-	data->link = device_link_add(dev, iommu->dev,
-				     DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME);

 	return 0;
 }


Would you like to check the desired data processing in more detail then?

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

Re: [Cocci] Software analysis with SmPL around unchecked pointer function calls

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 1357 bytes --]



On Fri, 18 Oct 2019, Markus Elfring wrote:

> > expression *y;
>
> This specific information is interesting.
>
> It was mentioned that further software development concerns
> can occur around the possible application of function pointers.
> Now I would like to point another analysis concern out.
>
> The following SmPL search approach does not present the source code place
>
> @display@
> expression* x, y;
> @@
> *y = (x)(...);
>  ... when != y

device_link_add does not look like a function pointer.

julia

>
>
> which can found by an other script variant for further considerations.
>
> @display@
> expression x;
> @@
> *x = device_link_add(...);
>  ... when != x
>
>
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/iommu/rockchip-iommu.c?id=c4b9850b3676869ac0def5885d781d17f64b3a86#n1057
>
> elfring@Sonne:~/Projekte/Linux/next-patched> spatch drivers/iommu/rockchip-iommu.c ~/Projekte/Coccinelle/janitor/show_unchecked_device_link_add1.cocci
> …
> @@ -1072,8 +1072,6 @@ static int rk_iommu_add_device(struct de
>  	iommu_group_put(group);
>
>  	iommu_device_link(&iommu->iommu, dev);
> -	data->link = device_link_add(dev, iommu->dev,
> -				     DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME);
>
>  	return 0;
>  }
>
>
> Would you like to check the desired data processing in more detail then?
>
> Regards,
> Markus
>

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

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

Re: [Cocci] Software analysis with SmPL around unchecked pointer function calls

[Markus Elfring]

>> @display@
>> expression* x, y;
>> @@
>> *y = (x)(...);
>>  ... when != y
>
> device_link_add does not look like a function pointer.

Can a known function name be equivalent to a function pointer?

See also:
https://en.cppreference.com/w/c/language/pointer#Pointers_to_functions
https://denniskubes.com/2013/03/22/basics-of-function-pointers-in-c/
https://en.wikipedia.org/wiki/Function_pointer#Example_in_C
https://www.learncpp.com/cpp-tutorial/78-function-pointers/

Does the semantic patch language (Coccinelle software) need an extra
data type conversion?

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

Re: [Cocci] Software analysis with SmPL around unchecked pointer function calls

[Markus Elfring]

> Now I would like to point another analysis concern out.

You informed me about an approach for the usage of when constraints
with the semantic patch language in the following way according to
the discussion topic “Detection of unused function return values”
(on 2011-12-11).

“…
Rule r checks that there is no use on any execution path.
…”


I came along the use case again to work with related information.
The currently discussed SmPL script variant points also the following
source code place out for further considerations.
https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/gpu/drm/arm/display/komeda/komeda_dev.c#L210
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/arm/display/komeda/komeda_dev.c?id=c4b9850b3676869ac0def5885d781d17f64b3a86#n222

…
@@ -222,… @@ struct komeda_dev *komeda_dev_create(str

 	clk_prepare_enable(mdev->aclk);

-	mdev->funcs = product->identify(mdev->reg_base, &mdev->chip);
 	if (!komeda_product_match(mdev, product->product_id)) {
…
	mdev->funcs->init_format_table(mdev);

	err = mdev->funcs->enum_resources(mdev);
…


Now I would appreciate once more if the description for the supported
software behaviour can be completed for the safe usage of SmPL
code exclusion specifications.
I see that a function pointer is appropriately used here.
Thus I wonder where my understanding of the software situation around
the program “spatch” seems still too incomplete.

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

Re: [Cocci] Software analysis with SmPL around unchecked pointer function calls

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 2012 bytes --]



On Sat, 19 Oct 2019, Markus Elfring wrote:

> > Now I would like to point another analysis concern out.
>
> You informed me about an approach for the usage of when constraints
> with the semantic patch language in the following way according to
> the discussion topic “Detection of unused function return values”
> (on 2011-12-11).
>
> “…
> Rule r checks that there is no use on any execution path.
> …”
>
>
> I came along the use case again to work with related information.
> The currently discussed SmPL script variant points also the following
> source code place out for further considerations.
> https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/gpu/drm/arm/display/komeda/komeda_dev.c#L210
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/arm/display/komeda/komeda_dev.c?id=c4b9850b3676869ac0def5885d781d17f64b3a86#n222
>
> …
> @@ -222,… @@ struct komeda_dev *komeda_dev_create(str
>
>  	clk_prepare_enable(mdev->aclk);
>
> -	mdev->funcs = product->identify(mdev->reg_base, &mdev->chip);
>  	if (!komeda_product_match(mdev, product->product_id)) {
> …
> 	mdev->funcs->init_format_table(mdev);
>
> 	err = mdev->funcs->enum_resources(mdev);
> …
>
>
> Now I would appreciate once more if the description for the supported
> software behaviour can be completed for the safe usage of SmPL
> code exclusion specifications.
> I see that a function pointer is appropriately used here.
> Thus I wonder where my understanding of the software situation around
> the program “spatch” seems still too incomplete.

I have no idea what you are asking about here.  Are you concerned that you
don't know the return type of mdev->funcs->init_format_table?  Coccinelle
has limited access to type information, as it may not be able to find some
relevant header files.  It doesn't even try very hard to find header files
unless you use options such as --all-includes or --recursive-includes,
which will incur a substantial performance overhead.

julia

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

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

Re: [Cocci] Software analysis with SmPL around unchecked pointer function calls

[Markus Elfring]

>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/arm/display/komeda/komeda_dev.c?id=c4b9850b3676869ac0def5885d781d17f64b3a86#n222
>>
>> …
>> @@ -222,… @@ struct komeda_dev *komeda_dev_create(str
>>
>>  	clk_prepare_enable(mdev->aclk);
>>
>> -	mdev->funcs = product->identify(mdev->reg_base, &mdev->chip);
>>  	if (!komeda_product_match(mdev, product->product_id)) {
>> …
>> 	mdev->funcs->init_format_table(mdev);
>>
>> 	err = mdev->funcs->enum_resources(mdev);
>> …
>>
>>
>> Now I would appreciate once more if the description for the supported
>> software behaviour can be completed for the safe usage of SmPL
>> code exclusion specifications.
>> I see that a function pointer is appropriately used here.
>> Thus I wonder where my understanding of the software situation around
>> the program “spatch” seems still too incomplete.
>
> I have no idea what you are asking about here.

I hope that our communication difficulties can eventually be reduced
also at this place.


> Are you concerned that you don't know the return type of mdev->funcs->init_format_table?

No. - It should be clear that this is a member function call, shouldn't it?
The used function pointer was set to the return value from a call of
the function “product->identify(…)”.
Should the subsequent statements (behind the line which was marked by
the SmPL asterisk functionality with a minus character) qualify for
the desired exclusion of pointer expressions by a SmPL when constraint
for the metavariable “y”?

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

Re: [Cocci] Software analysis with SmPL around unchecked pointer function calls

[Markus Elfring]

>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/arm/display/komeda/komeda_dev.c?id=c4b9850b3676869ac0def5885d781d17f64b3a86#n222
>>
>> …
>> @@ -222,… @@ struct komeda_dev *komeda_dev_create(str
>>
>>  	clk_prepare_enable(mdev->aclk);
>>
>> -	mdev->funcs = product->identify(mdev->reg_base, &mdev->chip);
>>  	if (!komeda_product_match(mdev, product->product_id)) {
>> …
>> 	mdev->funcs->init_format_table(mdev);
>>
>> 	err = mdev->funcs->enum_resources(mdev);
>> …
>>
>>
>> Now I would appreciate once more if the description for the supported
>> software behaviour can be completed for the safe usage of SmPL
>> code exclusion specifications.
…
> I have no idea what you are asking about here.

I hope that another wording approach can contribute another bit
to a better common understanding of the involved source code
analysis expectations.


> Are you concerned that you don't know the return type of mdev->funcs->init_format_table?

No, not in this test case.

This member function is declared with the return type “void”.
https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/gpu/drm/arm/display/komeda/komeda_dev.h#L83
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/arm/display/komeda/komeda_dev.h?id=c4b9850b3676869ac0def5885d781d17f64b3a86#n94

I would find this function call questionable otherwise.

The desired function is determined over the pointer “mdev->funcs”
which was provided by a call of the function “product->identify(…)”.
The provided function pointer is actually not directly checked
after the data structure member assignment.
This could be an analysis concern. (But it can be determined by inspection
of involved source files that a valid pointer will probably be set.
I assume that the exclusion of null pointers would be too challenging
for the discussed tiny SmPL script.)

The uncertainty around the partly (un)documented software behaviour
for SmPL when constraints makes it unclear then if the presented
source code place should finally be treated as a false positive.
Should it have been excluded because pointer expressions should be detectable
for the metavariable “y” (a bit later)?

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

Re: [Cocci] Software analysis with SmPL around unchecked pointer function calls

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 2604 bytes --]



On Sat, 19 Oct 2019, Markus Elfring wrote:

> >> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/arm/display/komeda/komeda_dev.c?id=c4b9850b3676869ac0def5885d781d17f64b3a86#n222
> >>
> >> …
> >> @@ -222,… @@ struct komeda_dev *komeda_dev_create(str
> >>
> >>  	clk_prepare_enable(mdev->aclk);
> >>
> >> -	mdev->funcs = product->identify(mdev->reg_base, &mdev->chip);
> >>  	if (!komeda_product_match(mdev, product->product_id)) {
> >> …
> >> 	mdev->funcs->init_format_table(mdev);
> >>
> >> 	err = mdev->funcs->enum_resources(mdev);
> >> …
> >>
> >>
> >> Now I would appreciate once more if the description for the supported
> >> software behaviour can be completed for the safe usage of SmPL
> >> code exclusion specifications.
> …
> > I have no idea what you are asking about here.
>
> I hope that another wording approach can contribute another bit
> to a better common understanding of the involved source code
> analysis expectations.
>
>
> > Are you concerned that you don't know the return type of mdev->funcs->init_format_table?
>
> No, not in this test case.
>
> This member function is declared with the return type “void”.
> https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/gpu/drm/arm/display/komeda/komeda_dev.h#L83
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/arm/display/komeda/komeda_dev.h?id=c4b9850b3676869ac0def5885d781d17f64b3a86#n94
>
> I would find this function call questionable otherwise.
>
> The desired function is determined over the pointer “mdev->funcs”
> which was provided by a call of the function “product->identify(…)”.
> The provided function pointer is actually not directly checked
> after the data structure member assignment.
> This could be an analysis concern. (But it can be determined by inspection
> of involved source files that a valid pointer will probably be set.
> I assume that the exclusion of null pointers would be too challenging
> for the discussed tiny SmPL script.)
>
> The uncertainty around the partly (un)documented software behaviour
> for SmPL when constraints makes it unclear then if the presented
> source code place should finally be treated as a false positive.
> Should it have been excluded because pointer expressions should be detectable
> for the metavariable “y” (a bit later)?

Coccinelle only knows the type of mdev->funcs if it sees the type
definition of mdev.  It doesn't take into account the subsequent usage of
mdev->funcs to determine that this value is a pointer.

julia

>
> Regards,
> Markus
>

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

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

Re: [Cocci] Software analysis with SmPL around unchecked pointer function calls

[Markus Elfring]

>> The uncertainty around the partly (un)documented software behaviour
>> for SmPL when constraints makes it unclear then if the presented
>> source code place should finally be treated as a false positive.

Are you going to do anything more for this concern?


>> Should it have been excluded because pointer expressions should be detectable
>> for the metavariable “y” (a bit later)?
>
> Coccinelle only knows the type of mdev->funcs if it sees the type
> definition of mdev.

This information can be appropriate.

But we can also guess that another pointer operation should usually happen
when another arrow was specified behind such an expression, can't we?


> It doesn't take into account the subsequent usage of mdev->funcs
> to determine that this value is a pointer.

Should this software behaviour be changed anyhow?

Can the data processing become more helpful around the influence
of pointer expressions?

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

Re: [Cocci] Software analysis with SmPL around unchecked pointer function calls

[Markus Elfring]

>> The uncertainty around the partly (un)documented software behaviour
>> for SmPL when constraints makes it unclear then if the presented
>> source code place should finally be treated as a false positive.

Will it be easier to clarify this open issue based on a smaller
source code example?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/chelsio/libcxgb/libcxgb_cm.c?id=531e93d11470aa2e14e6a3febef50d9bc7bab7a1#n76

static bool
cxgb_our_interface(struct cxgb4_lld_info *lldi,
		   struct net_device *(*get_real_dev)(struct net_device *),
		   struct net_device *egress_dev)
{
	int i;

	egress_dev = get_real_dev(egress_dev);
	for (i = 0; i < lldi->nports; i++)
		if (lldi->ports[i] == egress_dev)
			return true;
	return false;
}


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