Looking for places where there is an of_node_put on some paths
but not on others. This SmPL checks that there is a put of the
same data elsewhere in the function, so perhaps that will
alleviate the concern about puts where they are not needed,
while still making it possible to find the ones that are needed.
Suggested-by: Julia Lawall <[email protected]>
Signed-off-by: Wen Yang <[email protected]>
Reviewed-by: Julia Lawall <[email protected]>
---
scripts/coccinelle/free/of_node_put.cocci | 57 +++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
create mode 100644 scripts/coccinelle/free/of_node_put.cocci
diff --git a/scripts/coccinelle/free/of_node_put.cocci b/scripts/coccinelle/free/of_node_put.cocci
new file mode 100644
index 0000000..6a29830
--- /dev/null
+++ b/scripts/coccinelle/free/of_node_put.cocci
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0
+/// Find missing of_node_put
+///
+// Confidence: Moderate
+// Comments:
+// Options: --no-includes --include-headers
+
+virtual report
+virtual org
+
+@r exists@
+local idexpression struct device_node *x;
+identifier f;
+statement S,S1,S2;
+expression e,e1;
+position p1,p2;
+type T,T1;
+@@
+
+(
+x = f@p1(...);
+... when != e = (T)x
+if (x == NULL || ...) S1
+else S2
+... when != of_node_put(x)
+ when != if (x) { ... of_node_put(x) ... }
+ when != e1 = (T1)x
+(
+return x;
+|
+return@p2 ...;
+)
+&
+x = f(...)
+...
+if (<+...x...+>) S
+...
+of_node_put(x);
+)
+
+@script:python depends on report@
+p1 << r.p1;
+p2 << r.p2;
+@@
+
+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.")
+
+@script:python depends on org@
+p1 << r.p1;
+p2 << r.p2;
+@@
+
+cocci.print_main("acquired a node pointer with refcount incremented", p1)
+cocci.print_secs("needed of_node_put", p2)
--
2.9.5
On Fri, 15 Mar 2019, Wen Yang wrote:
> Looking for places where there is an of_node_put on some paths
> but not on others. This SmPL checks that there is a put of the
> same data elsewhere in the function, so perhaps that will
> alleviate the concern about puts where they are not needed,
> while still making it possible to find the ones that are needed.
>
> Suggested-by: Julia Lawall <[email protected]>
> Signed-off-by: Wen Yang <[email protected]>
> Reviewed-by: Julia Lawall <[email protected]>
> ---
> scripts/coccinelle/free/of_node_put.cocci | 57 +++++++++++++++++++++++++++++++
> 1 file changed, 57 insertions(+)
> create mode 100644 scripts/coccinelle/free/of_node_put.cocci
>
> diff --git a/scripts/coccinelle/free/of_node_put.cocci b/scripts/coccinelle/free/of_node_put.cocci
> new file mode 100644
> index 0000000..6a29830
> --- /dev/null
> +++ b/scripts/coccinelle/free/of_node_put.cocci
> @@ -0,0 +1,57 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/// Find missing of_node_put
> +///
> +// Confidence: Moderate
> +// Comments:
> +// Options: --no-includes --include-headers
> +
> +virtual report
> +virtual org
> +
> +@r exists@
> +local idexpression struct device_node *x;
> +identifier f;
> +statement S,S1,S2;
> +expression e,e1;
> +position p1,p2;
> +type T,T1;
> +@@
> +
> +(
> +x = f@p1(...);
> +... when != e = (T)x
I suggest the following:
(
if (x) { ... of_node_put(x) ... }
|
> +if (x == NULL || ...) S1
> +else S2
> +... when != of_node_put(x)
> + when != if (x) { ... of_node_put(x) ... }
> + when != e1 = (T1)x
> +(
> +return x;
> +|
> +return@p2 ...;
> +)
)
If the first test is for success of the allocation and may lead to an
of_node_put, then you can stop. Perhaps
if (x) { ... when forall
of_node_put(x) ... }
there would be better, to check if there is always a put. This could also
be done on the other if (x)
> +&
> +x = f(...)
> +...
> +if (<+...x...+>) S
> +...
> +of_node_put(x);
There is actually an opportunity here for more reports. Perhaps we can
assume that if the function calls of_node_put on anything, then it is
needed on everything. So this could be of_node_put(...). But the
downside of that is that the original x = f(...) may now let through
things that are not reference counted. So you would want two rules, first
this one where the function is f and there is a of_node_put on the result
of the function, and another one where you consider a known set of
functions, and then allow a subsequent of_node_put on anything.
It would take some care to ensure that there is only one report per call
site.
julia
> +)
> +
> +@script:python depends on report@
> +p1 << r.p1;
> +p2 << r.p2;
> +@@
> +
> +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.")
> +
> +@script:python depends on org@
> +p1 << r.p1;
> +p2 << r.p2;
> +@@
> +
> +cocci.print_main("acquired a node pointer with refcount incremented", p1)
> +cocci.print_secs("needed of_node_put", p2)
> --
> 2.9.5
>
>
> +/// Find missing of_node_put
> +///
> +// Confidence: Moderate
How much would you like to improve the situation around recurring software
development concerns for such source code analysis approaches?
> +virtual report
> +virtual org
I would interpret the provided SmPL code in the way that it will not generate
adjusted (patched) C code so far.
Source code search results will be presented by these operation modes.
How do you think about to exchange the word “patch” by “code search”
at affected places (and in the subject) then?
> +@r exists@
> +local idexpression struct device_node *x;
> +identifier f;
> +statement S,S1,S2;
> +expression e,e1;
> +position p1,p2;
> +type T,T1;
> +@@
> +
> +(
> +x = f@p1(...);
How do you think about to express any more constraints for this function call?
> +... when != e = (T)x
Will it be safer to add another exclusion for the initial assignment target?
> + when != if (x) { ... of_node_put(x) ... }
I find the specification of this extra condition check questionable.
Should Masahiro Yamada be explicitly notified also for this attempt
to integrate another SmPL script?
Regards,
Markus