[analyzer] Finish moving alpha.core.SizeofPtr to clang-tidy #95118
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
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
added
clang
|
@llvm/pr-subscribers-clang Author: Donát Nagy (NagyDonat) ChangesThe checker 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 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
-
- struct s {};
-
- int test(struct s *p) {
- return sizeof(p);
- // warn: sizeof(ptr) can produce an unexpected result
- }
-
.. _alpha-core-StackAddressAsyncEscape:
alpha.core.StackAddressAsyncEscape (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<HasDocumentation>;
-def SizeofPointerChecker : Checker<"SizeofPtr">,
- HelpText<"Warn about unintended use of sizeof() on pointer expressions">,
- Documentation<HasDocumentation>;
-
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<WalkAST> {
- 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;
-
- // If an explicit type is used in the code, usually the coder knows what they are
- // doing.
- if (E->isArgumentType())
- return;
-
- QualType T = E->getTypeOfArgument();
- if (T->isPointerType()) {
-
- // Many false positives have the form 'sizeof *p'. This is reasonable
- // because people know what they are doing when they intentionally
- // dereference the pointer.
- Expr *ArgEx = E->getArgumentExpr();
- if (!isa<DeclRefExpr>(ArgEx->IgnoreParens()))
- return;
-
- PathDiagnosticLocation ELoc =
- PathDiagnosticLocation::createBegin(E, BR.getSourceManager(), AC);
- BR.EmitBasicReport(AC->getDecl(), Checker,
- "Potential unintended use of sizeof() on pointer type",
- categories::LogicError,
- "The code calls sizeof() on a pointer type. "
- "This can produce an unexpected result.",
- ELoc, ArgEx->getSourceRange());
- }
-}
-
-//===----------------------------------------------------------------------===//
-// SizeofPointerChecker
-//===----------------------------------------------------------------------===//
-
-namespace {
-class SizeofPointerChecker : public Checker<check::ASTCodeBody> {
-public:
- void checkASTCodeBody(const Decl *D, AnalysisManager& mgr,
- BugReporter &BR) const {
- WalkAST walker(BR, this, mgr.getAnalysisDeclContext(D));
- walker.Visit(D->getBody());
- }
-};
-}
-
-void ento::registerSizeofPointerChecker(CheckerManager &mgr) {
- mgr.registerChecker<SizeofPointerChecker>();
-}
-
-bool ento::shouldRegisterSizeofPointerChecker(const CheckerManager &mgr) {
- return true;
-}
diff --git a/clang/test/Analysis/sizeofpointer.c b/clang/test/Analysis/sizeofpointer.c
deleted file mode 100644
index 14ddbd1a8b107..0000000000000
--- a/clang/test/Analysis/sizeofpointer.c
+++ /dev/null
@@ -1,8 +0,0 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.SizeofPtr -verify %s
-
-struct s {
-};
-
-int f(struct s *p) {
- return sizeof(p); // expected-warning{{The code calls sizeof() on a pointer type. This can produce an unexpected result}}
-}
diff --git a/clang/www/analyzer/alpha_checks.html b/clang/www/analyzer/alpha_checks.html
index 411baae695b93..501a9bcbc82a9 100644
--- a/clang/www/analyzer/alpha_checks.html
+++ b/clang/www/analyzer/alpha_checks.html
@@ -239,22 +239,6 @@ <h3 id="core_alpha_checkers">Core Alpha Checkers</h3>
</pre></div></div></td></tr>
-<tr><td><a id="alpha.core.SizeofPtr"><div class="namedescr expandable"><span class="name">
-alpha.core.SizeofPtr</span><span class="lang">
-(C)</span><div class="descr">
-Warn about unintended use of <code>sizeof()</code> on pointer
-expressions.</div></div></a></td>
-<td><div class="exampleContainer expandable">
-<div class="example"><pre>
-struct s {};
-
-int test(struct s *p) {
- return sizeof(p);
- // warn: sizeof(ptr) can produce an unexpected result
-}
-</pre></div></div></td></tr>
-
-
<tr><td><a id="alpha.core.StackAddressAsyncEscape"><div class="namedescr expandable"><span class="name">
alpha.core.StackAddressAsyncEscape</span><span class="lang">
(C)</span><div class="descr">
|
|
@llvm/pr-subscribers-clang-static-analyzer-1 Author: Donát Nagy (NagyDonat) ChangesThe checker 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 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
-
- struct s {};
-
- int test(struct s *p) {
- return sizeof(p);
- // warn: sizeof(ptr) can produce an unexpected result
- }
-
.. _alpha-core-StackAddressAsyncEscape:
alpha.core.StackAddressAsyncEscape (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<HasDocumentation>;
-def SizeofPointerChecker : Checker<"SizeofPtr">,
- HelpText<"Warn about unintended use of sizeof() on pointer expressions">,
- Documentation<HasDocumentation>;
-
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<WalkAST> {
- 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;
-
- // If an explicit type is used in the code, usually the coder knows what they are
- // doing.
- if (E->isArgumentType())
- return;
-
- QualType T = E->getTypeOfArgument();
- if (T->isPointerType()) {
-
- // Many false positives have the form 'sizeof *p'. This is reasonable
- // because people know what they are doing when they intentionally
- // dereference the pointer.
- Expr *ArgEx = E->getArgumentExpr();
- if (!isa<DeclRefExpr>(ArgEx->IgnoreParens()))
- return;
-
- PathDiagnosticLocation ELoc =
- PathDiagnosticLocation::createBegin(E, BR.getSourceManager(), AC);
- BR.EmitBasicReport(AC->getDecl(), Checker,
- "Potential unintended use of sizeof() on pointer type",
- categories::LogicError,
- "The code calls sizeof() on a pointer type. "
- "This can produce an unexpected result.",
- ELoc, ArgEx->getSourceRange());
- }
-}
-
-//===----------------------------------------------------------------------===//
-// SizeofPointerChecker
-//===----------------------------------------------------------------------===//
-
-namespace {
-class SizeofPointerChecker : public Checker<check::ASTCodeBody> {
-public:
- void checkASTCodeBody(const Decl *D, AnalysisManager& mgr,
- BugReporter &BR) const {
- WalkAST walker(BR, this, mgr.getAnalysisDeclContext(D));
- walker.Visit(D->getBody());
- }
-};
-}
-
-void ento::registerSizeofPointerChecker(CheckerManager &mgr) {
- mgr.registerChecker<SizeofPointerChecker>();
-}
-
-bool ento::shouldRegisterSizeofPointerChecker(const CheckerManager &mgr) {
- return true;
-}
diff --git a/clang/test/Analysis/sizeofpointer.c b/clang/test/Analysis/sizeofpointer.c
deleted file mode 100644
index 14ddbd1a8b107..0000000000000
--- a/clang/test/Analysis/sizeofpointer.c
+++ /dev/null
@@ -1,8 +0,0 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.SizeofPtr -verify %s
-
-struct s {
-};
-
-int f(struct s *p) {
- return sizeof(p); // expected-warning{{The code calls sizeof() on a pointer type. This can produce an unexpected result}}
-}
diff --git a/clang/www/analyzer/alpha_checks.html b/clang/www/analyzer/alpha_checks.html
index 411baae695b93..501a9bcbc82a9 100644
--- a/clang/www/analyzer/alpha_checks.html
+++ b/clang/www/analyzer/alpha_checks.html
@@ -239,22 +239,6 @@ <h3 id="core_alpha_checkers">Core Alpha Checkers</h3>
</pre></div></div></td></tr>
-<tr><td><a id="alpha.core.SizeofPtr"><div class="namedescr expandable"><span class="name">
-alpha.core.SizeofPtr</span><span class="lang">
-(C)</span><div class="descr">
-Warn about unintended use of <code>sizeof()</code> on pointer
-expressions.</div></div></a></td>
-<td><div class="exampleContainer expandable">
-<div class="example"><pre>
-struct s {};
-
-int test(struct s *p) {
- return sizeof(p);
- // warn: sizeof(ptr) can produce an unexpected result
-}
-</pre></div></div></td></tr>
-
-
<tr><td><a id="alpha.core.StackAddressAsyncEscape"><div class="namedescr expandable"><span class="name">
alpha.core.StackAddressAsyncEscape</span><span class="lang">
(C)</span><div class="descr">
|
steakhal
approved these changes
Jun 11, 2024
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.) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The checker
alpha.core.SizeofPtrwas a very simple checker that did not rely on path sensitive analysis and was very similar to the (more complex and refined) clang-tidy checkbugprone-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.SizeofPtrwithin clang-tidy; now this commit finishes the transfer by deletingalpha.core.SizeofPtr.