[Cocci] Searching for functions with negative return values as error indication

[Cocci] Searching for functions with negative return values as error indication

[Markus Elfring]

Hello,

Several functions for a programming language like C are designed in the way
that values are returned by an integral data type.
Specific value ranges can indicate then a failed function call.
A well-known variant of this design pattern is that negative return values
represent failures (while the other values can be used for succesful data processing.
Can the semantic patch language help any more to determine the list of
functions which use this style of error reporting in a selected code base?

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

Re: [Cocci] Searching for functions with negative return values as error indication

[Julia Lawall]

On Fri, 10 Apr 2020, Markus Elfring wrote:

> Hello,
>
> Several functions for a programming language like C are designed in the way
> that values are returned by an integral data type.
> Specific value ranges can indicate then a failed function call.
> A well-known variant of this design pattern is that negative return values
> represent failures (while the other values can be used for succesful data processing.
> Can the semantic patch language help any more to determine the list of
> functions which use this style of error reporting in a selected code base?

Maybe you can just take inspiration from this paper:

https://pages.lip6.fr/Julia.Lawall/dsn09.pdf

julia

>
> Regards,
> Markus
> _______________________________________________
> 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

Re: [Cocci] Searching for functions with negative return values as error indication

[Markus Elfring]

> Maybe you can just take inspiration from this paper:
>
> https://pages.lip6.fr/Julia.Lawall/dsn09.pdf

I find the information from this document “WYSIWIB: A Declarative Approach
to Finding API Protocols and Bugs in Linux Code” also interesting.
Did the mentioned tools “Search”, “Instantiate”, “MakeBugReport” and “MultiSearch”
evolve any further (besides the Coccinelle software in the meantime)?

It seems to be more popular to look for inconsistent error checks together with
pointer data types.
Can the software situation be improved also around integral data types currently?

Will the known limitations of Coccinelle for protocol and bug finding
get any more software development attention?

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

Re: [Cocci] Reporting variations of condition checks according to function calls

[Markus Elfring]

> Maybe you can just take inspiration from this paper:
>
> https://pages.lip6.fr/Julia.Lawall/dsn09.pdf

I got another idea which can be related to information from the section
“Case Study: Inconsistent Error Checks” in your document “WYSIWIB:
A Declarative Approach to Finding API Protocols and Bugs in Linux Code”.
A key aspect is the knowledge about failure predicates for functions
from programming interfaces.
How do you think about to improve the knowledge by the means of advanced
source code analysis?

I got the impression that such analysis attempts can become especially
challenging if data processing resources might occasionally be too limited
for a growing code base like Linux software.
Would you like to display statistics around the distribution of condition
checks after function calls (with help of the semantic patch language)?

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

Re: [Cocci] Reporting variations of condition checks according to function calls

[Julia Lawall]

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



On Sat, 11 Apr 2020, Markus Elfring wrote:

> > Maybe you can just take inspiration from this paper:
> >
> > https://pages.lip6.fr/Julia.Lawall/dsn09.pdf
>
> I got another idea which can be related to information from the section
> “Case Study: Inconsistent Error Checks” in your document “WYSIWIB:
> A Declarative Approach to Finding API Protocols and Bugs in Linux Code”.
> A key aspect is the knowledge about failure predicates for functions
> from programming interfaces.
> How do you think about to improve the knowledge by the means of advanced
> source code analysis?
>
> I got the impression that such analysis attempts can become especially
> challenging if data processing resources might occasionally be too limited
> for a growing code base like Linux software.
> Would you like to display statistics around the distribution of condition
> checks after function calls (with help of the semantic patch language)?

I'm working on other things.  If you want something done, please do it
yourself.

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] Reporting variations of condition checks according to function calls

[Markus Elfring]

>> Would you like to display statistics around the distribution of condition
>> checks after function calls (with help of the semantic patch language)?
>
> I'm working on other things.

Thanks for your feedback.


> If you want something done, please do it yourself.

I assumed that you (or other contributors) could help to reduce my concerns
also around limited data processing resources for challenging source code
analysis approaches.

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

Re: [Cocci] Reporting variations of condition checks according to function calls

[Markus Elfring]

> If you want something done, please do it yourself.

I hope that another clarification can be achieved also for the software
behaviour of the following source code analysis approach.


@initialize:python@
@@
import sqlalchemy, sys
sys.stderr.write("\n".join(["Using SQLAlchemy version:",
                            sqlalchemy.__version__]))
sys.stderr.write("\n")
from sqlalchemy import create_engine, Column, Integer, String
engine = create_engine("sqlite:///:memory:", echo = False)

from sqlalchemy.ext.declarative import declarative_base
base = declarative_base()

class pair(base):
   __tablename__ = "pairs"
   function = Column(String, primary_key = True)
   source_file = Column(String, primary_key = True)
   line = Column(Integer, primary_key = True)
   column = Column(Integer, primary_key = True)
   action = Column(String)
   check = Column(String)

from sqlalchemy.orm import sessionmaker
configured_session = sessionmaker(bind = engine)
session = configured_session()
base.metadata.create_all(engine)

def store_positions(places, check_in, action_in):
    """Add source code positions to an internal table."""
    for place in places:
       entry = pair(function = place.current_element,
                    source_file = place.file,
                    line = place.line,
                    column = int(place.column) + 1,
                    action = action_in,
                    check = check_in
                   )
       session.add(entry)

@find1@
expression action, e;
position p;
statement is, es;
@@
 e = action(...);
 if@p (<+... e ...+>)
    is
 else
    es

@check1@
expression check;
position find1.p;
statement find1.is, find1.es;
@@
 if@p (check)
    is
 else
    es

@script:python collection1@
c << check1.check;
action << find1.action;
places << find1.p;
@@
store_positions(places, c, action)

@finalize:python@
@@
session.commit()
from sqlalchemy import func
entries = session.query(func.count("*")).select_from(pair).scalar()

if entries > 0:
   delimiter = "|"
   sys.stdout.write(delimiter.join(['action',
                                    'check',
                                    '"source file"',
                                    "line",
                                    "column"
                                    ]))
   sys.stdout.write("\r\n")

   for action, \
       check, \
       file, \
       line, \
       column in session.query(pair.action,
                               pair.check,
                               pair.source_file,
                               pair.line,
                               pair.column
                              ).order_by(pair.action,
                                         pair.check,
                                         pair.source_file,
                                         pair.line,
                                         pair.column
                                        ):
      sys.stdout.write(delimiter.join([action,
                                       check,
                                       file,
                                       str(line),
                                       str(column)
                                      ]))
      sys.stdout.write("\r\n")
else:
   sys.stderr.write("No result for this analysis!\n")


elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch --parse-cocci list_condition_checks_after_function_calls2.cocci
…
warning: iso drop_else does not match the code below on line 55
if@p (check) iselse es
…
elfring@Sonne:~/Projekte/Linux/next-patched> spatch ~/Projekte/Coccinelle/janitor/list_condition_checks_after_function_calls2.cocci drivers/gpu/drm/mcde/mcde_drv.c
…
Using SQLAlchemy version:
1.3.10
HANDLING: drivers/gpu/drm/mcde/mcde_drv.c
No result for this analysis!


Will any software development possibilities become interesting here?

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

Re: [Cocci] Reporting variations of condition checks according to function calls

[Markus Elfring]

>> I hope that another clarification can be achieved also for the software
>> behaviour of the following source code analysis approach.
>
> I don't run code that involves databases.

I find the handling of higher level data storage relevant and useful
also for such a software test.


> If you believe that there is a problem with the semantic patch,
> you have to make a small version that illustrates the problem.

My SmPL script example seems to work as expected (in principle)
if I omit the else branch from the SmPL rules “find1” and “check1”.

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

Re: [Cocci] More support for SmPL data processing with databases?

[Markus Elfring]

>> I hope that another clarification can be achieved also for the software
>> behaviour of the following source code analysis approach.
>
> I don't run code that involves databases.

Can the situation evolve in a way so that this programming interface
will become better supported together with programming languages
and development tools you are more familiar with (so far)?

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

Re: [Cocci] More support for SmPL data processing with databases?

[Julia Lawall]

On Sun, 12 Apr 2020, Markus Elfring wrote:

> >> I hope that another clarification can be achieved also for the software
> >> behaviour of the following source code analysis approach.
> >
> > I don't run code that involves databases.
>
> Can the situation evolve in a way so that this programming interface
> will become better supported together with programming languages
> and development tools you are more familiar with (so far)?

I think that your issue about something matching or not has nothing to do
with the database code, and you could easily remove it for the purposes of
reporting a concern with Coccinelle.

julia

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

Re: [Cocci] More support for SmPL data processing with databases?

[Markus Elfring]

> I think that your issue about something matching or not has nothing to do
> with the database code,

Such a view can be partly appropriate.


> and you could easily remove it

I hope that the understanding of the presented SmPL code example
could also be sufficient in the way that the data processing
for the SmPL rules “find1” and “check1” will need another bit
of clarification.


> for the purposes of reporting a concern with Coccinelle.

Another code variant can eventually be clarified a bit later.

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

Re: [Cocci] Checking the application of the SmPL isomorphism “drop_else”

[Markus Elfring]

> I noticed that there was a report about an isomorphism not applying.

I hoped for another clarification also for the message “warning: iso drop_else
does not match the code below on line 55” (and the corresponding debug display).


> That issurely the problem.  So you have to figure out why it does not apply.
> There is an isomorphism that drops else's in ifs, but that only applies if
> the else is a metavariable that is not used elsewhere.

I am looking for further improvements in this software area.

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

Re: [Cocci] Reporting variations of condition checks according to function calls

[Markus Elfring]

>> I hope that another clarification can be achieved also for the software
>> behaviour of the following source code analysis approach.
>
> I don't run code that involves databases.  If you believe that there is a
> problem with the semantic patch, you have to make a small version that
> illustrates the problem.

Would you find the test result easier to clarify for the following
SmPL script example?


@display1@
expression action, e;
position p;
statement is, es;
@@
*e = action(...);
 if@p (<+... e ...+>)
    is
 else
    es

@display2@
expression check;
position display1.p;
statement display1.is, display1.es;
@@
*if@p (check)
    is
 else
    es


elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch --parse-cocci show_condition_checks_after_function_calls2.cocci
…
warning: iso drop_else does not match the code below on line 17
if@p (check) iselse es
…


Will any software improvements become interesting for such an use case?

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

Re: [Cocci] Reporting variations of condition checks according to function calls

[Julia Lawall]

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



On Sun, 12 Apr 2020, Markus Elfring wrote:

> >> I hope that another clarification can be achieved also for the software
> >> behaviour of the following source code analysis approach.
> >
> > I don't run code that involves databases.  If you believe that there is a
> > problem with the semantic patch, you have to make a small version that
> > illustrates the problem.
>
> Would you find the test result easier to clarify for the following
> SmPL script example?
>
>
> @display1@
> expression action, e;
> position p;
> statement is, es;
> @@
> *e = action(...);
>  if@p (<+... e ...+>)
>     is
>  else
>     es
>
> @display2@
> expression check;
> position display1.p;
> statement display1.is, display1.es;

The problem is that you inherit es.  Since you inherit it, Coccinelle
considers that its presence is important, and so the isomorphism will not
eliminate it.  It is sufficient to make another pattern for the case with
no else.

julia

> @@
> *if@p (check)
>     is
>  else
>     es
>
>
> elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch --parse-cocci show_condition_checks_after_function_calls2.cocci
> …
> warning: iso drop_else does not match the code below on line 17
> if@p (check) iselse es
> …
>
>
> Will any software improvements become interesting for such an use case?
>
> 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] Reporting variations of condition checks according to function calls

[Markus Elfring]

>> @display1@
>> expression action, e;
>> position p;
>> statement is, es;
>> @@
>> *e = action(...);
>>  if@p (<+... e ...+>)
>>     is
>>  else
>>     es
>>
>> @display2@
>> expression check;
>> position display1.p;
>> statement display1.is, display1.es;
>
> The problem is that you inherit es.  Since you inherit it, Coccinelle
> considers that its presence is important, and so the isomorphism will not
> eliminate it.

Thanks for your explanation of such consequences by the metavariable inheritance.


> It is sufficient to make another pattern for the case with no else.

I might stumble on another software design conflict (or the need
for corresponding software extensions).

I tried the specification of the SmPL rule “display2” out because I wanted
to determine the source code which was matched by the SmPL nest construct
in the SmPL rule “display1”.
The specification of a SmPL disjunction in the first rule would trigger
other known development difficulties (or open issues), wouldn't it?

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

Re: [Cocci] Reporting variations of condition checks according to function calls

[Markus Elfring]

>>>> @display2@
>>>> expression check;
>>>> position display1.p;
>>>> statement display1.is, display1.es;
>>>
>>> The problem is that you inherit es.  Since you inherit it, Coccinelle
>>> considers that its presence is important, and so the isomorphism will not
>>> eliminate it.
>>
>> Thanks for your explanation of such consequences by the metavariable inheritance.

It seems that the specification of the inheritance for the metavariable “es”
can be (temporarily) omitted as a workaround.


>>> It is sufficient to make another pattern for the case with no else.
>>
>> I might stumble on another software design conflict (or the need
>> for corresponding software extensions).
>>
>> I tried the specification of the SmPL rule “display2” out because I wanted
>> to determine the source code which was matched by the SmPL nest construct
>> in the SmPL rule “display1”.
>> The specification of a SmPL disjunction in the first rule would trigger
>> other known development difficulties (or open issues), wouldn't it?
>
> No idea.

I would prefer to find the shown statement combination by a single SmPL rule.
Can the matched source code from the construct “<+... e ...+>” be assigned
to a metavariable like “check”?

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

Re: [Cocci] Reporting variations of condition checks according to function calls

[Julia Lawall]

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



On Sun, 12 Apr 2020, Markus Elfring wrote:

> >>>> @display2@
> >>>> expression check;
> >>>> position display1.p;
> >>>> statement display1.is, display1.es;
> >>>
> >>> The problem is that you inherit es.  Since you inherit it, Coccinelle
> >>> considers that its presence is important, and so the isomorphism will not
> >>> eliminate it.
> >>
> >> Thanks for your explanation of such consequences by the metavariable inheritance.
>
> It seems that the specification of the inheritance for the metavariable “es”
> can be (temporarily) omitted as a workaround.
>
>
> >>> It is sufficient to make another pattern for the case with no else.
> >>
> >> I might stumble on another software design conflict (or the need
> >> for corresponding software extensions).
> >>
> >> I tried the specification of the SmPL rule “display2” out because I wanted
> >> to determine the source code which was matched by the SmPL nest construct
> >> in the SmPL rule “display1”.
> >> The specification of a SmPL disjunction in the first rule would trigger
> >> other known development difficulties (or open issues), wouldn't it?
> >
> > No idea.
>
> I would prefer to find the shown statement combination by a single SmPL rule.
> Can the matched source code from the construct “<+... e ...+>” be assigned
> to a metavariable like “check”?

(
  <+... e ...+>
&
  check
)

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] Reporting variations of condition checks according to function calls

[Markus Elfring]

>> Can the matched source code from the construct “<+... e ...+>” be assigned
>> to a metavariable like “check”?
>
> (
>   <+... e ...+>
> &
>   check
> )

Did I just stumble on incomplete knowledge for the safe application
of the conjunction functionality with the semantic patch language?

It seems that some source code search approaches can be improved
in such a way.

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

Re: [Cocci] More support for SmPL data processing with databases?

[Markus Elfring]

> I think that your issue about something matching or not has nothing to do
> with the database code, and you could easily remove it for the purposes of
> reporting a concern with Coccinelle.

Software evolution can be continued also together with your constructive feedback.

I adapted another SmPL script according to a recent information.
https://lore.kernel.org/cocci/17a4592b-92bd-e4c9-8481-7d46616cb1d0@web.de/T/#m4f59721d2011ac4ded602f2e63e0c63f22a15fa9
https://systeme.lip6.fr/pipermail/cocci/2020-April/007133.html

It can become more interesting to increase also the application of SmPL conjunctions
like the following.

@find@
expression action, check, result;
position p;
statement is, es;
@@
 result = action(...);
 if@p (
(
      <+... result ...+>
&     check
)     )
     is
 else
     es


Now I stumble on corresponding software development challenges.

elfring@Sonne:~/Projekte/Linux/next-patched> spatch ~/Projekte/Coccinelle/janitor/list_condition_checks_after_function_calls2.cocci drivers/gpu/drm/mcde/mcde_drv.c
…
Using SQLAlchemy version:
1.3.15
…
sqlalchemy.exc.IntegrityError: (sqlite3.IntegrityError) UNIQUE constraint failed: pairs.function, pairs.source_file, pairs.line, pairs.column
[SQL: INSERT INTO pairs (function, source_file, line, "column", action, "check") VALUES (?, ?, ?, ?, ?, ?)]
…


I hope that it can become easier to clarify where unexpected duplicate keys
would occur as in my test approach.

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

Re: [Cocci] More support for SmPL data processing with databases?

[Markus Elfring]

>> I hope that it can become easier to clarify where unexpected duplicate keys
>> would occur as in my test approach.
>
> Use --debug and just print out the infomation rather than putting is in
> your database.

We have got different views around this logging approach.


> With the database you add complexity for nothing.

I disagree to this view.

Database software usually provides some helpful functionality, doesn't it?

The mentioned programming interface quickly pointed an unique constraint violation out.
Now I am looking again for possible solutions.

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

Re: [Cocci] More support for SmPL data processing with databases?

[Markus Elfring]

> I hope that it can become easier to clarify where unexpected duplicate keys
> would occur as in my test approach.

There is the possibility to apply information according to a clarification
request on a topic like “Checking the handling of unique keys/indexes”
from 2014-12-20.
https://groups.google.com/d/msg/sqlalchemy/klmUwiirIQw/g6NLiVAGGAIJ


elfring@Sonne:~/Projekte/Linux/next-patched> spatch ~/Projekte/Coccinelle/janitor/list_condition_checks_after_function_calls2c.cocci drivers/gpu/drm/mcde/mcde_drv.c
…
Using SQLAlchemy version:
1.3.15
…
%% function:
mcde_probe
line:
322
%% function:
mcde_probe
line:
403
action|check|"source file"|line|column
…


How should the error reporting be clarified around such source code places?
https://elixir.bootlin.com/linux/v5.6.3/source/drivers/gpu/drm/mcde/mcde_drv.c#L307
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/mcde/mcde_drv.c?id=8f3d9f354286745c751374f5f1fcafee6b3f3136#n308


I observe that the following SmPL code variant can result also in
a significant difference.

@find@
expression action, check, result;
position p;
statement is, es;
@@
 result = action(...);
 if (
(   <+... result ...+>
&   check@p
)   )
    is
 else
    es


How will the application of SmPL conjunctions evolve further?

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

Re: [Cocci] More support for SmPL data processing with databases?

[Markus Elfring]

>> I observe that the following SmPL code variant can result also in
>> a significant difference.
>>
>> @find@
>> expression action, check, result;
>> position p;
>> statement is, es;
>> @@
>>  result = action(...);
>>  if (
>> (   <+... result ...+>
>> &   check@p
>> )   )
>>     is
>>  else
>>     es
>>
>>
>> How will the application of SmPL conjunctions evolve further?
>
> I really have no idea what you are talking about.

I suggest to take another look for the software behaviour.


> that contains all of the semantic patch variants that you want to have
> These semantic patches should involve no use of databases.

Please compare details with the following SmPL code example.

@find@
expression action, check, result;
position p;
statement is, es;
@@
 result = action(...);
 if@p (
(     <+... result ...+>
&     check
)     )
     is
 else
     es


Under which circumstances will the Coccinelle software provide duplicate data
for their processing by (Python) script rules?


> The database itself doesn't bring any value.

Another unique constraint violation was noticed by using such technology.
I am curious when you are going to take related functionality better into account.

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

Re: [Cocci] More support for SmPL data processing with databases?

[Markus Elfring]

>>> The database itself doesn't bring any value.
>>
>> Another unique constraint violation was noticed by using such technology.
>
> It's an accident.

This might be. - I got into the development mood to publish patches
for a few source files of Linux modules according to another search approach.
I picked one of these files up for the discussed test.


> You could have noticed the same thing by just printing the results
> in the python script.

This can be also possible in principle.
I find the programming convenience different.


>> I am curious when you are going to take related functionality better into account.
>
> Never.

Interesting …


> It has nothing to do with Coccinelle.

Do I present another advanced use case for the application of known software tools?


> When you make a bug report, you should show the minimal code that shows the problem.

I hope that variations can occasionally help also to achieve a better understanding.

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

Re: [Cocci] More support for SmPL data processing with databases?

[Markus Elfring]

> Now you have the two scripts, which is good, but you haven't included the
> test data that causes the problem.  Please make one mail that includes everything.

I find that I provided relevant data already.
https://systeme.lip6.fr/pipermail/cocci/2020-April/007135.html
https://lore.kernel.org/cocci/ab6d402e-3c12-25ab-162a-f200c382dd0a@web.de/

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

Re: [Cocci] More support for SmPL data processing with databases?

[Markus Elfring]

>> Do I present another advanced use case for the application of known software tools?
>
> No.

Such a view can be also interesting.


> The faults in linux paper (work done 10 years ago) uses a database as well.

Thanks for this background information.

How will the software evolve further in affected areas?

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

Re: [Cocci] More support for SmPL data processing with databases?

[Markus Elfring]

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

>> How will the application of SmPL conjunctions evolve further?
>
> I really have no idea what you are talking about.

I suggest to take another look at affected implementation details.


> Make one single mail that contains all of the semantic patch variants
> that you want to have considered and explain what the problem is.

Would you find the following data processing approach easier to clarify?

I extracted a bit of source code from a known file for your convenience
as an attachment.
https://elixir.bootlin.com/linux/v5.6.3/source/drivers/gpu/drm/mcde/mcde_drv.c#L307
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/mcde/mcde_drv.c?id=8f3d9f354286745c751374f5f1fcafee6b3f3136#n308


> These semantic patches should involve no use of databases.

A Python dictionary can also be used as a data storage technique
for the discussed system test.


@initialize:python@
@@
import sys

records = {}

class integrity_error:
   pass

def store_positions(places, check_in, action_in):
    """Add source code positions to an internal table."""
    for place in places:
       key = (place.file, place.line, int(place.column) + 1)

       if key in records:
          sys.stderr.write("\n".join(["-> duplicate data",
                                      "file:", key[0],
                                      "function:", place.current_element,
                                      "line:", str(place.line)]))
          sys.stderr.write("\n")
          raise integrity_error
       else:
          records[key] = (action_in, check_in, place.current_element)

@find@
expression action, check, result;
position p;
statement is, es;
@@
 result = action(...);
 if@p (
(     <+... result ...+>
&     check
)     )
     is
 else
     es

@script:python collection@
c << find.check;
action << find.action;
places << find.p;
@@
store_positions(places, c, action)

@finalize:python@
@@
if len(records) > 0:
   delimiter = "|"
   sys.stdout.write(delimiter.join(['action',
                                    'check',
                                    '"source file"',
                                    'line',
                                    'column'
                                    ]))
   sys.stdout.write("\r\n")

   for key, value in records.items():
      sys.stdout.write(delimiter.join([value[0],
                                       value[1],
                                       key[0],
                                       key[1],
                                       str(key[2])
                                      ]))
      sys.stdout.write("\r\n")
else:
   sys.stderr.write("No result for this analysis!\n")


elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch list_condition_checks_after_function_calls7.cocci ../Probe/mcde_drv-excerpt1.c
…
-> duplicate data
file:
../Probe/mcde_drv-excerpt1.c
function:
mcde_probe
line:
16
Traceback …:
  …, line 30, in store_positions
__main__.integrity_error: <__main__.integrity_error instance at 0x7fd3bff4d730>
…


Would you like compare the software behaviour any further with
the following rule variant for the semantic patch language?

@find@
expression action, check, result;
position p;
statement is, es;
@@
 result = action(...);
 if (
(   <+... result ...+>
&   check@p
)   )
    is
 else
    es


Regards,
Markus

[-- Attachment #2: mcde_drv-excerpt1.c --]
[-- Type: text/x-csrc, Size: 5686 bytes --]

// SPDX-License-Identifier: GPL-2.0
static int mcde_probe(struct platform_device *pdev)
{
	struct device *dev = &pdev->dev;
	struct drm_device *drm;
	struct mcde *mcde;
	struct component_match *match = NULL;
	struct resource *res;
	u32 pid;
	u32 val;
	int irq;
	int ret;
	int i;

	mcde = kzalloc(sizeof(*mcde), GFP_KERNEL);
	if (!mcde)
		return -ENOMEM;
	mcde->dev = dev;

	ret = drm_dev_init(&mcde->drm, &mcde_drm_driver, dev);
	if (ret) {
		kfree(mcde);
		return ret;
	}
	drm = &mcde->drm;
	drm->dev_private = mcde;
	platform_set_drvdata(pdev, drm);

	/* Enable continuous updates: this is what Linux' framebuffer expects */
	mcde->oneshot_mode = false;
	drm->dev_private = mcde;

	/* First obtain and turn on the main power */
	mcde->epod = devm_regulator_get(dev, "epod");
	if (IS_ERR(mcde->epod)) {
		ret = PTR_ERR(mcde->epod);
		dev_err(dev, "can't get EPOD regulator\n");
		goto dev_unref;
	}
	ret = regulator_enable(mcde->epod);
	if (ret) {
		dev_err(dev, "can't enable EPOD regulator\n");
		goto dev_unref;
	}
	mcde->vana = devm_regulator_get(dev, "vana");
	if (IS_ERR(mcde->vana)) {
		ret = PTR_ERR(mcde->vana);
		dev_err(dev, "can't get VANA regulator\n");
		goto regulator_epod_off;
	}
	ret = regulator_enable(mcde->vana);
	if (ret) {
		dev_err(dev, "can't enable VANA regulator\n");
		goto regulator_epod_off;
	}
	/*
	 * The vendor code uses ESRAM (onchip RAM) and need to activate
	 * the v-esram34 regulator, but we don't use that yet
	 */

	/* Clock the silicon so we can access the registers */
	mcde->mcde_clk = devm_clk_get(dev, "mcde");
	if (IS_ERR(mcde->mcde_clk)) {
		dev_err(dev, "unable to get MCDE main clock\n");
		ret = PTR_ERR(mcde->mcde_clk);
		goto regulator_off;
	}
	ret = clk_prepare_enable(mcde->mcde_clk);
	if (ret) {
		dev_err(dev, "failed to enable MCDE main clock\n");
		goto regulator_off;
	}
	dev_info(dev, "MCDE clk rate %lu Hz\n", clk_get_rate(mcde->mcde_clk));

	mcde->lcd_clk = devm_clk_get(dev, "lcd");
	if (IS_ERR(mcde->lcd_clk)) {
		dev_err(dev, "unable to get LCD clock\n");
		ret = PTR_ERR(mcde->lcd_clk);
		goto clk_disable;
	}
	mcde->hdmi_clk = devm_clk_get(dev, "hdmi");
	if (IS_ERR(mcde->hdmi_clk)) {
		dev_err(dev, "unable to get HDMI clock\n");
		ret = PTR_ERR(mcde->hdmi_clk);
		goto clk_disable;
	}

	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
	mcde->regs = devm_ioremap_resource(dev, res);
	if (IS_ERR(mcde->regs)) {
		dev_err(dev, "no MCDE regs\n");
		ret = -EINVAL;
		goto clk_disable;
	}

	irq = platform_get_irq(pdev, 0);
	if (!irq) {
		ret = -EINVAL;
		goto clk_disable;
	}

	ret = devm_request_irq(dev, irq, mcde_irq, 0, "mcde", mcde);
	if (ret) {
		dev_err(dev, "failed to request irq %d\n", ret);
		goto clk_disable;
	}

	/*
	 * Check hardware revision, we only support U8500v2 version
	 * as this was the only version used for mass market deployment,
	 * but surely you can add more versions if you have them and
	 * need them.
	 */
	pid = readl(mcde->regs + MCDE_PID);
	dev_info(dev, "found MCDE HW revision %d.%d (dev %d, metal fix %d)\n",
		 (pid & MCDE_PID_MAJOR_VERSION_MASK)
		 >> MCDE_PID_MAJOR_VERSION_SHIFT,
		 (pid & MCDE_PID_MINOR_VERSION_MASK)
		 >> MCDE_PID_MINOR_VERSION_SHIFT,
		 (pid & MCDE_PID_DEVELOPMENT_VERSION_MASK)
		 >> MCDE_PID_DEVELOPMENT_VERSION_SHIFT,
		 (pid & MCDE_PID_METALFIX_VERSION_MASK)
		 >> MCDE_PID_METALFIX_VERSION_SHIFT);
	if (pid != 0x03000800) {
		dev_err(dev, "unsupported hardware revision\n");
		ret = -ENODEV;
		goto clk_disable;
	}

	/* Set up the main control, watermark level at 7 */
	val = 7 << MCDE_CONF0_IFIFOCTRLWTRMRKLVL_SHIFT;
	/* 24 bits DPI: connect LSB Ch B to D[0:7] */
	val |= 3 << MCDE_CONF0_OUTMUX0_SHIFT;
	/* TV out: connect LSB Ch B to D[8:15] */
	val |= 3 << MCDE_CONF0_OUTMUX1_SHIFT;
	/* Don't care about this muxing */
	val |= 0 << MCDE_CONF0_OUTMUX2_SHIFT;
	/* 24 bits DPI: connect MID Ch B to D[24:31] */
	val |= 4 << MCDE_CONF0_OUTMUX3_SHIFT;
	/* 5: 24 bits DPI: connect MSB Ch B to D[32:39] */
	val |= 5 << MCDE_CONF0_OUTMUX4_SHIFT;
	/* Syncmux bits zero: DPI channel A and B on output pins A and B resp */
	writel(val, mcde->regs + MCDE_CONF0);

	/* Enable automatic clock gating */
	val = readl(mcde->regs + MCDE_CR);
	val |= MCDE_CR_MCDEEN | MCDE_CR_AUTOCLKG_EN;
	writel(val, mcde->regs + MCDE_CR);

	/* Clear any pending interrupts */
	mcde_display_disable_irqs(mcde);
	writel(0, mcde->regs + MCDE_IMSCERR);
	writel(0xFFFFFFFF, mcde->regs + MCDE_RISERR);

	/* Spawn child devices for the DSI ports */
	devm_of_platform_populate(dev);

	/* Create something that will match the subdrivers when we bind */
	for (i = 0; i < ARRAY_SIZE(mcde_component_drivers); i++) {
		struct device_driver *drv = &mcde_component_drivers[i]->driver;
		struct device *p = NULL, *d;

		while ((d = platform_find_device_by_driver(p, drv))) {
			put_device(p);
			component_match_add(dev, &match, mcde_compare_dev, d);
			p = d;
		}
		put_device(p);
	}
	if (!match) {
		dev_err(dev, "no matching components\n");
		ret = -ENODEV;
		goto clk_disable;
	}
	if (IS_ERR(match)) {
		dev_err(dev, "could not create component match\n");
		ret = PTR_ERR(match);
		goto clk_disable;
	}
	ret = component_master_add_with_match(&pdev->dev, &mcde_drm_comp_ops,
					      match);
	if (ret) {
		dev_err(dev, "failed to add component master\n");
		goto clk_disable;
	}
	return 0;

clk_disable:
	clk_disable_unprepare(mcde->mcde_clk);
regulator_off:
	regulator_disable(mcde->vana);
regulator_epod_off:
	regulator_disable(mcde->epod);
dev_unref:
	drm_dev_put(drm);
	return ret;

}

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

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

Re: [Cocci] More support for SmPL data processing with databases?

[Julia Lawall]

I tried the following semantic patches:

@find@
expression action, check, result;
position p;
statement is, es;
@@
 result = action(...);
 if (
(   <+... result ...+>
&   check@p
)   )
    is
 else
    es

@script:ocaml@
action << find.action;
check << find.check;
p << find.p;
@@
Printf.printf "%s %s %s\n" action check (Dumper.dump p)

----------------------------

@find@
expression action, check, result;
position p;
statement is, es;
@@
 result = action(...);
 if@p (
(     <+... result ...+>
&     check
)     )
     is
 else
     es

@script:ocaml@
action << find.action;
check << find.check;
p << find.p;
@@
Printf.printf "%s %s %s\n" action check (Dumper.dump p)

On the code you provided whichis unfortunately much too long to include in
a message and that you could have surely shortened to illustrate whatever
is the problem.

I get the same output from both semantic patches, except that the line
numbers are different for the position variable p, because you have put it
in a different place.

I don't see any reason why making the script code call a database would
have any impact on this issue.

If you do get different results for the above semantic patches, please
make a small .c file that illustrates the problem.  The .c file should be
such that it illustrates the problem and no smaller file also illustrates
the problem.

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

Re: [Cocci] More support for SmPL data processing with databases?

[Markus Elfring]

> @script:ocaml@
> action << find.action;
> check << find.check;
> p << find.p;
> @@
> Printf.printf "%s %s %s\n" action check (Dumper.dump p)
>
> On the code you provided whichis unfortunately much too long to include in
> a message

I found my code examples short enough.


> and that you could have surely shortened to illustrate whatever
> is the problem.

Does such a feedback indicate that you are still struggling with the understanding
for the presented software situation?


> I get the same output from both semantic patches, except that the line
> numbers are different for the position variable p,

Does this information contain a contradiction?

Do you observe any repeated data sets?


> because you have put it in a different place.

Yes. - Thus I would expect that different column numbers will be displayed
while the line numbers should be (mostly?) identical.


> I don't see any reason why making the script code call a database would
> have any impact on this issue.

The database can provide ACID properties for parallel data processing,
can't it?

I would like to perform aggregate computations finally.
Would you become interested in counting record sets anyhow?


> If you do get different results for the above semantic patches,
> please make a small .c file that illustrates the problem.

I presented questionable differences once more a moment ago.
https://lore.kernel.org/cocci/7228efff-76bb-44c9-e2cd-ea19bbd49abf@web.de/
https://systeme.lip6.fr/pipermail/cocci/2020-April/007142.html

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

Re: [Cocci] More support for SmPL data processing with databases?

[Markus Elfring]

> Maybe you consider the following results to be a problem:
>
> kzalloc mcde [("mcde_probe", 2, 0, 198, 1, "m.c", 16, 1, 16, 3)]
> kzalloc ! mcde [("mcde_probe", 2, 0, 198, 1, "m.c", 16, 1, 16, 3)]
> platform_get_irq irq [("mcde_probe", 2, 0, 198, 1, "m.c", 97, 1, 97, 3)]
> platform_get_irq ! irq [("mcde_probe", 2, 0, 198, 1, "m.c", 97, 1, 97, 3)]
>
> The positions are the same.  The result is because of an isomorphism that
> exchanges the branches of a conditional.

Can the data which are provided because of isomorphism transformations
be distinguished by an additional attribute?


> I think that you could have figured this out by yourself.  If you just
> printed out the data instead of wrapping it in lots of database noise,
> then you could have seen what was going on.

I got the impression that there are further software development
challenges to consider (also for safer counting of record sets).

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

Re: [Cocci] More support for SmPL data processing with databases?

[Julia Lawall]

On Mon, 13 Apr 2020, Markus Elfring wrote:

> > Maybe you consider the following results to be a problem:
> >
> > kzalloc mcde [("mcde_probe", 2, 0, 198, 1, "m.c", 16, 1, 16, 3)]
> > kzalloc ! mcde [("mcde_probe", 2, 0, 198, 1, "m.c", 16, 1, 16, 3)]
> > platform_get_irq irq [("mcde_probe", 2, 0, 198, 1, "m.c", 97, 1, 97, 3)]
> > platform_get_irq ! irq [("mcde_probe", 2, 0, 198, 1, "m.c", 97, 1, 97, 3)]
> >
> > The positions are the same.  The result is because of an isomorphism that
> > exchanges the branches of a conditional.
>
> Can the data which are provided because of isomorphism transformations
> be distinguished by an additional attribute?

I think that you just want to disable the isomorphism.  Put disable neg_if
in the initial @@ of your rule.

julia


>
>
> > I think that you could have figured this out by yourself.  If you just
> > printed out the data instead of wrapping it in lots of database noise,
> > then you could have seen what was going on.
>
> I got the impression that there are further software development
> challenges to consider (also for safer counting of record sets).
>
> Regards,
> Markus
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] More support for SmPL data processing with databases?

[Markus Elfring]

>>> Maybe you consider the following results to be a problem:
>>>
>>> kzalloc mcde [("mcde_probe", 2, 0, 198, 1, "m.c", 16, 1, 16, 3)]
>>> kzalloc ! mcde [("mcde_probe", 2, 0, 198, 1, "m.c", 16, 1, 16, 3)]
>>> platform_get_irq irq [("mcde_probe", 2, 0, 198, 1, "m.c", 97, 1, 97, 3)]
>>> platform_get_irq ! irq [("mcde_probe", 2, 0, 198, 1, "m.c", 97, 1, 97, 3)]
>>>
>>> The positions are the same.  The result is because of an isomorphism that
>>> exchanges the branches of a conditional.

I would find it nice to explain the different software behaviour for the mentioned
SmPL code variants

A:
 if@p (
(     <+... result ...+>
&     check
)     )


B:
 if (
(   <+... result ...+>
&   check@p
)   )



>> Can the data which are provided because of isomorphism transformations
>> be distinguished by an additional attribute?
>
> I think that you just want to disable the isomorphism.  Put disable neg_if
> in the initial @@ of your rule.

Thanks for this suggestion.

Source code analysis can be continued also with this configuration option.


Now I become also more interested in improvements around parallel data processing.
Which data structures would support aggregate computations for a growing code base?

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

Re: [Cocci] More support for SmPL data processing with databases?

[Julia Lawall]

On Mon, 13 Apr 2020, Markus Elfring wrote:

> >>> Maybe you consider the following results to be a problem:
> >>>
> >>> kzalloc mcde [("mcde_probe", 2, 0, 198, 1, "m.c", 16, 1, 16, 3)]
> >>> kzalloc ! mcde [("mcde_probe", 2, 0, 198, 1, "m.c", 16, 1, 16, 3)]
> >>> platform_get_irq irq [("mcde_probe", 2, 0, 198, 1, "m.c", 97, 1, 97, 3)]
> >>> platform_get_irq ! irq [("mcde_probe", 2, 0, 198, 1, "m.c", 97, 1, 97, 3)]
> >>>
> >>> The positions are the same.  The result is because of an isomorphism that
> >>> exchanges the branches of a conditional.
>
> I would find it nice to explain the different software behaviour for the mentioned
> SmPL code variants

Run spatch -parse-cocci and you will easily see.  The if stays in the same
place.  When the isomorphism exchanges the branches, the condition gets
negated, so in one case you get the position of the full condition and in
the other case you get the position of the part of the condition under the
negation.

julia

>
> A:
>  if@p (
> (     <+... result ...+>
> &     check
> )     )
>
>
> B:
>  if (
> (   <+... result ...+>
> &   check@p
> )   )
>
>
>
> >> Can the data which are provided because of isomorphism transformations
> >> be distinguished by an additional attribute?
> >
> > I think that you just want to disable the isomorphism.  Put disable neg_if
> > in the initial @@ of your rule.
>
> Thanks for this suggestion.
>
> Source code analysis can be continued also with this configuration option.
>
>
> Now I become also more interested in improvements around parallel data processing.
> Which data structures would support aggregate computations for a growing code base?
>
> Regards,
> Markus
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] More support for SmPL data processing with databases?

[Markus Elfring]

>> I would find it nice to explain the different software behaviour for the mentioned
>> SmPL code variants
>
> Run spatch -parse-cocci and you will easily see.  The if stays in the same
> place.  When the isomorphism exchanges the branches, the condition gets
> negated, so in one case you get the position of the full condition and
> in the other case you get the position of the part of the condition under
> the negation.

Do any of these transformations avoid the reporting of duplicate source
code positions?


>>> I think that you just want to disable the isomorphism.  Put disable neg_if
>>> in the initial @@ of your rule.

Will a need evolve to switch isomorphisms completely off for safer
(and efficient) analysis of original source code?

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

Re: [Cocci] More support for SmPL data processing with databases?

[Julia Lawall]

On Mon, 13 Apr 2020, Markus Elfring wrote:

> >> I would find it nice to explain the different software behaviour for the mentioned
> >> SmPL code variants
> >
> > Run spatch -parse-cocci and you will easily see.  The if stays in the same
> > place.  When the isomorphism exchanges the branches, the condition gets
> > negated, so in one case you get the position of the full condition and
> > in the other case you get the position of the part of the condition under
> > the negation.
>
> Do any of these transformations avoid the reporting of duplicate source
> code positions?

Scripts are run for each set of metavariable bindings.  But there is no
special treatment of positions.

julia

>
>
> >>> I think that you just want to disable the isomorphism.  Put disable neg_if
> >>> in the initial @@ of your rule.
>
> Will a need evolve to switch isomorphisms completely off for safer
> (and efficient) analysis of original source code?
>
> Regards,
> Markus
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] More support for SmPL data processing with databases?

[Markus Elfring]

>>>>> I think that you just want to disable the isomorphism.  Put disable neg_if
>>>>> in the initial @@ of your rule.
>>
>> Will a need evolve to switch isomorphisms completely off for safer
>> (and efficient) analysis of original source code?

Will the corresponding system configuration get any further development considerations?

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

Re: [Cocci] Checking the application of the SmPL isomorphism “drop_else”

[Markus Elfring]

> I hoped for another clarification also for the message “warning: iso drop_else
> does not match the code below on line 55” (and the corresponding debug display).

I imagine that there is another bit of fine-tuning possible.


>> There is an isomorphism that drops else's in ifs, but that only applies if
>> the else is a metavariable that is not used elsewhere.
>
> I am looking for further improvements in this software area.

https://github.com/coccinelle/coccinelle/blob/6b4bb692f208bfe86e62a616724570d9310c7150/standard.iso#L421

It seems that the code transformations “neg_if” and “ne_if” are applied
before “drop_else”.
Should the warning be avoided then if the else branch became non-empty
because of the conversion?

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