Coccinelle archive on lore.kernel.org
 help / color / Atom feed
From: <wen.yang99@zte.com.cn>
To: <julia.lawall@lip6.fr>
Cc: wang.yi59@zte.com.cn, michal.lkml@markovi.net,
	nicolas.palix@imag.fr, linux-kernel@vger.kernel.org,
	Markus.Elfring@web.de, cocci@systeme.lip6.fr
Subject: Re: [Cocci] [PATCH v2] coccinelle: semantic code search formissingof_node_put
Date: Fri, 5 Jul 2019 13:57:24 +0800 (CST)
Message-ID: <201907051357245235750@zte.com.cn> (raw)
In-Reply-To: <alpine.DEB.2.21.1907050727550.18245@hadrien>

[-- Attachment #1.1: Type: text/plain, Size: 5877 bytes --]

> > > > > +x = @p1\(of_find_all_nodes\|
> > > >
> > > > I would find this SmPL disjunction easier to read without the usage
> > > > of extra backslashes.
> > > >
> > > > +x =
> > > > +(of_…
> > > > +|of_…
> > > > +)@p1(...);
> > >
> > > Did you actually test this?  I doubt that a position metavariable can be
> > > put on a ) of a disjunction.
> > >
> > > > > +|
> > > > > +return x;
> > > > > +|
> > > > > +return of_fwnode_handle(x);
> > > >
> > > > Can a nested SmPL disjunction be helpful at such places?
> > > >
> > > > +|return
> > > > +(x
> > > > +|of_fwnode_handle(x)
> > > > +);
> > >
> > > The original code is much more readable.  The internal representation will
> > > be the same.
> > >
> > > > > +    when != v4l2_async_notifier_add_fwnode_subdev(<...x...>)
> > > >
> > > > Would the specification variant “<+... x ...+>” be relevant
> > > > for the parameter selection?
> > >
> > > I'm indeed quite surprised that <...x...> would be accepted by the parser..
> >
> > Hi julia,
> >
> > Thank you for your comments.
> > We tested and found that both <...x...> and <+... x ...+> variants work fine.
> > We use <... x ...> instead of <+... x ...+> here to eliminate the following false positives:
> >
> > ./drivers/media/platform/qcom/camss/camss.c:504:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 479, but without a corresponding object release within this function.
> >
> > 465 static int camss_of_parse_ports(struct camss *camss)
> > 466 {
> > ...
> > 479 remote = of_graph_get_remote_port_parent(node);
> > ...
> > 486 asd = v4l2_async_notifier_add_fwnode_subdev(
> > 487 &camss->notifier, of_fwnode_handle(remote), ---> v4l2_async_notifier_add_fwnode_subdev will pass remote to camss->notifier.
> > 488 sizeof(*csd));
> > ...
> > 504 return num_subdevs;
> 
> I suspect that what is happening is that there is a runtime error, but
> that error is caught somewhere and you don't see it. 

Thanks.
You are right, there is indeed a runtime error. 
Since make coccicheck adds the "-very-quiet" parameter by default, we didn't find it.

$ spatch --sp-file   of_node_put.cocci   -D report drivers/media/platform/am437x/am437x-vpfe.c
init_defs_builtins: /usr/local/bin/../lib/coccinelle/standard.h
HANDLING: drivers/media/platform/am437x/am437x-vpfe.c
exn while in timeout_function
only handling multi and no when code in a nest expr

>  Could you send me again the entire semantic patch so I can check on this?
> 

Thanks.
The entire SmPL is as follows:

$ cat of_node_put.cocci
// SPDX-License-Identifier: GPL-2.0
/// Find missing of_node_put
///
// Confidence: Moderate
// Copyright: (C) 2018-2019 Wen Yang, ZTE.
// Comments:
// Options: --no-includes --include-headers

virtual report
virtual org

@initialize:python@
@@

seen = set()

def add_if_not_present (p1, p2):
    if (p1, p2) not in seen:
        seen.add((p1, p2))
        return True
    return False

def display_report(p1, p2):
    if add_if_not_present(p1[0].line, p2[0].line):
       coccilib.report.print_report(p2[0],
                                    "ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line "
                                    + p1[0].line
                                    + ", but without a corresponding object release within this function.")

def display_org(p1, p2):
    cocci.print_main("acquired a node pointer with refcount incremented", p1)
    cocci.print_secs("needed of_node_put", p2)

@r1 exists@
local idexpression struct device_node *x;
expression e, e1;
position p1, p2;
statement S;
type T;
@@

x = @p1\(of_find_all_nodes\|
         of_get_cpu_node\|
         of_get_parent\|
         of_get_next_parent\|
         of_get_next_child\|
         of_get_next_cpu_node\|
         of_get_compatible_child\|
         of_get_child_by_name\|
         of_find_node_opts_by_path\|
         of_find_node_by_name\|
         of_find_node_by_type\|
         of_find_compatible_node\|
         of_find_node_with_property\|
         of_find_matching_node_and_match\|
         of_find_node_by_phandle\|
         of_parse_phandle\|
         of_find_next_cache_node\|
         of_get_next_available_child\)(...);
...
if (x == NULL || ...) S
... when != e = (T)x
    when != true x == NULL
    when != of_node_put(x)
    when != of_get_next_parent(x)
    when != of_find_matching_node(x, ...)
    when != if (x) { ... return x; }
    when != v4l2_async_notifier_add_fwnode_subdev(<...x...>)
    when != e1 = of_fwnode_handle(x)
(
if (x) { ... when forall
         of_node_put(x) ... }
|
return x;
|
return of_fwnode_handle(x);
|
return@p2 ...;
)

@script:python depends on report && r1@
p1 << r1.p1;
p2 << r1.p2;
@@

display_report(p1, p2)

@script:python depends on org && r1@
p1 << r1.p1;
p2 << r1.p2;
@@

display_org(p1, p2)

@r2 exists@
local idexpression struct device_node *x;
expression e, e1;
position p1, p2;
identifier f;
statement S;
type T;
@@

(
x = f@p1(...);
... when != e = (T)x
    when != true x == NULL
    when != of_node_put(x)
    when != of_get_next_parent(x)
    when != of_find_matching_node(x, ...)
    when != if (x) { ... return x; }
    when != v4l2_async_notifier_add_fwnode_subdev(<...x...>)
    when != e1 = of_fwnode_handle(x)
(
if (x) { ... when forall
         of_node_put(x) ... }
|
return x;
|
return of_fwnode_handle(x);
|
return@p2 ...;
)
&
x = f(...)
...
if (<+...x...+>) S
...
of_node_put(x);
)
@script:python depends on report && r2@
p1 << r2.p1;
p2 << r2.p2;
@@

display_report(p1, p2)

@script:python depends on org && r2@
p1 << r2.p1;
p2 << r2.p2;
@@

display_org(p1, p2)

> I think that what you want is:
> 
> when != v4l2_async_notifier_add_fwnode_subdev(...,<+...x...+>,...)
> 
> ie x occurring somewhere within some argument.

Thank you very much for your suggestion. 
Applying it will solve this problem, thank you.

--
Thanks and regards,
Wen

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

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

  reply index

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-28  2:58 [Cocci] [PATCH v2] coccinelle: semantic code search for missing of_node_put Wen Yang
2019-06-28  9:38 ` Markus Elfring
2019-06-28 11:07   ` Julia Lawall
2019-06-28 14:16     ` [Cocci] [v2] " Markus Elfring
2019-06-28 14:16     ` Markus Elfring
2019-07-04  3:03     ` [Cocci] [PATCH v2] coccinelle: semantic code search for missingof_node_put wen.yang99
2019-07-04  6:28       ` [Cocci] [v2] coccinelle: semantic code search for missing of_node_put Markus Elfring
2019-07-05  5:29       ` [Cocci] [PATCH v2] coccinelle: semantic code search for missingof_node_put Julia Lawall
2019-07-05  5:57         ` wen.yang99 [this message]
2019-07-05  6:17           ` [Cocci] [PATCH v2] coccinelle: semantic code search formissingof_node_put Julia Lawall
2019-07-05  6:45             ` [Cocci] [v2] coccinelle: semantic code search for missing of_node_put Markus Elfring
2019-06-29  7:40   ` Markus Elfring
2019-06-29  7:49     ` Julia Lawall
2019-06-29  8:35       ` Markus Elfring
2019-06-29 19:30       ` [Cocci] [v2] Coccinelle: Testing SmPL constraints Markus Elfring
2019-07-04  2:41   ` [Cocci] 答复: Re: [PATCH v2] coccinelle: semantic code search for missingof_node_put wen.yang99
2019-07-04  5:40     ` [Cocci] [v2] coccinelle: semantic code search for missing of_node_put Markus Elfring

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201907051357245235750@zte.com.cn \
    --to=wen.yang99@zte.com.cn \
    --cc=Markus.Elfring@web.de \
    --cc=cocci@systeme.lip6.fr \
    --cc=julia.lawall@lip6.fr \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michal.lkml@markovi.net \
    --cc=nicolas.palix@imag.fr \
    --cc=wang.yi59@zte.com.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Coccinelle archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/cocci/0 cocci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 cocci cocci/ https://lore.kernel.org/cocci \
		cocci@systeme.lip6.fr cocci@archiver.kernel.org
	public-inbox-index cocci


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/fr.lip6.systeme.cocci


AGPL code for this site: git clone https://public-inbox.org/ public-inbox