[PATCH] coccinelle: Check for missing NULL terminators in of_device_id tables

[PATCH] coccinelle: Check for missing NULL terminators in of_device_id tables

[Stephen Boyd]

Failure to terminate an of_device_id table can lead to confusing
failures depending on where the compiler places the array. Add a
check to make sure these tables are terminated. Thanks to Mitchel
Humpherys for coming up with the initial pattern.

Cc: Mitchel Humpherys <mitchelh@codeaurora.org>
Cc: Julia Lawall <Julia.Lawall@lip6.fr>
Cc: Gilles Muller <Gilles.Muller@lip6.fr>
Cc: Nicolas Palix <nicolas.palix@imag.fr>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 scripts/coccinelle/misc/of_table.cocci | 81 ++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)
 create mode 100644 scripts/coccinelle/misc/of_table.cocci

diff --git a/scripts/coccinelle/misc/of_table.cocci b/scripts/coccinelle/misc/of_table.cocci
new file mode 100644
index 000000000000..da8bfa69fa3e
--- /dev/null
+++ b/scripts/coccinelle/misc/of_table.cocci
@@ -0,0 +1,81 @@
+/// Make sure of_device_id tables are NULL terminated
+//
+// Keywords: of_table
+// Confidence: Medium
+// Options: --include-headers
+
+virtual patch
+virtual context
+virtual org
+virtual report
+
+@bad_of_table exists@
+identifier var, arr;
+expression E;
+@@
+... struct of_device_id arr[] = {
+	...,
+	{
+	...,
+	.var = E,
+	...
+	}
+};
+
+@depends on bad_of_table && context@
+identifier var, arr;
+expression E;
+@@
+ ... struct of_device_id arr[] = {
+	...,
+	{
+	...,
+*	.var = E,
+	...
+	}
+};
+
+@depends on bad_of_table && patch@
+identifier var, arr;
+expression E;
+@@
+... struct of_device_id arr[] = {
+	...,
+	{
+	...,
+	.var = E,
+	...
+-	}
++	},
++	{ }
+};
+
+@r depends on bad_of_table exists@
+position p1;
+identifier var, arr;
+expression E;
+@@
+... struct of_device_id arr[] = {
+	...,
+	{
+	...,
+	.var = E,
+	...
+	}
+	@p1
+};
+
+@script:python depends on org@
+p1 << r.p1;
+arr << r.arr;
+@@
+
+cocci.print_main(arr,p1)
+
+@script:python depends on report@
+p1 << r.p1;
+arr << r.arr;
+@@
+
+msg = "%s is not NULL terminated at line %s" % (arr, p1[0].line)
+coccilib.report.print_report(p1[0],msg)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH] coccinelle: Check for missing NULL terminators in of_device_id tables

[Julia Lawall]

On Fri, 23 May 2014, Stephen Boyd wrote:

> Failure to terminate an of_device_id table can lead to confusing
> failures depending on where the compiler places the array. Add a
> check to make sure these tables are terminated. Thanks to Mitchel
> Humpherys for coming up with the initial pattern.
> 
> Cc: Mitchel Humpherys <mitchelh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Cc: Julia Lawall <Julia.Lawall-L2FTfq7BK8M@public.gmane.org>
> Cc: Gilles Muller <Gilles.Muller-L2FTfq7BK8M@public.gmane.org>
> Cc: Nicolas Palix <nicolas.palix-wMUr/XMoy4A@public.gmane.org>
> Cc: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
>  scripts/coccinelle/misc/of_table.cocci | 81 ++++++++++++++++++++++++++++++++++
>  1 file changed, 81 insertions(+)
>  create mode 100644 scripts/coccinelle/misc/of_table.cocci
> 
> diff --git a/scripts/coccinelle/misc/of_table.cocci b/scripts/coccinelle/misc/of_table.cocci
> new file mode 100644
> index 000000000000..da8bfa69fa3e
> --- /dev/null
> +++ b/scripts/coccinelle/misc/of_table.cocci
> @@ -0,0 +1,81 @@
> +/// Make sure of_device_id tables are NULL terminated
> +//
> +// Keywords: of_table
> +// Confidence: Medium
> +// Options: --include-headers
> +
> +virtual patch
> +virtual context
> +virtual org
> +virtual report
> +
> +@bad_of_table exists@
> +identifier var, arr;
> +expression E;
> +@@
> +... struct of_device_id arr[] = {
> +	...,
> +	{
> +	...,
> +	.var = E,
> +	...
> +	}
> +};

You shouldn't need any of the ...s in these rules.

julia

> +@depends on bad_of_table && context@
> +identifier var, arr;
> +expression E;
> +@@
> + ... struct of_device_id arr[] = {
> +	...,
> +	{
> +	...,
> +*	.var = E,
> +	...
> +	}
> +};
> +
> +@depends on bad_of_table && patch@
> +identifier var, arr;
> +expression E;
> +@@
> +... struct of_device_id arr[] = {
> +	...,
> +	{
> +	...,
> +	.var = E,
> +	...
> +-	}
> ++	},
> ++	{ }
> +};
> +
> +@r depends on bad_of_table exists@
> +position p1;
> +identifier var, arr;
> +expression E;
> +@@
> +... struct of_device_id arr[] = {
> +	...,
> +	{
> +	...,
> +	.var = E,
> +	...
> +	}
> +	@p1
> +};
> +
> +@script:python depends on org@
> +p1 << r.p1;
> +arr << r.arr;
> +@@
> +
> +cocci.print_main(arr,p1)
> +
> +@script:python depends on report@
> +p1 << r.p1;
> +arr << r.arr;
> +@@
> +
> +msg = "%s is not NULL terminated at line %s" % (arr, p1[0].line)
> +coccilib.report.print_report(p1[0],msg)
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] coccinelle: Check for missing NULL terminators in of_device_id tables

[Rob Herring]

On Fri, May 23, 2014 at 6:41 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> Failure to terminate an of_device_id table can lead to confusing
> failures depending on where the compiler places the array. Add a
> check to make sure these tables are terminated. Thanks to Mitchel
> Humpherys for coming up with the initial pattern.

I've definitely debugged one of those problems. Do you have any actual
fixes or there are no errors in the kernel?

Rob

>
> Cc: Mitchel Humpherys <mitchelh@codeaurora.org>
> Cc: Julia Lawall <Julia.Lawall@lip6.fr>
> Cc: Gilles Muller <Gilles.Muller@lip6.fr>
> Cc: Nicolas Palix <nicolas.palix@imag.fr>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  scripts/coccinelle/misc/of_table.cocci | 81 ++++++++++++++++++++++++++++++++++
>  1 file changed, 81 insertions(+)
>  create mode 100644 scripts/coccinelle/misc/of_table.cocci
>
> diff --git a/scripts/coccinelle/misc/of_table.cocci b/scripts/coccinelle/misc/of_table.cocci
> new file mode 100644
> index 000000000000..da8bfa69fa3e
> --- /dev/null
> +++ b/scripts/coccinelle/misc/of_table.cocci
> @@ -0,0 +1,81 @@
> +/// Make sure of_device_id tables are NULL terminated
> +//
> +// Keywords: of_table
> +// Confidence: Medium
> +// Options: --include-headers
> +
> +virtual patch
> +virtual context
> +virtual org
> +virtual report
> +
> +@bad_of_table exists@
> +identifier var, arr;
> +expression E;
> +@@
> +... struct of_device_id arr[] = {
> +       ...,
> +       {
> +       ...,
> +       .var = E,
> +       ...
> +       }
> +};
> +
> +@depends on bad_of_table && context@
> +identifier var, arr;
> +expression E;
> +@@
> + ... struct of_device_id arr[] = {
> +       ...,
> +       {
> +       ...,
> +*      .var = E,
> +       ...
> +       }
> +};
> +
> +@depends on bad_of_table && patch@
> +identifier var, arr;
> +expression E;
> +@@
> +... struct of_device_id arr[] = {
> +       ...,
> +       {
> +       ...,
> +       .var = E,
> +       ...
> +-      }
> ++      },
> ++      { }
> +};
> +
> +@r depends on bad_of_table exists@
> +position p1;
> +identifier var, arr;
> +expression E;
> +@@
> +... struct of_device_id arr[] = {
> +       ...,
> +       {
> +       ...,
> +       .var = E,
> +       ...
> +       }
> +       @p1
> +};
> +
> +@script:python depends on org@
> +p1 << r.p1;
> +arr << r.arr;
> +@@
> +
> +cocci.print_main(arr,p1)
> +
> +@script:python depends on report@
> +p1 << r.p1;
> +arr << r.arr;
> +@@
> +
> +msg = "%s is not NULL terminated at line %s" % (arr, p1[0].line)
> +coccilib.report.print_report(p1[0],msg)
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
>

Re: [PATCH] coccinelle: Check for missing NULL terminators in of_device_id tables

[Stephen Boyd]

On 05/25/14 07:13, Rob Herring wrote:
> On Fri, May 23, 2014 at 6:41 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> Failure to terminate an of_device_id table can lead to confusing
>> failures depending on where the compiler places the array. Add a
>> check to make sure these tables are terminated. Thanks to Mitchel
>> Humpherys for coming up with the initial pattern.
> I've definitely debugged one of those problems. Do you have any actual
> fixes or there are no errors in the kernel?
>
>

Yes I sent five patches out. Here's one for example
https://lkml.org/lkml/2014/5/23/731

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH] coccinelle: Check for missing NULL terminators in of_device_id tables

[Stephen Boyd]

On 05/24/14 00:57, Julia Lawall wrote:
> On Fri, 23 May 2014, Stephen Boyd wrote:
>
>> +... struct of_device_id arr[] = {
>> +	...,
>> +	{
>> +	...,
>> +	.var = E,
>> +	...
>> +	}
>> +};
> You shouldn't need any of the ...s in these rules.
>

Hm.. Removing all the ...s in these rules only gives me 3 matches now. I
used to get 5.

Before

$ COCCI=scripts/coccinelle/misc/of_table.cocci make coccicheck MODE=report

Please check for false positives in the output before submitting a patch.
When using "patch" mode, carefully review the patch before submitting it.

./sound/soc/pxa/pxa-ssp.c:810:36-37: pxa_ssp_of_ids is not NULL terminated at line 810
./drivers/mfd/tps65218.c:199:32-33: of_tps65218_match_table is not NULL terminated at line 199
./drivers/staging/mt29f_spinand/mt29f_spinand.c:926:34-35: spinand_dt is not NULL terminated at line 926
./drivers/clk/clk-u300.c:1170:1-2: u300_clk_match is not NULL terminated at line 1170
./drivers/clk/st/clkgen-pll.c:655:1-2: c32_gpu_pll_of_match is not NULL terminated at line 655


After

$ COCCI=scripts/coccinelle/misc/of_table.cocci make coccicheck MODE=report

Please check for false positives in the output before submitting a patch.
When using "patch" mode, carefully review the patch before submitting it.

./sound/soc/pxa/pxa-ssp.c:810:36-37: pxa_ssp_of_ids is not NULL terminated at line 810
./drivers/mfd/tps65218.c:199:32-33: of_tps65218_match_table is not NULL terminated at line 199
./drivers/staging/mt29f_spinand/mt29f_spinand.c:926:34-35: spinand_dt is not NULL terminated at line 926


Removing most of the ...s seems to work though. I'll send a v2.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html