[Cocci] Checking for a null pointer with SmPL

[Cocci] Checking for a null pointer with SmPL

[Markus Elfring]

Hello,

A patch on a topic like “staging/rtl8723bs/core/rtw_ap: Remove redundant call
to memset” caught also my software development attention.
https://lkml.org/lkml/2019/6/15/220
https://lore.kernel.org/patchwork/patch/1089416/
https://lore.kernel.org/lkml/20190616033527.GA14062@hari-Inspiron-1545/


The following script for the semantic patch language points the shown
change possibility out as expected.

@display@
expression ex;
identifier zm =~ "_zmalloc";
statement is;
@@
 ex = zm(...);
 if (ex == NULL)
    is

*memset(ex, 0, ...);



I would expect that the following SmPL script can work in a similar way.

@display@
expression ex;
identifier zm =~ "_zmalloc";
statement is, es;
@@
 ex = zm(...);
 if (ex)
    is
 else
    es

*memset(ex, 0, ...);


But this approach does not point an update candidate out at the moment.
How do you think about the software situation?

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

Re: [Cocci] Checking for a null pointer with SmPL

[Julia Lawall]

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



On Sun, 16 Jun 2019, Markus Elfring wrote:

> Hello,
>
> A patch on a topic like “staging/rtl8723bs/core/rtw_ap: Remove redundant call
> to memset” caught also my software development attention.
> https://lkml.org/lkml/2019/6/15/220
> https://lore.kernel.org/patchwork/patch/1089416/
> https://lore.kernel.org/lkml/20190616033527.GA14062@hari-Inspiron-1545/
>
>
> The following script for the semantic patch language points the shown
> change possibility out as expected.
>
> @display@
> expression ex;
> identifier zm =~ "_zmalloc";
> statement is;
> @@
>  ex = zm(...);
>  if (ex == NULL)
>     is
>
> *memset(ex, 0, ...);
>
>
>
> I would expect that the following SmPL script can work in a similar way.
>
> @display@
> expression ex;
> identifier zm =~ "_zmalloc";
> statement is, es;
> @@
>  ex = zm(...);
>  if (ex)
>     is
>  else
>     es
>
> *memset(ex, 0, ...);
>
>
> But this approach does not point an update candidate out at the moment.
> How do you think about the software situation?

In the second case, Coccinelle has no way of knowing that ex is a pointer,
so it doesn't feel motivated to consider that ex should be compared to
NULL.

I would imagine that you would have gooten at least some relevant
information if you had tried spatch --parse-cocci.

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] Checking for a null pointer with SmPL

[Markus Elfring]

> In the second case, Coccinelle has no way of knowing that ex is a pointer,

Can the software support pointer expressions better?


> so it doesn't feel motivated to consider that ex should be compared to NULL.

Should an isomorphism specification like “not_ptr2” help here?
https://github.com/coccinelle/coccinelle/blob/19ee1697bf152d37a78a20cefe148775bf4b0e0d/standard.iso#L157


> I would imagine that you would have gooten at least some relevant
> information if you had tried spatch --parse-cocci.

This data display can give useful hints.
Are we waiting also for a more complete software documentation in
such an application area?

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

Re: [Cocci] Checking for a null pointer with SmPL

[Markus Elfring]

>> Should an isomorphism specification like “not_ptr2” help here?
>> https://github.com/coccinelle/coccinelle/blob/19ee1697bf152d37a78a20cefe148775bf4b0e0d/standard.iso#L157
>
> No, because Coccinelle does not know that it is a pointer.
> How should it know?

Can the software determine that the expression metavariable refers to
a pointer variable (within the implementation of a function like “rtw_ap_set_key”)?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/rtl8723bs/core/rtw_ap.c?id=9e0babf2c06c73cda2c0cd37a1653d823adb40ec#n1479
https://elixir.bootlin.com/linux/v5.2-rc5/source/drivers/staging/rtl8723bs/core/rtw_ap.c#L1479

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

Re: [Cocci] Checking for a null pointer with SmPL

[Julia Lawall]

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



On Mon, 17 Jun 2019, Markus Elfring wrote:

> >> Should an isomorphism specification like “not_ptr2” help here?
> >> https://github.com/coccinelle/coccinelle/blob/19ee1697bf152d37a78a20cefe148775bf4b0e0d/standard.iso#L157
> >
> > No, because Coccinelle does not know that it is a pointer.
> > How should it know?
>
> Can the software determine that the expression metavariable refers to
> a pointer variable (within the implementation of a function like “rtw_ap_set_key”)?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/rtl8723bs/core/rtw_ap.c?id=9e0babf2c06c73cda2c0cd37a1653d823adb40ec#n1479
> https://elixir.bootlin.com/linux/v5.2-rc5/source/drivers/staging/rtl8723bs/core/rtw_ap.c#L1479

Isomorphisms happen in advance, not during the matching process.

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] Checking for a null pointer with SmPL

[Markus Elfring]

> Isomorphisms happen in advance, not during the matching process.

Can the mentioned SmPL script variant work also without taking extra
software transformations by isomorphism specifications into account?

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

Re: [Cocci] Checking for a null pointer with SmPL

[Markus Elfring]

>> Can the mentioned SmPL script variant work also without taking extra
>> software transformations by isomorphism specifications into account?
>
> Well, you already know that it doesn't.  So oviously the answe is that it cannot.

At the moment.


> If you indicate that the expression ex is a pointer then maybe it will work.

Should the available data type information be sufficient for the handling
of pointers in the discussed use case?

Example:
	struct setkey_parm *psetkeyparm;


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

Re: [Cocci] Checking for a null pointer with SmPL

[Markus Elfring]

>> Example:
>> 	struct setkey_parm *psetkeyparm;
>
> As I already told you, the isomorphisms are applied before matching
> against the C code.  At that time, this information is not available.

Under which circumstances will the Coccinelle software become able
to take available data type information completely into account?

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

Re: [Cocci] Checking for a null pointer with SmPL

[Markus Elfring]

> As I already told you, the isomorphisms are applied before matching
> against the C code.  At that time, this information is not available.

Can the software situation be clarified also by omitting Coccinelle's isomorphisms?
(Can this functionality be temporarily switched off for a specific SmPL rule?)
https://github.com/coccinelle/coccinelle/blob/c6d7554edf7c4654aeae4d33c3f040e300682f23/docs/manual/cocci_syntax.tex#L125

How relevant is it for the handling of a source code search specification
like “if (ex) is else es” by the semantic patch language
(if you would like to support the omission of the code part “== NULL” at all)?

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

Re: [Cocci] Checking for a null pointer with SmPL

[Markus Elfring]

> If you turn off isomorphisms, it will only match exactly what you have written.

This setting can occasionally be appropriate.


> Isomorphisms have a cost, so the provided isomorphism only converts !X to
> X == NULL when X is a pointer.  If you want to make an isomorphism that
> does something else, please feel free.

If data type information can not be taken into account for the transformation
of a source code search specification at this point, it can become interesting
to reactivate selected “isomorphism” rules on demand only after type information
is available at a later data processing step.

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

Re: [Cocci] Checking for a null pointer with SmPL

[Markus Elfring]

> Isomorphisms have a cost, so the provided isomorphism only converts !X to
> X == NULL when X is a pointer.

It seems that the Coccinelle software has got restrictions on
the interpretation of pointers so far.
Is a pointer recognised for application of isomorphisms by
the semantic patch language only when the metavariable type contains
an asterisk (instead of expecting that SmPL expression metavariables
can benefit from this functionality already without an explicit “*”)?

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

Re: [Cocci] Checking for a null pointer with SmPL

[Markus Elfring]

> How would Coccinelle know that a pointer is expected if there is no asterisk?

SmPL expression metavariables can handle also pointer variables from C code,
can't they?

Type distinctions have got other consequences for isomorphism definitions.

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

Re: [Cocci] Handling of pointer expressions by SmPL

[Markus Elfring]

> How would Coccinelle know that a pointer is expected if there is no asterisk?

I am still trying to achieve a better understanding around the interpretation
of the specification “expression *X;”.
https://github.com/coccinelle/coccinelle/blob/19ee1697bf152d37a78a20cefe148775bf4b0e0d/standard.iso#L151

It looks like that it should refer to a pointer dereference
(according to a view for the semantic patch language).
But the really desired meaning might be different for the support
of pointer expressions by the isomorphism definition language.

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

Re: [Cocci] Handling of pointer expressions by SmPL

[Markus Elfring]

>> https://github.com/coccinelle/coccinelle/blob/19ee1697bf152d37a78a20cefe148775bf4b0e0d/standard.iso#L151
>>
>> It looks like that it should refer to a pointer dereference
>> (according to a view for the semantic patch language).
>> But the really desired meaning might be different for the support
>> of pointer expressions by the isomorphism definition language.
>
> Metavariables have types.  Here X has pointer type.

Our understanding of this software detail seems to match.

But I assume that the asterisk can be treated in a different way by
the isomorphism definition language in comparison to the semantic
patch language.
How much does context-dependent interpretation matter here?


> There is no need for X to match a pointer that is dereferenced.

I got further development imaginations around the places where
you would specify pointer dereferences by metavariables instead of
leaving asterisks in source code search fragments.

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

Re: [Cocci] Handling of pointer expressions by SmPL

[Julia Lawall]

On Wed, 19 Jun 2019, Markus Elfring wrote:

> >> https://github.com/coccinelle/coccinelle/blob/19ee1697bf152d37a78a20cefe148775bf4b0e0d/standard.iso#L151
> >>
> >> It looks like that it should refer to a pointer dereference
> >> (according to a view for the semantic patch language).
> >> But the really desired meaning might be different for the support
> >> of pointer expressions by the isomorphism definition language.
> >
> > Metavariables have types.  Here X has pointer type.
>
> Our understanding of this software detail seems to match.
>
> But I assume that the asterisk can be treated in a different way by
> the isomorphism definition language in comparison to the semantic
> patch language.
> How much does context-dependent interpretation matter here?
>
>
> > There is no need for X to match a pointer that is dereferenced.
>
> I got further development imaginations around the places where
> you would specify pointer dereferences by metavariables instead of
> leaving asterisks in source code search fragments.

This is incorrect.

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

Re: [Cocci] Handling of pointer expressions by SmPL

[Markus Elfring]

>>> Metavariables have types.  Here X has pointer type.
>>
>> Our understanding of this software detail seems to match.
>>
>> But I assume that the asterisk can be treated in a different way by
>> the isomorphism definition language in comparison to the semantic
>> patch language.
>> How much does context-dependent interpretation matter here?
>>
>>
>>> There is no need for X to match a pointer that is dereferenced.
>>
>> I got further development imaginations around the places where
>> you would specify pointer dereferences by metavariables instead of
>> leaving asterisks in source code search fragments.
>
> This is incorrect.

Which detail do you find inappropriate from my feedback?

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

Re: [Cocci] Handling of pointer expressions by SmPL

[Markus Elfring]

> You can't specify pointer dereferences by metavariables.

Thanks for another clarification for a detail by the Coccinelle software.

I assumed that this would be useful functionality for the specification
“expression *X;”.


> The pattern is only in the pattern.  It is not partly in the metavariables.

Would you like to add this information to the software documentation?

* Does the asterisk express only a specific restriction on the
  metavariable type then?

* Which source code would you match (or exclude) for pointer expressions?

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

Re: [Cocci] Handling of pointer expressions by SmPL

[Markus Elfring]

>> Would you like to add this information to the software documentation?
>
> Not really.  The documentation can't defend against everyone's imagination.

Will it become helpful to mention the metavariable type “expression *”
also in the SmPL manual?
https://github.com/coccinelle/coccinelle/blob/cad4c0705f9e37f501531e133d3a47bc56ed0ce2/docs/manual/cocci_syntax.tex#L67


The following SmPL script variant can work as expected then.

@display@
expression* ex;
identifier zm =~ "_zmalloc";
statement is, es;
@@
 ex = zm(...);
 if (ex)
    is
 else
    es

*memset(ex, 0, ...);


elfring@Sonne:~/Projekte/Linux/next-patched> spatch ~/Projekte/Coccinelle/janitor/show_questionable_memset3.cocci drivers/staging/rtl8723bs/core/rtw_ap.c
…


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

Re: [Cocci] Handling of pointer expressions by SmPL

[Markus Elfring]

>> Would you like to add this information to the software documentation?
>
> Not really.  The documentation can't defend against everyone's imagination.

Will it become helpful to mention the metavariable type “expression *”
also in the SmPL manual?
https://github.com/coccinelle/coccinelle/blob/cad4c0705f9e37f501531e133d3a47bc56ed0ce2/docs/manual/cocci_syntax.tex#L67


The following SmPL script variant can work as expected then.

@display@
expression* ex;
identifier zm =~ "_zmalloc";
statement is, es;
@@
 ex = zm(...);
 if (ex)
    is
 else
    es

*memset(ex, 0, ...);


elfring@Sonne:~/Projekte/Linux/next-patched> spatch ~/Projekte/Coccinelle/janitor/show_questionable_memset3.cocci drivers/staging/rtl8723bs/core/rtw_ap.c
…


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