[PATCH v3 0/2] Add a section for static analysis tools

[PATCH v3 0/2] Add a section for static analysis tools

[Marcelo Schmitt]

Hi all,

This is the third patch version in the direction of complementing the
testing guide documentation page with information about static analysis
tools.

Thank you for your suggestions and comments so far.
These docs wouldn't get so helpful without them.

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

Change log v1 -> v2:
- New patch adding considerations on when to use each tool
- Brought generic tool characteristics to the intro paragraph
- Made explicit that these tools run at compile time
- Added a note of caution about false positives
- Updated Coccinelle info to make it sound better and be more skimmable


Marcelo Schmitt (2):
  Documentation: dev-tools: Add a section for static analysis tools
  Documentation: dev-tools: Enhance static analysis section with
    discussion

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

-- 
2.35.1

[PATCH v3 1/2] Documentation: dev-tools: Add a section for static analysis tools

[Marcelo Schmitt]

Complement the Kernel Testing Guide documentation page by adding a
section about static analysis tools.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
Acked-by: Daniel Latypov <dlatypov@google.com>
Acked-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Julia Lawall <julia.lawall@inria.fr>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>
---
Change log v2 -> v3:
- Added Julia's acknowledgment tag

Change log v1 -> v2:
- Brought generic tool characteristics to the intro paragraph
- Made explicit that these tools run at compile time
- Added a note of caution about false positives
- Updated Coccinelle info to make it sound better and be more skimmable

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

diff --git a/Documentation/dev-tools/testing-overview.rst b/Documentation/dev-tools/testing-overview.rst
index 65feb81edb14..b5e02dd3fd94 100644
--- a/Documentation/dev-tools/testing-overview.rst
+++ b/Documentation/dev-tools/testing-overview.rst
@@ -115,3 +115,34 @@ that none of these errors are occurring during the test.
 Some of these tools integrate with KUnit or kselftest and will
 automatically fail tests if an issue is detected.
 
+Static Analysis Tools
+=====================
+
+In addition to testing a running kernel, one can also analyze kernel source code
+directly (**at compile time**) using **static analysis** tools. The tools
+commonly used in the kernel allow one to inspect the whole source tree or just
+specific files within it. They make it easier to detect and fix problems during
+the development process.
+
+Sparse can help test the kernel by performing type-checking, lock checking,
+value range checking, in addition to reporting various errors and warnings while
+examining the code. See the Documentation/dev-tools/sparse.rst documentation
+page for details on how to use it.
+
+Smatch extends Sparse and provides additional checks for programming logic
+mistakes such as missing breaks in switch statements, unused return values on
+error checking, forgetting to set an error code in the return of an error path,
+etc. Smatch also has tests against more serious issues such as integer
+overflows, null pointer dereferences, and memory leaks. See the project page at
+http://smatch.sourceforge.net/.
+
+Coccinelle is another static analyzer at our disposal. Coccinelle is often used
+to aid refactoring and collateral evolution of source code, but it can also help
+to avoid certain bugs that occur in common code patterns. The types of tests
+available include API tests, tests for correct usage of kernel iterators, checks
+for the soundness of free operations, analysis of locking behavior, and further
+tests known to help keep consistent kernel usage. See the
+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.
-- 
2.35.1

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

[Marcelo Schmitt]

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 <marcelo.schmitt1@gmail.com>
Acked-by: David Gow <davidgow@google.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Julia Lawall <julia.lawall@inria.fr>
---
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

Re: [PATCH v3 0/2] Add a section for static analysis tools

[David Gow]

On Thu, Mar 31, 2022 at 5:49 AM Marcelo Schmitt
<marcelo.schmitt1@gmail.com> wrote:
>
> Hi all,
>
> This is the third patch version in the direction of complementing the
> testing guide documentation page with information about static analysis
> tools.
>
> Thank you for your suggestions and comments so far.
> These docs wouldn't get so helpful without them.
>
> 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
>
> Change log v1 -> v2:
> - New patch adding considerations on when to use each tool
> - Brought generic tool characteristics to the intro paragraph
> - Made explicit that these tools run at compile time
> - Added a note of caution about false positives
> - Updated Coccinelle info to make it sound better and be more skimmable
>
>
> Marcelo Schmitt (2):
>   Documentation: dev-tools: Add a section for static analysis tools
>   Documentation: dev-tools: Enhance static analysis section with
>     discussion
>
>  Documentation/dev-tools/testing-overview.rst | 63 ++++++++++++++++++++
>  1 file changed, 63 insertions(+)
>

This is looking pretty good to me: thanks for helping to improve the
documentation!

CCing Hu Haowen as an FYI for the zh_CN translation.

-- David

Re: [PATCH v3 1/2] Documentation: dev-tools: Add a section for static analysis tools

[Dongliang Mu]

On Thu, Mar 31, 2022 at 12:07 PM Marcelo Schmitt
<marcelo.schmitt1@gmail.com> wrote:
>
> Complement the Kernel Testing Guide documentation page by adding a
> section about static analysis tools.
>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
> Acked-by: Daniel Latypov <dlatypov@google.com>
> Acked-by: Dan Carpenter <dan.carpenter@oracle.com>
> Acked-by: Julia Lawall <julia.lawall@inria.fr>
> Reviewed-by: David Gow <davidgow@google.com>
> Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>
> ---
> Change log v2 -> v3:
> - Added Julia's acknowledgment tag
>
> Change log v1 -> v2:
> - Brought generic tool characteristics to the intro paragraph
> - Made explicit that these tools run at compile time
> - Added a note of caution about false positives
> - Updated Coccinelle info to make it sound better and be more skimmable
>
>  Documentation/dev-tools/testing-overview.rst | 31 ++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/Documentation/dev-tools/testing-overview.rst b/Documentation/dev-tools/testing-overview.rst
> index 65feb81edb14..b5e02dd3fd94 100644
> --- a/Documentation/dev-tools/testing-overview.rst
> +++ b/Documentation/dev-tools/testing-overview.rst
> @@ -115,3 +115,34 @@ that none of these errors are occurring during the test.
>  Some of these tools integrate with KUnit or kselftest and will
>  automatically fail tests if an issue is detected.
>
> +Static Analysis Tools
> +=====================
> +
> +In addition to testing a running kernel, one can also analyze kernel source code
> +directly (**at compile time**) using **static analysis** tools. The tools
> +commonly used in the kernel allow one to inspect the whole source tree or just
> +specific files within it. They make it easier to detect and fix problems during
> +the development process.
> +
> +Sparse can help test the kernel by performing type-checking, lock checking,
> +value range checking, in addition to reporting various errors and warnings while
> +examining the code. See the Documentation/dev-tools/sparse.rst documentation
> +page for details on how to use it.
> +
> +Smatch extends Sparse and provides additional checks for programming logic
> +mistakes such as missing breaks in switch statements, unused return values on
> +error checking, forgetting to set an error code in the return of an error path,
> +etc. Smatch also has tests against more serious issues such as integer
> +overflows, null pointer dereferences, and memory leaks. See the project page at
> +http://smatch.sourceforge.net/.
> +
> +Coccinelle is another static analyzer at our disposal. Coccinelle is often used
> +to aid refactoring and collateral evolution of source code, but it can also help
> +to avoid certain bugs that occur in common code patterns. The types of tests
> +available include API tests, tests for correct usage of kernel iterators, checks
> +for the soundness of free operations, analysis of locking behavior, and further
> +tests known to help keep consistent kernel usage. See the
> +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.

Hi Marcelo,

Should we include static analysis tools based on LLVM? For example,
Clang static analysis.

> --
> 2.35.1
>

Re: [PATCH v3 1/2] Documentation: dev-tools: Add a section for static analysis tools

[Marcelo Schmitt]

Hi Dongliang,

On 03/31, Dongliang Mu wrote:
> On Thu, Mar 31, 2022 at 12:07 PM Marcelo Schmitt
> <marcelo.schmitt1@gmail.com> wrote:
> >
> > Complement the Kernel Testing Guide documentation page by adding a
> > section about static analysis tools.
> >
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
> > Acked-by: Daniel Latypov <dlatypov@google.com>
> > Acked-by: Dan Carpenter <dan.carpenter@oracle.com>
> > Acked-by: Julia Lawall <julia.lawall@inria.fr>
> > Reviewed-by: David Gow <davidgow@google.com>
> > Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>
> > ---
> > Change log v2 -> v3:
> > - Added Julia's acknowledgment tag
> >
> > Change log v1 -> v2:
> > - Brought generic tool characteristics to the intro paragraph
> > - Made explicit that these tools run at compile time
> > - Added a note of caution about false positives
> > - Updated Coccinelle info to make it sound better and be more skimmable
> >
> >  Documentation/dev-tools/testing-overview.rst | 31 ++++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> >
> > diff --git a/Documentation/dev-tools/testing-overview.rst b/Documentation/dev-tools/testing-overview.rst
> > index 65feb81edb14..b5e02dd3fd94 100644
> > --- a/Documentation/dev-tools/testing-overview.rst
> > +++ b/Documentation/dev-tools/testing-overview.rst
> > @@ -115,3 +115,34 @@ that none of these errors are occurring during the test.
> >  Some of these tools integrate with KUnit or kselftest and will
> >  automatically fail tests if an issue is detected.
> >
> > +Static Analysis Tools
> > +=====================
> > +
> > +In addition to testing a running kernel, one can also analyze kernel source code
> > +directly (**at compile time**) using **static analysis** tools. The tools
> > +commonly used in the kernel allow one to inspect the whole source tree or just
> > +specific files within it. They make it easier to detect and fix problems during
> > +the development process.
> > +
> > +Sparse can help test the kernel by performing type-checking, lock checking,
> > +value range checking, in addition to reporting various errors and warnings while
> > +examining the code. See the Documentation/dev-tools/sparse.rst documentation
> > +page for details on how to use it.
> > +
> > +Smatch extends Sparse and provides additional checks for programming logic
> > +mistakes such as missing breaks in switch statements, unused return values on
> > +error checking, forgetting to set an error code in the return of an error path,
> > +etc. Smatch also has tests against more serious issues such as integer
> > +overflows, null pointer dereferences, and memory leaks. See the project page at
> > +http://smatch.sourceforge.net/.
> > +
> > +Coccinelle is another static analyzer at our disposal. Coccinelle is often used
> > +to aid refactoring and collateral evolution of source code, but it can also help
> > +to avoid certain bugs that occur in common code patterns. The types of tests
> > +available include API tests, tests for correct usage of kernel iterators, checks
> > +for the soundness of free operations, analysis of locking behavior, and further
> > +tests known to help keep consistent kernel usage. See the
> > +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.
> 
> Hi Marcelo,
> 
> Should we include static analysis tools based on LLVM? For example,
> Clang static analysis.

I think that would be a good addition. I haven't checked out Clang tools
though, so it would take me a bit more time to write something about that.

> 
> > --
> > 2.35.1
> >

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

[David Gow]

On Thu, Mar 31, 2022 at 5:50 AM Marcelo Schmitt
<marcelo.schmitt1@gmail.com> 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 <marcelo.schmitt1@gmail.com>
> Acked-by: David Gow <davidgow@google.com>
> Cc: Dan Carpenter <dan.carpenter@oracle.com>
> Cc: Julia Lawall <julia.lawall@inria.fr>
> ---
> 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

Re: [PATCH v3 1/2] Documentation: dev-tools: Add a section for static analysis tools

[Jonathan Corbet]

Marcelo Schmitt <marcelo.schmitt1@gmail.com> writes:

> On 03/31, Dongliang Mu wrote:

>> Should we include static analysis tools based on LLVM? For example,
>> Clang static analysis.
>
> I think that would be a good addition. I haven't checked out Clang tools
> though, so it would take me a bit more time to write something about that.

That seems like a good topic for a future patch.  Meanwhile I've applied
this series, thanks.

jon