[all-commits] [llvm/llvm-project] 546c81: [clang-tidy] Improve sizeof(pointer) handling in b...

[Donát Nagy via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 546c816a529835a4cf89deecff957ea336a94fa2
      https://github.com/llvm/llvm-project/commit/546c816a529835a4cf89deecff957ea336a94fa2
  Author: Donát Nagy <donat.nagy at ericsson.com>
  Date:   2024-06-11 (Tue, 11 Jun 2024)

  Changed paths:
    M clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
    M clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h
    M clang-tools-extra/docs/ReleaseNotes.rst
    M clang-tools-extra/docs/clang-tidy/checks/bugprone/sizeof-expression.rst
    M clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-2.c
    A clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-any-pointer.cpp
    M clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp

  Log Message:
  -----------
  [clang-tidy] Improve sizeof(pointer) handling in bugprone-sizeof-expression (#94356)

This commit reimplements the functionality of the Clang Static Analyzer
checker `alpha.core.SizeofPointer` within clang-tidy by adding a new
(off-by-default) option to bugprone-sizeof-expression which activates
reporting all the `sizeof(ptr)` expressions (where ptr is an expression
that produces a pointer).

The main motivation for this change is that `alpha.core.SizeofPointer`
was an AST-based checker, which did not rely on the path sensitive
capabilities of the Static Analyzer, so there was no reason to keep it
in the Static Analyzer instead of the more lightweight clang-tidy.

After this commit I'm planning to create a separate commit that deletes
`alpha.core.SizeofPointer` from Clang Static Analyzer.

It was natural to place this moved logic in bugprone-sizeof-expression,
because that check already provided several heuristics that reported
various especially suspicious classes of `sizeof(ptr)` expressions.

The new mode `WarnOnSizeOfPointer` is off-by-default, so it won't
surprise the existing users; but it can provide a more through coverage
for the vulnerability CWE-467 ("Use of sizeof() on a Pointer Type") than
the existing partial heuristics.

Previously this checker had an exception that the RHS of a
`sizeof(array) / sizeof(array[0])` expression is not reported; I
generalized this to an exception that the check doesn't report
`sizeof(expr[0])` and `sizeof(*expr)`. This idea is taken from the
Static Analyzer checker `alpha.core.SizeofPointer` (which had an
exception for `*expr`), but analysis of open source projects confirmed
that this indeed eliminates lots of unwanted results.

Note that the suppression of `sizeof(expr[0])` and `sizeof(*expr)`
reports also affects the "old" mode `WarnOnSizeOfPointerToAggregate`
which is enabled by default.

This commit also replaces the old message "suspicious usage of
'sizeof(A*)'; pointer to aggregate" with two more concrete messages; but
I feel that this tidy check would deserve a through cleanup of all the
diagnostic messages that it can produce. (I added a FIXME to mark one
outright misleading message.)



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications