[Cocci] [PATCH] coccinelle: reduce false positives

[Julia Lawall]

Some files use both a non-devm allocation and a devm_allocation. Don't
complain about a free when the same function contains a non-devm
allocation.

Signed-off-by: Julia Lawall <***@lip6.fr>

---
scripts/coccinelle/free/devm_free.cocci | 55 +++++++++++++++++++++++++++++++-
1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/scripts/coccinelle/free/devm_free.cocci b/scripts/coccinelle/free/devm_free.cocci
index c990d2c..b2a2cf8b 100644
--- a/scripts/coccinelle/free/devm_free.cocci
+++ b/scripts/coccinelle/free/devm_free.cocci
@@ -56,9 +56,62 @@ expression x;
x = devm_ioport_map(...)
)

+@safe depends on context || org || report exists@
+expression x;
+position p;
+@@
+
+(
+ x = kmalloc(...)
+|
+ x = kvasprintf(...)
+|
+ x = kasprintf(...)
+|
+ x = kzalloc(...)
+|
+ x = kmalloc_array(...)
+|
+ x = kcalloc(...)
+|
+ x = kstrdup(...)
+|
+ x = kmemdup(...)
+|
+ x = get_free_pages(...)
+|
+ x = request_irq(...)
+|
+ x = ioremap(...)
+|
+ x = ioremap_nocache(...)
+|
+ x = ioport_map(...)
+)
+...
+(
+ ***@p(x)
+|
+ ***@p(x)
+|
+ ***@p(x, ...)
+|
+ ***@p(x, ...)
+|
+ ***@p(x, ...)
+|
+ ***@p(x)
+|
+ ***@p(x)
+|
+ ***@p(x)
+|
+ ***@p(x)
+)
+
@pb@
expression r.x;
-position p;
+position p != safe.p;
@@

(

[Dan Carpenter]

Post by Julia Lawall
Some files use both a non-devm allocation and a devm_allocation. Don't
complain about a free when the same function contains a non-devm
allocation.

That's surprising... Do you have an example false positive?

regards,
dan carpenter

[Julia Lawall]

Here are the results that are eliminated by my change:

drivers/clk/axs10x/pll_clock.c:323:1-6 kfree(pll_clk);
drivers/clk/clk-gpio.c:131:2-7 kfree(clk_gpio);
drivers/clk/clk-hsdk-pll.c:410:1-6 kfree(pll_clk);
drivers/clk/hisilicon/clk.c:97:1-6 kfree(clk_data);
drivers/mfd/syscon.c:130:1-8 iounmap(base);
drivers/mfd/syscon.c:132:1-6 kfree(syscon);
drivers/pinctrl/freescale/pinctrl-mxs.c:139:2-7 kfree(group);
drivers/pinctrl/samsung/pinctrl-exynos5440.c:264:1-6 kfree(gname);
drivers/platform/chrome/cros_ec_debugfs.c:248:1-6 kfree(msg);
drivers/pwm/pwm-lp3943.c:56:3-8 kfree(pwm_map);

The semantic patch is pretty naive in that it assumes that all uses of the
same name point to the same thing.

Looking through the code, it looks like sometimes both an __init and a
probe function are provided, and the __init function doesn't have access to
a device object.

julia

[Dan Carpenter]

Post by Julia Lawall
drivers/clk/axs10x/pll_clock.c:323:1-6 kfree(pll_clk);
drivers/clk/clk-gpio.c:131:2-7 kfree(clk_gpio);
drivers/clk/clk-hsdk-pll.c:410:1-6 kfree(pll_clk);
drivers/clk/hisilicon/clk.c:97:1-6 kfree(clk_data);
drivers/mfd/syscon.c:130:1-8 iounmap(base);
drivers/mfd/syscon.c:132:1-6 kfree(syscon);
drivers/pinctrl/freescale/pinctrl-mxs.c:139:2-7 kfree(group);
drivers/pinctrl/samsung/pinctrl-exynos5440.c:264:1-6 kfree(gname);
drivers/platform/chrome/cros_ec_debugfs.c:248:1-6 kfree(msg);
drivers/pwm/pwm-lp3943.c:56:3-8 kfree(pwm_map);
The semantic patch is pretty naive in that it assumes that all uses of the
same name point to the same thing.

Huh... It leads to false positives but it is an easier way to do cross
function analysis. I had wondered about that.

regards,
dan carpenter

[Masahiro Yamada]

Post by Julia Lawall
Some files use both a non-devm allocation and a devm_allocation. Don't
complain about a free when the same function contains a non-devm
allocation.
---
scripts/coccinelle/free/devm_free.cocci | 55 +++++++++++++++++++++++++++++++-
1 file changed, 54 insertions(+), 1 deletion(-)
diff --git a/scripts/coccinelle/free/devm_free.cocci b/scripts/coccinelle/free/devm_free.cocci
index c990d2c..b2a2cf8b 100644
--- a/scripts/coccinelle/free/devm_free.cocci
+++ b/scripts/coccinelle/free/devm_free.cocci
@@ -56,9 +56,62 @@ expression x;
x = devm_ioport_map(...)
)
+expression x;
+position p;
+
+(
+ x = kmalloc(...)
+|
+ x = kvasprintf(...)
+|
+ x = kasprintf(...)
+|
+ x = kzalloc(...)
+|
+ x = kmalloc_array(...)
+|
+ x = kcalloc(...)
+|
+ x = kstrdup(...)
+|
+ x = kmemdup(...)
+|
+ x = get_free_pages(...)
+|
+ x = request_irq(...)
+|
+ x = ioremap(...)
+|
+ x = ioremap_nocache(...)
+|
+ x = ioport_map(...)
+)
+...
+(
+|
+|
+|
+|
+|
+|
+|
+|
+)
+
@pb@
expression r.x;
-position p;
+position p != safe.p;
@@
(

Anyway, it looks like
this patch makes the situation better.


Applied to linux-kbuild/kbuild. Thanks!