[clang][analyzer] Remove checker 'alpha.core.CastSize'

[balazske]

No description provided.

[llvmbot]

@llvm/pr-subscribers-clang

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

Author: Balázs Kéri (balazske)

Changes

---

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

11 Files Affected:

• (modified) clang/docs/analyzer/checkers.rst (-12)
• (modified) clang/include/clang/StaticAnalyzer/Checkers/Checkers.td (-5)
• (modified) clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt (-1)
• (removed) clang/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp (-156)
• (removed) clang/test/Analysis/castsize.c (-26)
• (modified) clang/test/Analysis/malloc-annotations.c (-10)
• (modified) clang/test/Analysis/malloc-annotations.cpp (-1)
• (modified) clang/test/Analysis/malloc.c (-226)
• (modified) clang/test/Analysis/malloc.cpp (-4)
• (modified) clang/test/Analysis/misc-ps.m (-12)
• (modified) clang/test/Analysis/qt_malloc.cpp (+1-1)

diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index b2effadacf9f1..9350def672a6d 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -2932,18 +2932,6 @@ the locking/unlocking of ``mtx_t`` mutexes.
    mtx_lock(&mtx1); // warn: This lock has already been acquired
  }
 
-.. _alpha-core-CastSize:
-
-alpha.core.CastSize (C)
-"""""""""""""""""""""""
-Check when casting a malloc'ed type ``T``, whether the size is a multiple of the size of ``T``.
-
-.. code-block:: c
-
- void test() {
-   int *x = (int *) malloc(11); // warn
- }
-
 .. _alpha-core-CastToStruct:
 
 alpha.core.CastToStruct (C, C++)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 73f702de581d9..8905c62c01e49 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -257,11 +257,6 @@ def BoolAssignmentChecker : Checker<"BoolAssignment">,
   HelpText<"Warn about assigning non-{0,1} values to Boolean variables">,
   Documentation<HasDocumentation>;
 
-def CastSizeChecker : Checker<"CastSize">,
-  HelpText<"Check when casting a malloc'ed type T, whether the size is a "
-           "multiple of the size of T">,
-  Documentation<HasDocumentation>;
-
 def CastToStructChecker : Checker<"CastToStruct">,
   HelpText<"Check for cast from non-struct pointer to struct pointer">,
   Documentation<HasDocumentation>;
diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 78360418a8b81..5ec07686ed2fe 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -17,7 +17,6 @@ add_clang_library(clangStaticAnalyzerCheckers
   CStringChecker.cpp
   CStringSyntaxChecker.cpp
   CallAndMessageChecker.cpp
-  CastSizeChecker.cpp
   CastToStructChecker.cpp
   CastValueChecker.cpp
   CheckObjCDealloc.cpp
diff --git a/clang/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp
deleted file mode 100644
index 90c6537d71d9d..0000000000000
--- a/clang/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp
+++ /dev/null
@@ -1,156 +0,0 @@
-//=== CastSizeChecker.cpp ---------------------------------------*- 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
-//
-//===----------------------------------------------------------------------===//
-//
-// CastSizeChecker checks when casting a malloc'ed symbolic region to type T,
-// whether the size of the symbolic region is a multiple of the size of T.
-//
-//===----------------------------------------------------------------------===//
-
-#include "clang/AST/CharUnits.h"
-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
-#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
-#include "clang/StaticAnalyzer/Core/Checker.h"
-#include "clang/StaticAnalyzer/Core/CheckerManager.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
-
-using namespace clang;
-using namespace ento;
-
-namespace {
-class CastSizeChecker : public Checker< check::PreStmt<CastExpr> > {
-  const BugType BT{this, "Cast region with wrong size."};
-
-public:
-  void checkPreStmt(const CastExpr *CE, CheckerContext &C) const;
-};
-}
-
-/// Check if we are casting to a struct with a flexible array at the end.
-/// \code
-/// struct foo {
-///   size_t len;
-///   struct bar data[];
-/// };
-/// \endcode
-/// or
-/// \code
-/// struct foo {
-///   size_t len;
-///   struct bar data[0];
-/// }
-/// \endcode
-/// In these cases it is also valid to allocate size of struct foo + a multiple
-/// of struct bar.
-static bool evenFlexibleArraySize(ASTContext &Ctx, CharUnits RegionSize,
-                                  CharUnits TypeSize, QualType ToPointeeTy) {
-  const RecordType *RT = ToPointeeTy->getAs<RecordType>();
-  if (!RT)
-    return false;
-
-  const RecordDecl *RD = RT->getOriginalDecl()->getDefinitionOrSelf();
-  RecordDecl::field_iterator Iter(RD->field_begin());
-  RecordDecl::field_iterator End(RD->field_end());
-  const FieldDecl *Last = nullptr;
-  for (; Iter != End; ++Iter)
-    Last = *Iter;
-  assert(Last && "empty structs should already be handled");
-
-  const Type *ElemType = Last->getType()->getArrayElementTypeNoTypeQual();
-  if (!ElemType)
-    return false;
-  CharUnits FlexSize;
-  if (const ConstantArrayType *ArrayTy =
-        Ctx.getAsConstantArrayType(Last->getType())) {
-    FlexSize = Ctx.getTypeSizeInChars(ElemType);
-    if (ArrayTy->getSize() == 1 && TypeSize > FlexSize)
-      TypeSize -= FlexSize;
-    else if (!ArrayTy->isZeroSize())
-      return false;
-  } else if (RD->hasFlexibleArrayMember()) {
-    FlexSize = Ctx.getTypeSizeInChars(ElemType);
-  } else {
-    return false;
-  }
-
-  if (FlexSize.isZero())
-    return false;
-
-  CharUnits Left = RegionSize - TypeSize;
-  if (Left.isNegative())
-    return false;
-
-  return Left % FlexSize == 0;
-}
-
-void CastSizeChecker::checkPreStmt(const CastExpr *CE,CheckerContext &C) const {
-  const Expr *E = CE->getSubExpr();
-  ASTContext &Ctx = C.getASTContext();
-  QualType ToTy = Ctx.getCanonicalType(CE->getType());
-  const PointerType *ToPTy = dyn_cast<PointerType>(ToTy.getTypePtr());
-
-  if (!ToPTy)
-    return;
-
-  QualType ToPointeeTy = ToPTy->getPointeeType();
-
-  // Only perform the check if 'ToPointeeTy' is a complete type.
-  if (ToPointeeTy->isIncompleteType())
-    return;
-
-  ProgramStateRef state = C.getState();
-  const MemRegion *R = C.getSVal(E).getAsRegion();
-  if (!R)
-    return;
-
-  const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(R);
-  if (!SR)
-    return;
-
-  SValBuilder &svalBuilder = C.getSValBuilder();
-
-  DefinedOrUnknownSVal Size = getDynamicExtent(state, SR, svalBuilder);
-  const llvm::APSInt *SizeInt = svalBuilder.getKnownValue(state, Size);
-  if (!SizeInt)
-    return;
-
-  CharUnits regionSize = CharUnits::fromQuantity(SizeInt->getZExtValue());
-  CharUnits typeSize = C.getASTContext().getTypeSizeInChars(ToPointeeTy);
-
-  // Ignore void, and a few other un-sizeable types.
-  if (typeSize.isZero())
-    return;
-
-  if (regionSize % typeSize == 0)
-    return;
-
-  if (evenFlexibleArraySize(Ctx, regionSize, typeSize, ToPointeeTy))
-    return;
-
-  if (ExplodedNode *errorNode = C.generateErrorNode()) {
-    constexpr llvm::StringLiteral Msg =
-        "Cast a region whose size is not a multiple of the destination type "
-        "size.";
-    auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, errorNode);
-    R->addRange(CE->getSourceRange());
-    C.emitReport(std::move(R));
-  }
-}
-
-void ento::registerCastSizeChecker(CheckerManager &mgr) {
-  mgr.registerChecker<CastSizeChecker>();
-}
-
-bool ento::shouldRegisterCastSizeChecker(const CheckerManager &mgr) {
-  // PR31226: C++ is more complicated than what this checker currently supports.
-  // There are derived-to-base casts, there are different rules for 0-size
-  // structures, no flexible arrays, etc.
-  // FIXME: Disabled on C++ for now.
-  const LangOptions &LO = mgr.getLangOpts();
-  return !LO.CPlusPlus;
-}
diff --git a/clang/test/Analysis/castsize.c b/clang/test/Analysis/castsize.c
deleted file mode 100644
index 81aa60c0414cd..0000000000000
--- a/clang/test/Analysis/castsize.c
+++ /dev/null
@@ -1,26 +0,0 @@
-// RUN: %clang_analyze_cc1 -verify %s \
-// RUN:   -analyzer-checker=core,unix.Malloc,alpha.core.CastSize
-
-typedef typeof(sizeof(int)) size_t;
-void *malloc(size_t);
-
-struct s1 {
-  int a;
-  char x[];
-};
-
-struct s2 {
-  int a[100];
-  char x[];
-};
-
-union u {
-  struct s1 a;
-  struct s2 b;
-};
-
-static union u *test() {
-  union u *req;
-  req = malloc(5); // expected-warning{{Cast a region whose size is not a multiple of the destination type size}}
-  return req;
-}
diff --git a/clang/test/Analysis/malloc-annotations.c b/clang/test/Analysis/malloc-annotations.c
index 68ac71d51e45c..d25f96d4ae1cd 100644
--- a/clang/test/Analysis/malloc-annotations.c
+++ b/clang/test/Analysis/malloc-annotations.c
@@ -1,7 +1,6 @@
 // RUN: %clang_analyze_cc1 -verify \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=alpha.deadcode.UnreachableCode \
-// RUN:   -analyzer-checker=alpha.core.CastSize \
 // RUN:   -analyzer-checker=unix.Malloc \
 // RUN:   -analyzer-checker=debug.ExprInspection \
 // RUN:   -analyzer-config unix.DynamicMemoryModeling:Optimistic=true %s
@@ -220,15 +219,6 @@ void f7_realloc(void) {
   x[0] = 'a'; // expected-warning{{Use of memory after it is released}}
 }
 
-void PR6123(void) {
-  int *x = malloc(11); // expected-warning{{Cast a region whose size is not a multiple of the destination type size}}
-}
-
-void PR7217(void) {
-  int *buf = malloc(2); // expected-warning{{Cast a region whose size is not a multiple of the destination type size}}
-  buf[1] = 'c'; // not crash
-}
-
 void mallocCastToVoid(void) {
   void *p = malloc(2);
   const void *cp = p; // not crash
diff --git a/clang/test/Analysis/malloc-annotations.cpp b/clang/test/Analysis/malloc-annotations.cpp
index 67a069d583efb..97fde387a2810 100644
--- a/clang/test/Analysis/malloc-annotations.cpp
+++ b/clang/test/Analysis/malloc-annotations.cpp
@@ -1,7 +1,6 @@
 // RUN: %clang_analyze_cc1 -verify \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=alpha.deadcode.UnreachableCode \
-// RUN:   -analyzer-checker=alpha.core.CastSize \
 // RUN:   -analyzer-checker=unix.Malloc \
 // RUN:   -analyzer-config unix.DynamicMemoryModeling:Optimistic=true %s
 
diff --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c
index 82eb364a78f96..65d92e9d136b6 100644
--- a/clang/test/Analysis/malloc.c
+++ b/clang/test/Analysis/malloc.c
@@ -1,7 +1,6 @@
 // RUN: %clang_analyze_cc1 -Wno-strict-prototypes -Wno-error=implicit-int -verify %s \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=alpha.deadcode.UnreachableCode \
-// RUN:   -analyzer-checker=alpha.core.CastSize \
 // RUN:   -analyzer-checker=unix \
 // RUN:   -analyzer-checker=debug.ExprInspection \
 // RUN:   -analyzer-checker=optin.taint.TaintPropagation \
@@ -495,231 +494,6 @@ void f7_realloc(void) {
   x[0] = 'a'; // expected-warning{{Use of memory after it is released}}
 }
 
-void PR6123(void) {
-  int *x = malloc(11); // expected-warning{{Cast a region whose size is not a multiple of the destination type size}}
-}
-
-void PR7217(void) {
-  int *buf = malloc(2); // expected-warning{{Cast a region whose size is not a multiple of the destination type size}}
-  buf[1] = 'c'; // not crash
-}
-
-void cast_emtpy_struct(void) {
-  struct st {
-  };
-
-  struct st *s = malloc(sizeof(struct st)); // no-warning
-  free(s);
-}
-
-void cast_struct_1(void) {
-  struct st {
-    int i[100];
-    char j[];
-  };
-
-  struct st *s = malloc(sizeof(struct st)); // no-warning
-  free(s);
-}
-
-void cast_struct_2(void) {
-  struct st {
-    int i[100];
-    char j[0];
-  };
-
-  struct st *s = malloc(sizeof(struct st)); // no-warning
-  free(s);
-}
-
-void cast_struct_3(void) {
-  struct st {
-    int i[100];
-    char j[1];
-  };
-
-  struct st *s = malloc(sizeof(struct st)); // no-warning
-  free(s);
-}
-
-void cast_struct_4(void) {
-  struct st {
-    int i[100];
-    char j[2];
-  };
-
-  struct st *s = malloc(sizeof(struct st)); // no-warning
-  free(s);
-}
-
-void cast_struct_5(void) {
-  struct st {
-    char i[200];
-    char j[1];
-  };
-
-  struct st *s = malloc(sizeof(struct st) - sizeof(char)); // no-warning
-  free(s);
-}
-
-void cast_struct_warn_1(void) {
-  struct st {
-    int i[100];
-    char j[2];
-  };
-
-  struct st *s = malloc(sizeof(struct st) + 2); // expected-warning{{Cast a region whose size is not a multiple of the destination type size}}
-  free(s);
-}
-
-void cast_struct_warn_2(void) {
-  struct st {
-    int i[100];
-    char j[2];
-  };
-
-  struct st *s = malloc(2); // expected-warning{{Cast a region whose size is not a multiple of the destination type size}}
-  free(s);
-}
-
-void cast_struct_flex_array_1(void) {
-  struct st {
-    int i[100];
-    char j[];
-  };
-
-  struct st *s = malloc(sizeof(struct st) + 3); // no-warning
-  free(s);
-}
-
-void cast_struct_flex_array_2(void) {
-  struct st {
-    int i[100];
-    char j[0];
-  };
-
-  struct st *s = malloc(sizeof(struct st) + 3); // no-warning
-  free(s);
-}
-
-void cast_struct_flex_array_3(void) {
-  struct st {
-    int i[100];
-    char j[1];
-  };
-
-  struct st *s = malloc(sizeof(struct st) + 3); // no-warning
-  free(s);
-}
-
-void cast_struct_flex_array_4(void) {
-  struct foo {
-    char f[32];
-  };
-  struct st {
-    char i[100];
-    struct foo data[];
-  };
-
-  struct st *s = malloc(sizeof(struct st) + 3 * sizeof(struct foo)); // no-warning
-  free(s);
-}
-
-void cast_struct_flex_array_5(void) {
-  struct foo {
-    char f[32];
-  };
-  struct st {
-    char i[100];
-    struct foo data[0];
-  };
-
-  struct st *s = malloc(sizeof(struct st) + 3 * sizeof(struct foo)); // no-warning
-  free(s);
-}
-
-void cast_struct_flex_array_6(void) {
-  struct foo {
-    char f[32];
-  };
-  struct st {
-    char i[100];
-    struct foo data[1];
-  };
-
-  struct st *s = malloc(sizeof(struct st) + 3 * sizeof(struct foo)); // no-warning
-  free(s);
-}
-
-void cast_struct_flex_array_warn_1(void) {
-  struct foo {
-    char f[32];
-  };
-  struct st {
-    char i[100];
-    struct foo data[];
-  };
-
-  struct st *s = malloc(3 * sizeof(struct st) + 3 * sizeof(struct foo)); // expected-warning{{Cast a region whose size is not a multiple of the destination type size}}
-  free(s);
-}
-
-void cast_struct_flex_array_warn_2(void) {
-  struct foo {
-    char f[32];
-  };
-  struct st {
-    char i[100];
-    struct foo data[0];
-  };
-
-  struct st *s = malloc(3 * sizeof(struct st) + 3 * sizeof(struct foo)); // expected-warning{{Cast a region whose size is not a multiple of the destination type size}}
-  free(s);
-}
-
-void cast_struct_flex_array_warn_3(void) {
-  struct foo {
-    char f[32];
-  };
-  struct st {
-    char i[100];
-    struct foo data[1];
-  };
-
-  struct st *s = malloc(3 * sizeof(struct st) + 3 * sizeof(struct foo)); // expected-warning{{Cast a region whose size is not a multiple of the destination type size}}
-  free(s);
-}
-
-void cast_struct_flex_array_warn_4(void) {
-  struct st {
-    int i[100];
-    int j[];
-  };
-
-  struct st *s = malloc(sizeof(struct st) + 3); // expected-warning{{Cast a region whose size is not a multiple of the destination type size}}
-  free(s);
-}
-
-void cast_struct_flex_array_warn_5(void) {
-  struct st {
-    int i[100];
-    int j[0];
-  };
-
-  struct st *s = malloc(sizeof(struct st) + 3); // expected-warning{{Cast a region whose size is not a multiple of the destination type size}}
-  free(s);
-}
-
-void cast_struct_flex_array_warn_6(void) {
-  struct st {
-    int i[100];
-    int j[1];
-  };
-
-  struct st *s = malloc(sizeof(struct st) + 3); // expected-warning{{Cast a region whose size is not a multiple of the destination type size}}
-  free(s);
-}
-
 void mallocCastToVoid(void) {
   void *p = malloc(2);
   const void *cp = p; // not crash
diff --git a/clang/test/Analysis/malloc.cpp b/clang/test/Analysis/malloc.cpp
index 2bbfaf6640b79..8025973dc73c1 100644
--- a/clang/test/Analysis/malloc.cpp
+++ b/clang/test/Analysis/malloc.cpp
@@ -1,7 +1,6 @@
 // RUN: %clang_analyze_cc1 -w -verify %s \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=alpha.deadcode.UnreachableCode \
-// RUN:   -analyzer-checker=alpha.core.CastSize \
 // RUN:   -analyzer-checker=unix.Malloc \
 // RUN:   -analyzer-checker=cplusplus.NewDelete \
 // RUN:   -analyzer-checker=optin.taint.TaintPropagation \
@@ -11,7 +10,6 @@
 // RUN:   -triple i386-unknown-linux-gnu \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=alpha.deadcode.UnreachableCode \
-// RUN:   -analyzer-checker=alpha.core.CastSize \
 // RUN:   -analyzer-checker=unix.Malloc \
 // RUN:   -analyzer-checker=cplusplus.NewDelete \
 // RUN:   -analyzer-checker=optin.taint.TaintPropagation \
@@ -20,7 +18,6 @@
 // RUN: %clang_analyze_cc1 -w -verify %s -DTEST_INLINABLE_ALLOCATORS \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=alpha.deadcode.UnreachableCode \
-// RUN:   -analyzer-checker=alpha.core.CastSize \
 // RUN:   -analyzer-checker=unix.Malloc \
 // RUN:   -analyzer-checker=cplusplus.NewDelete \
 // RUN:   -analyzer-checker=optin.taint.TaintPropagation \
@@ -30,7 +27,6 @@
 // RUN:   -triple i386-unknown-linux-gnu \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=alpha.deadcode.UnreachableCode \
-// RUN:   -analyzer-checker=alpha.core.CastSize \
 // RUN:   -analyzer-checker=unix.Malloc \
 // RUN:   -analyzer-checker=cplusplus.NewDelete \
 // RUN:   -analyzer-checker=optin.taint.TaintPropagation \
diff --git a/clang/test/Analysis/misc-ps.m b/clang/test/Analysis/misc-ps.m
index 9c19d0cb4556f..794d8bbceb459 100644
--- a/clang/test/Analysis/misc-ps.m
+++ b/clang/test/Analysis/misc-ps.m
@@ -1047,18 +1047,6 @@ void r8360854(int n) {
   *p = 0xDEADBEEF; // expected-warning{{null pointer}}
 }
 
-// PR 8050 - crash in CastSizeChecker when pointee is an incomplete type
-typedef long unsigned int __darwin_size_t;
-typedef __darwin_size_t size_t;
-void *malloc(size_t);
-
-struct PR8050;
-
-void pr8050(struct PR8050 **arg)
-{
-    *arg = malloc(1);
-}
-
 // Switch on enum should not consider default case live if all enum values are
 // covered.
 enum Cases { C1, C2, C3, C4 };
diff --git a/clang/test/Analysis/qt_malloc.cpp b/clang/test/Analysis/qt_malloc.cpp
index b55ea63271f3d..a9121b0a599be 100644
--- a/clang/test/Analysis/qt_malloc.cpp
+++ b/clang/test/Analysis/qt_malloc.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus -verify %s
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,alpha.deadcode.UnreachableCode,unix.Malloc,cplusplus -verify %s
 // expected-no-diagnostics
 #include "Inputs/qt-simulator.h"
 

[balazske]

According to https://discourse.llvm.org/t/remove-alpha-core-castsize/87974 this checker looks to be rarely used. In the current form it can have more false positives (I tested it on some C projects) than useful results. Additionally, the same problem could be found with other more reasonable checkers.

[NagyDonat]

I support the removal of this checker, because it implements a heuristic that finds and reports a very specific bug which is (as far as I see) not a "natural" mistake that would appear during real-world development.