[analyzer] Finish moving alpha.core.SizeofPtr to clang-tidy by NagyDonat · Pull Request #95118 · llvm/llvm-project (original) (raw)

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

@NagyDonat

The checker alpha.core.SizeofPtr was a very simple checker that did not rely on path sensitive analysis and was very similar to the (more complex and refined) clang-tidy check bugprone-sizeof-expression.

As there is no reason to maintain two separate implementations for the same goal (and clang-tidy is more lightweight and accessible than the Analyzer) I decided to move this functionality from the Static Analyzer to clang-tidy.

Recently my commit 546c816 reimplemented the advantageous parts of alpha.core.SizeofPtr within clang-tidy; now this commit finishes the transfer by deleting alpha.core.SizeofPtr.

@NagyDonat

The checker alpha.core.SizeofPtr was a very simple checker that did not rely on path sensitive analysis and was very similar to the (more complex and refined) clang-tidy check bugprone-sizeof-expression.

As there is no reason to maintain two separate implementations for the same goal (and clang-tidy is more lightweight and accessible than the Analyzer) I decided to move this functionality from the Static Analyzer to clang-tidy.

Recently my commit 546c816 reimplemented the advantageous parts of alpha.core.SizeofPtr within clang-tidy; now this commit finishes the transfer by deleting alpha.core.SizeofPtr.

@llvmbot

@llvm/pr-subscribers-clang

Author: Donát Nagy (NagyDonat)

Changes

The checker alpha.core.SizeofPtr was a very simple checker that did not rely on path sensitive analysis and was very similar to the (more complex and refined) clang-tidy check bugprone-sizeof-expression.

As there is no reason to maintain two separate implementations for the same goal (and clang-tidy is more lightweight and accessible than the Analyzer) I decided to move this functionality from the Static Analyzer to clang-tidy.

Recently my commit 546c816 reimplemented the advantageous parts of alpha.core.SizeofPtr within clang-tidy; now this commit finishes the transfer by deleting alpha.core.SizeofPtr.

Full diff: https://github.com/llvm/llvm-project/pull/95118.diff

6 Files Affected:

diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index f53dd545df5a9..d76ee241da797 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -2452,21 +2452,6 @@ Check for pointer subtractions on two pointers pointing to different memory chun int d = &y - &x; // warn } -.. _alpha-core-SizeofPtr:

-alpha.core.SizeofPtr (C) -"""""""""""""""""""""""" -Warn about unintended use of sizeof() on pointer expressions.

-.. code-block:: c

diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index a7f62ef7f2d07..429c334a0b24b 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -296,10 +296,6 @@ def PointerSubChecker : Checker<"PointerSub">, "different memory chunks">, Documentation; -def SizeofPointerChecker : Checker<"SizeofPtr">, - HelpText<"Warn about unintended use of sizeof() on pointer expressions">, - Documentation;

def TestAfterDivZeroChecker : Checker<"TestAfterDivZero">, HelpText<"Check for division by variable that is later compared against 0. " "Either the comparison is useless or there is division by zero.">, diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt index 68e829cace495..682cfa01bec96 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -24,7 +24,6 @@ add_clang_library(clangStaticAnalyzerCheckers CheckObjCInstMethSignature.cpp CheckPlacementNew.cpp CheckSecuritySyntaxOnly.cpp - CheckSizeofPointer.cpp CheckerDocumentation.cpp ChrootChecker.cpp CloneChecker.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckSizeofPointer.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckSizeofPointer.cpp deleted file mode 100644 index 0d2551f11583e..0000000000000 --- a/clang/lib/StaticAnalyzer/Checkers/CheckSizeofPointer.cpp +++ /dev/null @@ -1,96 +0,0 @@ -//==- CheckSizeofPointer.cpp - Check for sizeof on pointers ------- C++ --==// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// -// -// This file defines a check for unintended use of sizeof() on pointer -// expressions. -// -//===----------------------------------------------------------------------===//

-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" -#include "clang/AST/StmtVisitor.h" -#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" -#include "clang/StaticAnalyzer/Core/Checker.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"

-using namespace clang; -using namespace ento;

-namespace { -class WalkAST : public StmtVisitor { - BugReporter &BR; - const CheckerBase Checker; - AnalysisDeclContext AC;

-public: - WalkAST(BugReporter &br, const CheckerBase *checker, AnalysisDeclContext *ac) - : BR(br), Checker(checker), AC(ac) {} - void VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *E); - void VisitStmt(Stmt *S) { VisitChildren(S); } - void VisitChildren(Stmt *S); -}; -}

-void WalkAST::VisitChildren(Stmt *S) { - for (Stmt *Child : S->children()) - if (Child) - Visit(Child); -}

-// CWE-467: Use of sizeof() on a Pointer Type -void WalkAST::VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *E) { - if (E->getKind() != UETT_SizeOf) - return;

-
-alpha.core.SizeofPtr -(C)
-Warn about unintended use of sizeof() on pointer -expressions.
 -
-
-struct s {};

-int test(struct s *p) { - return sizeof(p); - // warn: sizeof(ptr) can produce an unexpected result -} -

@llvmbot

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Donát Nagy (NagyDonat)

Changes

The checker alpha.core.SizeofPtr was a very simple checker that did not rely on path sensitive analysis and was very similar to the (more complex and refined) clang-tidy check bugprone-sizeof-expression.

As there is no reason to maintain two separate implementations for the same goal (and clang-tidy is more lightweight and accessible than the Analyzer) I decided to move this functionality from the Static Analyzer to clang-tidy.

Recently my commit 546c816 reimplemented the advantageous parts of alpha.core.SizeofPtr within clang-tidy; now this commit finishes the transfer by deleting alpha.core.SizeofPtr.

Full diff: https://github.com/llvm/llvm-project/pull/95118.diff

6 Files Affected:

diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index f53dd545df5a9..d76ee241da797 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -2452,21 +2452,6 @@ Check for pointer subtractions on two pointers pointing to different memory chun int d = &y - &x; // warn } -.. _alpha-core-SizeofPtr:

-alpha.core.SizeofPtr (C) -"""""""""""""""""""""""" -Warn about unintended use of sizeof() on pointer expressions.

-.. code-block:: c

diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index a7f62ef7f2d07..429c334a0b24b 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -296,10 +296,6 @@ def PointerSubChecker : Checker<"PointerSub">, "different memory chunks">, Documentation; -def SizeofPointerChecker : Checker<"SizeofPtr">, - HelpText<"Warn about unintended use of sizeof() on pointer expressions">, - Documentation;

def TestAfterDivZeroChecker : Checker<"TestAfterDivZero">, HelpText<"Check for division by variable that is later compared against 0. " "Either the comparison is useless or there is division by zero.">, diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt index 68e829cace495..682cfa01bec96 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -24,7 +24,6 @@ add_clang_library(clangStaticAnalyzerCheckers CheckObjCInstMethSignature.cpp CheckPlacementNew.cpp CheckSecuritySyntaxOnly.cpp - CheckSizeofPointer.cpp CheckerDocumentation.cpp ChrootChecker.cpp CloneChecker.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckSizeofPointer.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckSizeofPointer.cpp deleted file mode 100644 index 0d2551f11583e..0000000000000 --- a/clang/lib/StaticAnalyzer/Checkers/CheckSizeofPointer.cpp +++ /dev/null @@ -1,96 +0,0 @@ -//==- CheckSizeofPointer.cpp - Check for sizeof on pointers ------- C++ --==// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// -// -// This file defines a check for unintended use of sizeof() on pointer -// expressions. -// -//===----------------------------------------------------------------------===//

-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" -#include "clang/AST/StmtVisitor.h" -#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" -#include "clang/StaticAnalyzer/Core/Checker.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"

-using namespace clang; -using namespace ento;

-namespace { -class WalkAST : public StmtVisitor { - BugReporter &BR; - const CheckerBase Checker; - AnalysisDeclContext AC;

-public: - WalkAST(BugReporter &br, const CheckerBase *checker, AnalysisDeclContext *ac) - : BR(br), Checker(checker), AC(ac) {} - void VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *E); - void VisitStmt(Stmt *S) { VisitChildren(S); } - void VisitChildren(Stmt *S); -}; -}

-void WalkAST::VisitChildren(Stmt *S) { - for (Stmt *Child : S->children()) - if (Child) - Visit(Child); -}

-// CWE-467: Use of sizeof() on a Pointer Type -void WalkAST::VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *E) { - if (E->getKind() != UETT_SizeOf) - return;

-
-alpha.core.SizeofPtr -(C)
-Warn about unintended use of sizeof() on pointer -expressions.
 -
-
-struct s {};

-int test(struct s *p) { - return sizeof(p); - // warn: sizeof(ptr) can produce an unexpected result -} -

steakhal

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should remember to mention this transfer explicitly in the release notes one day.
But given that we don't really maintain/sync the release notes that's for another day, closer to the release branchoff.

@NagyDonat

We should remember to mention this transfer explicitly in the release notes one day.

Yes, I'll try to remember it. By the way, it is already mentioned in the clang-tidy release notes (apparently there the release notes are kept in sync with active development), but it's definitely important to mention this on the analyzer side as well. (This was just an alpha checker, so I suppose that there weren't too many users, but still.)

Labels