2022-03-31 03:22:02

by Marcelo Schmitt

[permalink] [raw]
Subject: [PATCH v3 2/2] Documentation: dev-tools: Enhance static analysis section with discussion

Enhance the static analysis tools section with a discussion on when to
use each of them.

This was mainly taken from Dan Carpenter and Julia Lawall's comments on
a previous documentation patch for static analysis tools.

Lore: https://lore.kernel.org/linux-doc/20220329090911.GX3293@kadam/T/#mb97770c8e938095aadc3ee08f4ac7fe32ae386e6

Signed-off-by: Marcelo Schmitt <[email protected]>
Acked-by: David Gow <[email protected]>
Cc: Dan Carpenter <[email protected]>
Cc: Julia Lawall <[email protected]>
---
Change log v2 -> v3:
- Changed the paragraph about Sparse to make it sound better (hopefully)
- Minor adjusts to make the considerations about Coccinelle sound better
and be precise

Documentation/dev-tools/testing-overview.rst | 32 ++++++++++++++++++++
1 file changed, 32 insertions(+)

diff --git a/Documentation/dev-tools/testing-overview.rst b/Documentation/dev-tools/testing-overview.rst
index b5e02dd3fd94..0aaf6ea53608 100644
--- a/Documentation/dev-tools/testing-overview.rst
+++ b/Documentation/dev-tools/testing-overview.rst
@@ -146,3 +146,35 @@ Documentation/dev-tools/coccinelle.rst documentation page for details.

Beware, though, that static analysis tools suffer from **false positives**.
Errors and warns need to be evaluated carefully before attempting to fix them.
+
+When to use Sparse and Smatch
+-----------------------------
+
+Sparse does type checking, such as verifying that annotated variables do not
+cause endianness bugs, detecting places that use ``__user`` pointers improperly,
+and analyzing the compatibility of symbol initializers.
+
+Smatch does flow analysis and, if allowed to build the function database, it
+also does cross function analysis. Smatch tries to answer questions like where
+is this buffer allocated? How big is it? Can this index be controlled by the
+user? Is this variable larger than that variable?
+
+It's generally easier to write checks in Smatch than it is to write checks in
+Sparse. Nevertheless, there are some overlaps between Sparse and Smatch checks.
+
+Strong points of Smatch and Coccinelle
+--------------------------------------
+
+Coccinelle is probably the easiest for writing checks. It works before the
+pre-processor so it's easier to check for bugs in macros using Coccinelle.
+Coccinelle also creates patches for you, which no other tool does.
+
+For example, with Coccinelle you can do a mass conversion from
+``kmalloc(x * size, GFP_KERNEL)`` to ``kmalloc_array(x, size, GFP_KERNEL)``, and
+that's really useful. If you just created a Smatch warning and try to push the
+work of converting on to the maintainers they would be annoyed. You'd have to
+argue about each warning if can really overflow or not.
+
+Coccinelle does no analysis of variable values, which is the strong point of
+Smatch. On the other hand, Coccinelle allows you to do simple things in a simple
+way.
--
2.35.1


2022-04-02 19:08:30

by David Gow

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] Documentation: dev-tools: Enhance static analysis section with discussion

On Thu, Mar 31, 2022 at 5:50 AM Marcelo Schmitt
<[email protected]> wrote:
>
> Enhance the static analysis tools section with a discussion on when to
> use each of them.
>
> This was mainly taken from Dan Carpenter and Julia Lawall's comments on
> a previous documentation patch for static analysis tools.
>
> Lore: https://lore.kernel.org/linux-doc/20220329090911.GX3293@kadam/T/#mb97770c8e938095aadc3ee08f4ac7fe32ae386e6
>
> Signed-off-by: Marcelo Schmitt <[email protected]>
> Acked-by: David Gow <[email protected]>
> Cc: Dan Carpenter <[email protected]>
> Cc: Julia Lawall <[email protected]>
> ---
> Change log v2 -> v3:
> - Changed the paragraph about Sparse to make it sound better (hopefully)
> - Minor adjusts to make the considerations about Coccinelle sound better
> and be precise

These (plus the cut down note on Sparse/Smatch overlaps) are
definitely an improvement.

Assuming no-one with more knowledge of these that me objects, I think
this is good-to-go!

Thanks,
-- David