https://github.com/NagyDonat updated https://github.com/llvm/llvm-project/pull/103059
From 36821708145587553f13df8648920f281b318240 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com> Date: Tue, 13 Aug 2024 14:50:17 +0200 Subject: [PATCH 1/2] [analyzer] Delete alpha.security.MallocOverflow ...because it is too noisy to be useful right now, and its architecture is terrible, so it can't act a starting point of future development. The main problem with this checker is that it tries to do (or at least fake) path-sensitive analysis without actually using the established path-sensitive analysis engine. Instead of actually tracking the symbolic values and the known constraints on them, this checker blindly gropes the AST and uses heuristics like "this variable was seen in a comparison operator expression that is not a loop condition, so it's probably not too large" (which was improved in a separate commit to at least ignore comparison operators that appear after the actual `malloc()` call). This might have been acceptable in 2011 (when this checker was added), but since then we developed a significantly better standard approach for analysis and this old relic doesn't deserve to remain in the codebase. Needless to say, this primitive approach causes lots of false positives (and presumably false negatives as well), which ensures that this alpha checker won't be missed by the users. It would be good to eventually have a stable, path-sensitive checker that could succeed in the task where this hacky implementation fails, but the first step towards that goal is taking out this old garbage, which confuses the potential users or contributors. (I don't have plans for reimplementing the goals of this checker. It could happen eventually, but right now I have many plans that are higher priority than this.) --- clang/docs/analyzer/checkers.rst | 43 --- .../clang/StaticAnalyzer/Checkers/Checkers.td | 4 - .../StaticAnalyzer/Checkers/CMakeLists.txt | 1 - .../MallocOverflowSecurityChecker.cpp | 341 ------------------ clang/test/Analysis/malloc-overflow.c | 150 -------- clang/test/Analysis/malloc-overflow.cpp | 12 - clang/test/Analysis/malloc-overflow2.c | 40 -- clang/www/analyzer/potential_checkers.html | 2 - .../lib/StaticAnalyzer/Checkers/BUILD.gn | 1 - 9 files changed, 594 deletions(-) delete mode 100644 clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp delete mode 100644 clang/test/Analysis/malloc-overflow.c delete mode 100644 clang/test/Analysis/malloc-overflow.cpp delete mode 100644 clang/test/Analysis/malloc-overflow2.c diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 46b0b7b9c82376..0bfbc995579d41 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -2951,49 +2951,6 @@ Warn about buffer overflows (newer checker). char c = s[x]; // warn: index is tainted } -.. _alpha-security-MallocOverflow: - -alpha.security.MallocOverflow (C) -""""""""""""""""""""""""""""""""" -Check for overflows in the arguments to ``malloc()``. -It tries to catch ``malloc(n * c)`` patterns, where: - - - ``n``: a variable or member access of an object - - ``c``: a constant foldable integral - -This checker was designed for code audits, so expect false-positive reports. -One is supposed to silence this checker by ensuring proper bounds checking on -the variable in question using e.g. an ``assert()`` or a branch. - -.. code-block:: c - - void test(int n) { - void *p = malloc(n * sizeof(int)); // warn - } - - void test2(int n) { - if (n > 100) // gives an upper-bound - return; - void *p = malloc(n * sizeof(int)); // no warning - } - - void test3(int n) { - assert(n <= 100 && "Contract violated."); - void *p = malloc(n * sizeof(int)); // no warning - } - -Limitations: - - - The checker won't warn for variables involved in explicit casts, - since that might limit the variable's domain. - E.g.: ``(unsigned char)int x`` would limit the domain to ``[0,255]``. - The checker will miss the true-positive cases when the explicit cast would - not tighten the domain to prevent the overflow in the subsequent - multiplication operation. - - - It is an AST-based checker, thus it does not make use of the - path-sensitive taint-analysis. - .. _alpha-security-MmapWriteExec: alpha.security.MmapWriteExec (C) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 38b55a0eb0a7b0..fb4114619ac3d3 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -1039,10 +1039,6 @@ def ArrayBoundCheckerV2 : Checker<"ArrayBoundV2">, HelpText<"Warn about buffer overflows (newer checker)">, Documentation<HasDocumentation>; -def MallocOverflowSecurityChecker : Checker<"MallocOverflow">, - HelpText<"Check for overflows in the arguments to malloc()">, - Documentation<HasDocumentation>; - def MmapWriteExecChecker : Checker<"MmapWriteExec">, HelpText<"Warn on mmap() calls that are both writable and executable">, Documentation<HasDocumentation>; diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt index 682cfa01bec963..414282d58f779f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -63,7 +63,6 @@ add_clang_library(clangStaticAnalyzerCheckers MacOSKeychainAPIChecker.cpp MacOSXAPIChecker.cpp MallocChecker.cpp - MallocOverflowSecurityChecker.cpp MallocSizeofChecker.cpp MismatchedIteratorChecker.cpp MmapWriteExecChecker.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp deleted file mode 100644 index 3c8b38973c6b8c..00000000000000 --- a/clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp +++ /dev/null @@ -1,341 +0,0 @@ -// MallocOverflowSecurityChecker.cpp - Check for malloc overflows -*- 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 checker detects a common memory allocation security flaw. -// Suppose 'unsigned int n' comes from an untrusted source. If the -// code looks like 'malloc (n * 4)', and an attacker can make 'n' be -// say MAX_UINT/4+2, then instead of allocating the correct 'n' 4-byte -// elements, this will actually allocate only two because of overflow. -// Then when the rest of the program attempts to store values past the -// second element, these values will actually overwrite other items in -// the heap, probably allowing the attacker to execute arbitrary code. -// -//===----------------------------------------------------------------------===// - -#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" -#include "clang/AST/EvaluatedExprVisitor.h" -#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" -#include "clang/StaticAnalyzer/Core/Checker.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" -#include "llvm/ADT/APSInt.h" -#include "llvm/ADT/SmallVector.h" -#include <optional> -#include <utility> - -using namespace clang; -using namespace ento; -using llvm::APSInt; - -namespace { -struct MallocOverflowCheck { - const CallExpr *call; - const BinaryOperator *mulop; - const Expr *variable; - APSInt maxVal; - - MallocOverflowCheck(const CallExpr *call, const BinaryOperator *m, - const Expr *v, APSInt val) - : call(call), mulop(m), variable(v), maxVal(std::move(val)) {} -}; - -class MallocOverflowSecurityChecker : public Checker<check::ASTCodeBody> { -public: - void checkASTCodeBody(const Decl *D, AnalysisManager &mgr, - BugReporter &BR) const; - - void CheckMallocArgument( - SmallVectorImpl<MallocOverflowCheck> &PossibleMallocOverflows, - const CallExpr *TheCall, ASTContext &Context) const; - - void OutputPossibleOverflows( - SmallVectorImpl<MallocOverflowCheck> &PossibleMallocOverflows, - const Decl *D, BugReporter &BR, AnalysisManager &mgr) const; - -}; -} // end anonymous namespace - -// Return true for computations which evaluate to zero: e.g., mult by 0. -static inline bool EvaluatesToZero(APSInt &Val, BinaryOperatorKind op) { - return (op == BO_Mul) && (Val == 0); -} - -void MallocOverflowSecurityChecker::CheckMallocArgument( - SmallVectorImpl<MallocOverflowCheck> &PossibleMallocOverflows, - const CallExpr *TheCall, ASTContext &Context) const { - - /* Look for a linear combination with a single variable, and at least - one multiplication. - Reject anything that applies to the variable: an explicit cast, - conditional expression, an operation that could reduce the range - of the result, or anything too complicated :-). */ - const Expr *e = TheCall->getArg(0); - const BinaryOperator * mulop = nullptr; - APSInt maxVal; - - for (;;) { - maxVal = 0; - e = e->IgnoreParenImpCasts(); - if (const BinaryOperator *binop = dyn_cast<BinaryOperator>(e)) { - BinaryOperatorKind opc = binop->getOpcode(); - // TODO: ignore multiplications by 1, reject if multiplied by 0. - if (mulop == nullptr && opc == BO_Mul) - mulop = binop; - if (opc != BO_Mul && opc != BO_Add && opc != BO_Sub && opc != BO_Shl) - return; - - const Expr *lhs = binop->getLHS(); - const Expr *rhs = binop->getRHS(); - if (rhs->isEvaluatable(Context)) { - e = lhs; - maxVal = rhs->EvaluateKnownConstInt(Context); - if (EvaluatesToZero(maxVal, opc)) - return; - } else if ((opc == BO_Add || opc == BO_Mul) && - lhs->isEvaluatable(Context)) { - maxVal = lhs->EvaluateKnownConstInt(Context); - if (EvaluatesToZero(maxVal, opc)) - return; - e = rhs; - } else - return; - } else if (isa<DeclRefExpr, MemberExpr>(e)) - break; - else - return; - } - - if (mulop == nullptr) - return; - - // We've found the right structure of malloc argument, now save - // the data so when the body of the function is completely available - // we can check for comparisons. - - PossibleMallocOverflows.push_back( - MallocOverflowCheck(TheCall, mulop, e, maxVal)); -} - -namespace { -// A worker class for OutputPossibleOverflows. -class CheckOverflowOps : - public EvaluatedExprVisitor<CheckOverflowOps> { -public: - typedef SmallVectorImpl<MallocOverflowCheck> theVecType; - -private: - theVecType &toScanFor; - ASTContext &Context; - - bool isIntZeroExpr(const Expr *E) const { - if (!E->getType()->isIntegralOrEnumerationType()) - return false; - Expr::EvalResult Result; - if (E->EvaluateAsInt(Result, Context)) - return Result.Val.getInt() == 0; - return false; - } - - static const Decl *getDecl(const DeclRefExpr *DR) { return DR->getDecl(); } - static const Decl *getDecl(const MemberExpr *ME) { - return ME->getMemberDecl(); - } - - template <typename T1> - void Erase(const T1 *DR, - llvm::function_ref<bool(const MallocOverflowCheck &)> Pred) { - auto P = [DR, Pred](const MallocOverflowCheck &Check) { - if (const auto *CheckDR = dyn_cast<T1>(Check.variable)) - return getDecl(CheckDR) == getDecl(DR) && Pred(Check); - return false; - }; - llvm::erase_if(toScanFor, P); - } - - void CheckExpr(const Expr *E_p) { - const Expr *E = E_p->IgnoreParenImpCasts(); - const auto PrecedesMalloc = [E, this](const MallocOverflowCheck &c) { - return Context.getSourceManager().isBeforeInTranslationUnit( - E->getExprLoc(), c.call->getExprLoc()); - }; - if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(E)) - Erase<DeclRefExpr>(DR, PrecedesMalloc); - else if (const auto *ME = dyn_cast<MemberExpr>(E)) { - Erase<MemberExpr>(ME, PrecedesMalloc); - } - } - - // Check if the argument to malloc is assigned a value - // which cannot cause an overflow. - // e.g., malloc (mul * x) and, - // case 1: mul = <constant value> - // case 2: mul = a/b, where b > x - void CheckAssignmentExpr(BinaryOperator *AssignEx) { - bool assignKnown = false; - bool numeratorKnown = false, denomKnown = false; - APSInt denomVal; - denomVal = 0; - - // Erase if the multiplicand was assigned a constant value. - const Expr *rhs = AssignEx->getRHS(); - if (rhs->isEvaluatable(Context)) - assignKnown = true; - - // Discard the report if the multiplicand was assigned a value, - // that can never overflow after multiplication. e.g., the assignment - // is a division operator and the denominator is > other multiplicand. - const Expr *rhse = rhs->IgnoreParenImpCasts(); - if (const BinaryOperator *BOp = dyn_cast<BinaryOperator>(rhse)) { - if (BOp->getOpcode() == BO_Div) { - const Expr *denom = BOp->getRHS()->IgnoreParenImpCasts(); - Expr::EvalResult Result; - if (denom->EvaluateAsInt(Result, Context)) { - denomVal = Result.Val.getInt(); - denomKnown = true; - } - const Expr *numerator = BOp->getLHS()->IgnoreParenImpCasts(); - if (numerator->isEvaluatable(Context)) - numeratorKnown = true; - } - } - if (!assignKnown && !denomKnown) - return; - auto denomExtVal = denomVal.getExtValue(); - - // Ignore negative denominator. - if (denomExtVal < 0) - return; - - const Expr *lhs = AssignEx->getLHS(); - const Expr *E = lhs->IgnoreParenImpCasts(); - - auto pred = [assignKnown, numeratorKnown, - denomExtVal](const MallocOverflowCheck &Check) { - return assignKnown || - (numeratorKnown && (denomExtVal >= Check.maxVal.getExtValue())); - }; - - if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(E)) - Erase<DeclRefExpr>(DR, pred); - else if (const auto *ME = dyn_cast<MemberExpr>(E)) - Erase<MemberExpr>(ME, pred); - } - - public: - void VisitBinaryOperator(BinaryOperator *E) { - if (E->isComparisonOp()) { - const Expr * lhs = E->getLHS(); - const Expr * rhs = E->getRHS(); - // Ignore comparisons against zero, since they generally don't - // protect against an overflow. - if (!isIntZeroExpr(lhs) && !isIntZeroExpr(rhs)) { - CheckExpr(lhs); - CheckExpr(rhs); - } - } - if (E->isAssignmentOp()) - CheckAssignmentExpr(E); - EvaluatedExprVisitor<CheckOverflowOps>::VisitBinaryOperator(E); - } - - /* We specifically ignore loop conditions, because they're typically - not error checks. */ - void VisitWhileStmt(WhileStmt *S) { - return this->Visit(S->getBody()); - } - void VisitForStmt(ForStmt *S) { - return this->Visit(S->getBody()); - } - void VisitDoStmt(DoStmt *S) { - return this->Visit(S->getBody()); - } - - CheckOverflowOps(theVecType &v, ASTContext &ctx) - : EvaluatedExprVisitor<CheckOverflowOps>(ctx), - toScanFor(v), Context(ctx) - { } - }; -} - -// OutputPossibleOverflows - We've found a possible overflow earlier, -// now check whether Body might contain a comparison which might be -// preventing the overflow. -// This doesn't do flow analysis, range analysis, or points-to analysis; it's -// just a dumb "is there a comparison" scan. The aim here is to -// detect the most blatent cases of overflow and educate the -// programmer. -void MallocOverflowSecurityChecker::OutputPossibleOverflows( - SmallVectorImpl<MallocOverflowCheck> &PossibleMallocOverflows, - const Decl *D, BugReporter &BR, AnalysisManager &mgr) const { - // By far the most common case: nothing to check. - if (PossibleMallocOverflows.empty()) - return; - - // Delete any possible overflows which have a comparison. - CheckOverflowOps c(PossibleMallocOverflows, BR.getContext()); - c.Visit(mgr.getAnalysisDeclContext(D)->getBody()); - - // Output warnings for all overflows that are left. - for (const MallocOverflowCheck &Check : PossibleMallocOverflows) { - BR.EmitBasicReport( - D, this, "malloc() size overflow", categories::UnixAPI, - "the computation of the size of the memory allocation may overflow", - PathDiagnosticLocation::createOperatorLoc(Check.mulop, - BR.getSourceManager()), - Check.mulop->getSourceRange()); - } -} - -void MallocOverflowSecurityChecker::checkASTCodeBody(const Decl *D, - AnalysisManager &mgr, - BugReporter &BR) const { - - CFG *cfg = mgr.getCFG(D); - if (!cfg) - return; - - // A list of variables referenced in possibly overflowing malloc operands. - SmallVector<MallocOverflowCheck, 2> PossibleMallocOverflows; - - for (CFG::iterator it = cfg->begin(), ei = cfg->end(); it != ei; ++it) { - CFGBlock *block = *it; - for (CFGBlock::iterator bi = block->begin(), be = block->end(); - bi != be; ++bi) { - if (std::optional<CFGStmt> CS = bi->getAs<CFGStmt>()) { - if (const CallExpr *TheCall = dyn_cast<CallExpr>(CS->getStmt())) { - // Get the callee. - const FunctionDecl *FD = TheCall->getDirectCallee(); - - if (!FD) - continue; - - // Get the name of the callee. If it's a builtin, strip off the - // prefix. - IdentifierInfo *FnInfo = FD->getIdentifier(); - if (!FnInfo) - continue; - - if (FnInfo->isStr("malloc") || FnInfo->isStr("_MALLOC")) { - if (TheCall->getNumArgs() == 1) - CheckMallocArgument(PossibleMallocOverflows, TheCall, - mgr.getASTContext()); - } - } - } - } - } - - OutputPossibleOverflows(PossibleMallocOverflows, D, BR, mgr); -} - -void ento::registerMallocOverflowSecurityChecker(CheckerManager &mgr) { - mgr.registerChecker<MallocOverflowSecurityChecker>(); -} - -bool ento::shouldRegisterMallocOverflowSecurityChecker(const CheckerManager &mgr) { - return true; -} diff --git a/clang/test/Analysis/malloc-overflow.c b/clang/test/Analysis/malloc-overflow.c deleted file mode 100644 index 03fe15bccb62ee..00000000000000 --- a/clang/test/Analysis/malloc-overflow.c +++ /dev/null @@ -1,150 +0,0 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.security.MallocOverflow -verify %s - -#define NULL ((void *) 0) -typedef __typeof__(sizeof(int)) size_t; -extern void * malloc(size_t); - -void * f1(int n) -{ - return malloc(n * sizeof(int)); // expected-warning {{the computation of the size of the memory allocation may overflow}} -} - -void * f2(int n) -{ - return malloc(sizeof(int) * n); // // expected-warning {{the computation of the size of the memory allocation may overflow}} -} - -void * f3(void) -{ - return malloc(4 * sizeof(int)); // no-warning -} - -struct s4 -{ - int n; -}; - -void * f4(struct s4 *s) -{ - return malloc(s->n * sizeof(int)); // expected-warning {{the computation of the size of the memory allocation may overflow}} -} - -void * f5(struct s4 *s) -{ - struct s4 s2 = *s; - return malloc(s2.n * sizeof(int)); // expected-warning {{the computation of the size of the memory allocation may overflow}} -} - -void * f6(int n) -{ - return malloc((n + 1) * sizeof(int)); // expected-warning {{the computation of the size of the memory allocation may overflow}} -} - -extern void * malloc (size_t); - -void * f7(int n) -{ - if (n > 10) - return NULL; - return malloc(n * sizeof(int)); // no-warning -} - -void * f8(int n) -{ - if (n < 10) - return malloc(n * sizeof(int)); // no-warning - else - return NULL; -} - -void * f9(int n) -{ - int * x = malloc(n * sizeof(int)); // expected-warning {{the computation of the size of the memory allocation may overflow}} - for (int i = 0; i < n; i++) - x[i] = i; - return x; -} - -void * f10(int n) -{ - int * x = malloc(n * sizeof(int)); // expected-warning {{the computation of the size of the memory allocation may overflow}} - int i = 0; - while (i < n) - x[i++] = 0; - return x; -} - -void * f11(int n) -{ - int * x = malloc(n * sizeof(int)); // expected-warning {{the computation of the size of the memory allocation may overflow}} - int i = 0; - do { - x[i++] = 0; - } while (i < n); - return x; -} - -void * f12(int n) -{ - n = (n > 10 ? 10 : n); - int * x = malloc(n * sizeof(int)); // no-warning - for (int i = 0; i < n; i++) - x[i] = i; - return x; -} - -struct s13 -{ - int n; -}; - -void * f13(struct s13 *s) -{ - if (s->n > 10) - return NULL; - return malloc(s->n * sizeof(int)); // no-warning -} - -void * f14(int n) -{ - if (n < 0) - return NULL; - return malloc(n * sizeof(int)); // expected-warning {{the computation of the size of the memory allocation may overflow}} -} - -void *check_before_malloc(int n, int x) { - int *p = NULL; - if (n > 10) - return NULL; - if (x == 42) - p = malloc(n * sizeof(int)); // no-warning, the check precedes the allocation - - // Do some other stuff, e.g. initialize the memory. - return p; -} - -void *check_after_malloc(int n, int x) { - int *p = NULL; - if (x == 42) - p = malloc(n * sizeof(int)); // expected-warning {{the computation of the size of the memory allocation may overflow}} - - // The check is after the allocation! - if (n > 10) { - // Do something conditionally. - } - return p; -} - -#define GREATER_THAN(lhs, rhs) (lhs > rhs) -void *check_after_malloc_using_macros(int n, int x) { - int *p = NULL; - if (x == 42) - p = malloc(n * sizeof(int)); // expected-warning {{the computation of the size of the memory allocation may overflow}} - - if (GREATER_THAN(n, 10)) - return NULL; - - // Do some other stuff, e.g. initialize the memory. - return p; -} -#undef GREATER_THAN diff --git a/clang/test/Analysis/malloc-overflow.cpp b/clang/test/Analysis/malloc-overflow.cpp deleted file mode 100644 index e070217cf7d8e2..00000000000000 --- a/clang/test/Analysis/malloc-overflow.cpp +++ /dev/null @@ -1,12 +0,0 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.security.MallocOverflow -verify %s -// expected-no-diagnostics - -class A { -public: - A& operator<<(const A &a); -}; - -void f() { - A a = A(), b = A(); - a << b; -} diff --git a/clang/test/Analysis/malloc-overflow2.c b/clang/test/Analysis/malloc-overflow2.c deleted file mode 100644 index 7c580602e682ab..00000000000000 --- a/clang/test/Analysis/malloc-overflow2.c +++ /dev/null @@ -1,40 +0,0 @@ -// RUN: %clang_analyze_cc1 -triple x86_64-unknown-unknown -analyzer-checker=alpha.security.MallocOverflow,unix -verify %s -// RUN: %clang_analyze_cc1 -triple x86_64-unknown-unknown -analyzer-checker=alpha.security.MallocOverflow,unix,optin.portability -DPORTABILITY -verify %s - -typedef __typeof__(sizeof(int)) size_t; -extern void *malloc(size_t); -extern void free(void *ptr); - -void *malloc(unsigned long s); - -struct table { - int nentry; - unsigned *table; - unsigned offset_max; -}; - -static int table_build(struct table *t) { - - t->nentry = ((t->offset_max >> 2) + 31) / 32; - t->table = (unsigned *)malloc(sizeof(unsigned) * t->nentry); // expected-warning {{the computation of the size of the memory allocation may overflow}} - - int n; - n = 10000; - int *p = malloc(sizeof(int) * n); // no-warning - - free(p); - return t->nentry; -} - -static int table_build_1(struct table *t) { - t->nentry = (sizeof(struct table) * 2 + 31) / 32; - t->table = (unsigned *)malloc(sizeof(unsigned) * t->nentry); // no-warning - return t->nentry; -} - -void *f(int n) { - return malloc(n * 0 * sizeof(int)); -#ifdef PORTABILITY - // expected-warning@-2{{Call to 'malloc' has an allocation size of 0 bytes}} -#endif -} diff --git a/clang/www/analyzer/potential_checkers.html b/clang/www/analyzer/potential_checkers.html index ee9ba164387f34..ad789b83e71b71 100644 --- a/clang/www/analyzer/potential_checkers.html +++ b/clang/www/analyzer/potential_checkers.html @@ -90,8 +90,6 @@ <h3>memory</h3> memory.NegativeArraySize</span><span class="lang"> (C, C++)</span><div class="descr"> 'n' is used to specify the buffer size may be negative. -<br>Note: possibly an enhancement to <span class="name"> -alpha.security.MallocOverflow</span>. <p>Source: <a href="https://cwe.mitre.org/data/definitions/20.html">CWE-20, Example 2</a>.</p></div></div></td> <td><div class="exampleContainer expandable"> diff --git a/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn b/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn index 34b7822f4f400f..3b640ae41b9f62 100644 --- a/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn +++ b/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn @@ -75,7 +75,6 @@ static_library("Checkers") { "MacOSKeychainAPIChecker.cpp", "MacOSXAPIChecker.cpp", "MallocChecker.cpp", - "MallocOverflowSecurityChecker.cpp", "MallocSizeofChecker.cpp", "MismatchedIteratorChecker.cpp", "MmapWriteExecChecker.cpp", From 24126e3db4f064569836057f3ac6ee7bf941fd3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com> Date: Wed, 14 Aug 2024 14:05:05 +0200 Subject: [PATCH 2/2] Mention this change in the release notes --- clang/docs/ReleaseNotes.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 39e1b0fcb09bbd..007ccdf49dea19 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -347,6 +347,11 @@ Improvements Moved checkers ^^^^^^^^^^^^^^ +- The checker ``alpha.security.MallocOverflow`` was deleted because it was + badly implemented and its agressive logic produced too many false positives. + To detect too large arguments passed to malloc, consider using the checker + ``alpha.taint.TaintedAlloc``. + .. _release-notes-sanitizers: Sanitizers _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits