[llvm] 6e17ed9 - [analyzer] Consolidate array bound checkers (#125534)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 6 08:45:46 PST 2025
Author: Donát Nagy
Date: 2025-02-06T17:45:42+01:00
New Revision: 6e17ed9b04e5523cc910bf171c3122dcc64b86db
URL: https://github.com/llvm/llvm-project/commit/6e17ed9b04e5523cc910bf171c3122dcc64b86db
DIFF: https://github.com/llvm/llvm-project/commit/6e17ed9b04e5523cc910bf171c3122dcc64b86db.diff
LOG: [analyzer] Consolidate array bound checkers (#125534)
Before this commit, there were two alpha checkers that used different
algorithms/logic for detecting out of bounds memory access: the old
`alpha.security.ArrayBound` and the experimental, more complex
`alpha.security.ArrayBoundV2`.
After lots of quality improvement commits ArrayBoundV2 is now stable
enough to be moved out of the alpha stage. As indexing (and dereference)
are common operations, it still produces a significant amount of false
positives, but not much more than e.g. `core.NullDereference` or
`core.UndefinedBinaryOperatorResult`, so it should be acceptable as a
non-`core` checker.
At this point `alpha.security.ArrayBound` became obsolete (there is a
better tool for the same task), so I'm removing it from the codebase.
With this I can eliminate the ugly "V2" version mark almost everywhere
and rename `alpha.security.ArrayBoundV2` to `security.ArrayBound`.
(The version mark is preserved in the filename "ArrayBoundCheckerV2", to
ensure a clear git history. I'll rename it to "ArrayBoundChecker.cpp" in
a separate commit.)
This commit adapts the unit tests of `alpha.security.ArrayBound` to
testing the new `security.ArrayBound` (= old ArrayBoundV2). Currently
the names of the test files are very haphazard, I'll probably create a
separate followup commit that consolidates this.
Added:
clang/test/Analysis/out-of-bounds-constraint-check.c
Modified:
clang/docs/ReleaseNotes.rst
clang/docs/analyzer/checkers.rst
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
clang/lib/StaticAnalyzer/Core/MemRegion.cpp
clang/test/Analysis/index-type.c
clang/test/Analysis/misc-ps-region-store.m
clang/test/Analysis/no-outofbounds.c
clang/test/Analysis/out-of-bounds-diagnostics.c
clang/test/Analysis/out-of-bounds-new.cpp
clang/test/Analysis/out-of-bounds-notes.c
clang/test/Analysis/out-of-bounds.c
clang/test/Analysis/outofbound-notwork.c
clang/test/Analysis/outofbound.c
clang/test/Analysis/runtime-regression.c
clang/test/Analysis/taint-diagnostic-visitor.c
clang/test/Analysis/taint-generic.c
clang/test/Analysis/taint-generic.cpp
clang/www/analyzer/open_projects.html
clang/www/analyzer/potential_checkers.html
llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn
Removed:
clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
clang/test/Analysis/array-bound-v2-constraint-check.c
clang/test/Analysis/rdar-6541136-region.c
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 522905a152541a0..60de28e31c4f7be 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -263,6 +263,11 @@ Improvements
Moved checkers
^^^^^^^^^^^^^^
+- After lots of improvements, the checker ``alpha.security.ArrayBoundV2`` is
+ renamed to ``security.ArrayBound``. As this checker is stable now, the old
+ checker ``alpha.security.ArrayBound`` (which was searching for the same kind
+ of bugs with an
diff erent, simpler and less accurate algorithm) is removed.
+
.. _release-notes-sanitizers:
Sanitizers
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index e093b2d672a74e3..707067358fdfe30 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1332,10 +1332,69 @@ security
Security related checkers.
+.. _security-ArrayBound:
+
+security.ArrayBound (C, C++)
+""""""""""""""""""""""""""""
+Report out of bounds access to memory that is before the start or after the end
+of the accessed region (array, heap-allocated region, string literal etc.).
+This usually means incorrect indexing, but the checker also detects access via
+the operators ``*`` and ``->``.
+
+.. code-block:: c
+
+ void test_underflow(int x) {
+ int buf[100][100];
+ if (x < 0)
+ buf[0][x] = 1; // warn
+ }
+
+ void test_overflow() {
+ int buf[100];
+ int *p = buf + 100;
+ *p = 1; // warn
+ }
+
+If checkers like :ref:`unix-Malloc` or :ref:`cplusplus-NewDelete` are enabled
+to model the behavior of ``malloc()``, ``operator new`` and similar
+allocators), then this checker can also reports out of bounds access to
+dynamically allocated memory:
+
+.. code-block:: cpp
+
+ int *test_dynamic() {
+ int *mem = new int[100];
+ mem[-1] = 42; // warn
+ return mem;
+ }
+
+In uncertain situations (when the checker can neither prove nor disprove that
+overflow occurs), the checker assumes that the the index (more precisely, the
+memory offeset) is within bounds.
+
+However, if :ref:`optin-taint-GenericTaint` is enabled and the index/offset is
+tainted (i.e. it is influenced by an untrusted souce), then this checker
+reports the potential out of bounds access:
+
+.. code-block:: c
+
+ void test_with_tainted_index() {
+ char s[] = "abc";
+ int x = getchar();
+ char c = s[x]; // warn: potential out of bounds access with tainted index
+ }
+
+.. note::
+
+ This checker is an improved and renamed version of the checker that was
+ previously known as ``alpha.security.ArrayBoundV2``. The old checker
+ ``alpha.security.ArrayBound`` was removed when the (previously
+ "experimental") V2 variant became stable enough for regular use.
+
.. _security-cert-env-InvalidPtr:
security.cert.env.InvalidPtr
-""""""""""""""""""""""""""""""""""
+""""""""""""""""""""""""""""
Corresponds to SEI CERT Rules `ENV31-C <https://wiki.sei.cmu.edu/confluence/display/c/ENV31-C.+Do+not+rely+on+an+environment+pointer+following+an+operation+that+may+invalidate+it>`_ and `ENV34-C <https://wiki.sei.cmu.edu/confluence/display/c/ENV34-C.+Do+not+store+pointers+returned+by+certain+functions>`_.
@@ -3216,78 +3275,6 @@ Warns against using one vs. many plural pattern in code when generating localize
alpha.security
^^^^^^^^^^^^^^
-.. _alpha-security-ArrayBound:
-
-alpha.security.ArrayBound (C)
-"""""""""""""""""""""""""""""
-Warn about buffer overflows (older checker).
-
-.. code-block:: c
-
- void test() {
- char *s = "";
- char c = s[1]; // warn
- }
-
- struct seven_words {
- int c[7];
- };
-
- void test() {
- struct seven_words a, *p;
- p = &a;
- p[0] = a;
- p[1] = a;
- p[2] = a; // warn
- }
-
- // note: requires unix.Malloc or
- // alpha.unix.MallocWithAnnotations checks enabled.
- void test() {
- int *p = malloc(12);
- p[3] = 4; // warn
- }
-
- void test() {
- char a[2];
- int *b = (int*)a;
- b[1] = 3; // warn
- }
-
-.. _alpha-security-ArrayBoundV2:
-
-alpha.security.ArrayBoundV2 (C)
-"""""""""""""""""""""""""""""""
-Warn about buffer overflows (newer checker).
-
-.. code-block:: c
-
- void test() {
- char *s = "";
- char c = s[1]; // warn
- }
-
- void test() {
- int buf[100];
- int *p = buf;
- p = p + 99;
- p[1] = 1; // warn
- }
-
- // note: compiler has internal check for this.
- // Use -Wno-array-bounds to suppress compiler warning.
- void test() {
- int buf[100][100];
- buf[0][-1] = 1; // warn
- }
-
- // note: requires optin.taint check turned on.
- void test() {
- char s[] = "abc";
- int x = getchar();
- char c = s[x]; // warn: index is tainted
- }
-
.. _alpha-security-ReturnPtrRange:
alpha.security.ReturnPtrRange (C)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 1361da46c3c81dc..9bf491eac1e0ebd 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -989,30 +989,41 @@ def decodeValueOfObjCType : Checker<"decodeValueOfObjCType">,
let ParentPackage = Security in {
-def FloatLoopCounter : Checker<"FloatLoopCounter">,
- HelpText<"Warn on using a floating point value as a loop counter (CERT: "
- "FLP30-C, FLP30-CPP)">,
- Dependencies<[SecuritySyntaxChecker]>,
- Documentation<HasDocumentation>;
-
-def MmapWriteExecChecker : Checker<"MmapWriteExec">,
- HelpText<"Warn on mmap() calls with both writable and executable access">,
- Documentation<HasDocumentation>;
-
-def PointerSubChecker : Checker<"PointerSub">,
- HelpText<"Check for pointer subtractions on two pointers pointing to "
- "
diff erent memory chunks">,
- Documentation<HasDocumentation>;
-
-def PutenvStackArray : Checker<"PutenvStackArray">,
- HelpText<"Finds calls to the function 'putenv' which pass a pointer to "
- "an automatic (stack-allocated) array as the argument.">,
- Documentation<HasDocumentation>;
-
-def SetgidSetuidOrderChecker : Checker<"SetgidSetuidOrder">,
- HelpText<"Warn on possible reversed order of 'setgid(getgid()))' and "
- "'setuid(getuid())' (CERT: POS36-C)">,
- Documentation<HasDocumentation>;
+ def ArrayBoundChecker : Checker<"ArrayBound">,
+ HelpText<"Warn about out of bounds access to memory">,
+ Documentation<HasDocumentation>;
+
+ def FloatLoopCounter
+ : Checker<"FloatLoopCounter">,
+ HelpText<
+ "Warn on using a floating point value as a loop counter (CERT: "
+ "FLP30-C, FLP30-CPP)">,
+ Dependencies<[SecuritySyntaxChecker]>,
+ Documentation<HasDocumentation>;
+
+ def MmapWriteExecChecker
+ : Checker<"MmapWriteExec">,
+ HelpText<
+ "Warn on mmap() calls with both writable and executable access">,
+ Documentation<HasDocumentation>;
+
+ def PointerSubChecker
+ : Checker<"PointerSub">,
+ HelpText<"Check for pointer subtractions on two pointers pointing to "
+ "
diff erent memory chunks">,
+ Documentation<HasDocumentation>;
+
+ def PutenvStackArray
+ : Checker<"PutenvStackArray">,
+ HelpText<"Finds calls to the function 'putenv' which pass a pointer to "
+ "an automatic (stack-allocated) array as the argument.">,
+ Documentation<HasDocumentation>;
+
+ def SetgidSetuidOrderChecker
+ : Checker<"SetgidSetuidOrder">,
+ HelpText<"Warn on possible reversed order of 'setgid(getgid()))' and "
+ "'setuid(getuid())' (CERT: POS36-C)">,
+ Documentation<HasDocumentation>;
} // end "security"
@@ -1035,14 +1046,6 @@ let ParentPackage = ENV in {
let ParentPackage = SecurityAlpha in {
-def ArrayBoundChecker : Checker<"ArrayBound">,
- HelpText<"Warn about buffer overflows (older checker)">,
- Documentation<HasDocumentation>;
-
-def ArrayBoundCheckerV2 : Checker<"ArrayBoundV2">,
- HelpText<"Warn about buffer overflows (newer checker)">,
- Documentation<HasDocumentation>;
-
def ReturnPointerRangeChecker : Checker<"ReturnPtrRange">,
HelpText<"Check for an out-of-bound pointer being returned to callers">,
Documentation<HasDocumentation>;
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
deleted file mode 100644
index c990ad138f89050..000000000000000
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ /dev/null
@@ -1,92 +0,0 @@
-//== ArrayBoundChecker.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
-//
-//===----------------------------------------------------------------------===//
-//
-// This file defines ArrayBoundChecker, which is a path-sensitive check
-// which looks for an out-of-bound array element access.
-//
-//===----------------------------------------------------------------------===//
-
-#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"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
-
-using namespace clang;
-using namespace ento;
-
-namespace {
-class ArrayBoundChecker :
- public Checker<check::Location> {
- const BugType BT{this, "Out-of-bound array access"};
-
-public:
- void checkLocation(SVal l, bool isLoad, const Stmt* S,
- CheckerContext &C) const;
-};
-}
-
-void ArrayBoundChecker::checkLocation(SVal l, bool isLoad, const Stmt* LoadS,
- CheckerContext &C) const {
- // Check for out of bound array element access.
- const MemRegion *R = l.getAsRegion();
- if (!R)
- return;
-
- const ElementRegion *ER = dyn_cast<ElementRegion>(R);
- if (!ER)
- return;
-
- // Get the index of the accessed element.
- DefinedOrUnknownSVal Idx = ER->getIndex().castAs<DefinedOrUnknownSVal>();
-
- // Zero index is always in bound, this also passes ElementRegions created for
- // pointer casts.
- if (Idx.isZeroConstant())
- return;
-
- ProgramStateRef state = C.getState();
-
- // Get the size of the array.
- DefinedOrUnknownSVal ElementCount = getDynamicElementCount(
- state, ER->getSuperRegion(), C.getSValBuilder(), ER->getValueType());
-
- ProgramStateRef StInBound, StOutBound;
- std::tie(StInBound, StOutBound) = state->assumeInBoundDual(Idx, ElementCount);
- if (StOutBound && !StInBound) {
- ExplodedNode *N = C.generateErrorNode(StOutBound);
- if (!N)
- return;
-
- // FIXME: It would be nice to eventually make this diagnostic more clear,
- // e.g., by referencing the original declaration or by saying *why* this
- // reference is outside the range.
-
- // Generate a report for this bug.
- auto report = std::make_unique<PathSensitiveBugReport>(
- BT, "Access out-of-bound array element (buffer overflow)", N);
-
- report->addRange(LoadS->getSourceRange());
- C.emitReport(std::move(report));
- return;
- }
-
- // Array bound check succeeded. From this point forward the array bound
- // should always succeed.
- C.addTransition(StInBound);
-}
-
-void ento::registerArrayBoundChecker(CheckerManager &mgr) {
- mgr.registerChecker<ArrayBoundChecker>();
-}
-
-bool ento::shouldRegisterArrayBoundChecker(const CheckerManager &mgr) {
- return true;
-}
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
index 6422933c8828a91..6f8d6dbd573f403 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -6,11 +6,17 @@
//
//===----------------------------------------------------------------------===//
//
-// This file defines ArrayBoundCheckerV2, which is a path-sensitive check
-// which looks for an out-of-bound array element access.
+// This file defines security.ArrayBound, which is a path-sensitive checker
+// that looks for out of bounds access of memory regions.
//
//===----------------------------------------------------------------------===//
+// NOTE: The name of this file ends with "V2" because previously
+// "ArrayBoundChecker.cpp" contained the implementation of another (older and
+// simpler) checker that was called `alpha.security.ArrayBound`.
+// TODO: Rename this file to "ArrayBoundChecker.cpp" when it won't be confused
+// with that older file.
+
#include "clang/AST/CharUnits.h"
#include "clang/AST/ParentMapContext.h"
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
@@ -124,9 +130,9 @@ struct Messages {
// callbacks, we'd need to duplicate the logic that evaluates these
// expressions.) The `MemberExpr` callback would work as `PreStmt` but it's
// defined as `PostStmt` for the sake of consistency with the other callbacks.
-class ArrayBoundCheckerV2 : public Checker<check::PostStmt<ArraySubscriptExpr>,
- check::PostStmt<UnaryOperator>,
- check::PostStmt<MemberExpr>> {
+class ArrayBoundChecker : public Checker<check::PostStmt<ArraySubscriptExpr>,
+ check::PostStmt<UnaryOperator>,
+ check::PostStmt<MemberExpr>> {
BugType BT{this, "Out-of-bound access"};
BugType TaintBT{this, "Out-of-bound access", categories::TaintedData};
@@ -547,7 +553,7 @@ bool StateUpdateReporter::providesInformationAboutInteresting(
return false;
}
-void ArrayBoundCheckerV2::performCheck(const Expr *E, CheckerContext &C) const {
+void ArrayBoundChecker::performCheck(const Expr *E, CheckerContext &C) const {
const SVal Location = C.getSVal(E);
// The header ctype.h (from e.g. glibc) implements the isXXXXX() macros as
@@ -670,9 +676,9 @@ void ArrayBoundCheckerV2::performCheck(const Expr *E, CheckerContext &C) const {
C.addTransition(State, SUR.createNoteTag(C));
}
-void ArrayBoundCheckerV2::markPartsInteresting(PathSensitiveBugReport &BR,
- ProgramStateRef ErrorState,
- NonLoc Val, bool MarkTaint) {
+void ArrayBoundChecker::markPartsInteresting(PathSensitiveBugReport &BR,
+ ProgramStateRef ErrorState,
+ NonLoc Val, bool MarkTaint) {
if (SymbolRef Sym = Val.getAsSymbol()) {
// If the offset is a symbolic value, iterate over its "parts" with
// `SymExpr::symbols()` and mark each of them as interesting.
@@ -693,10 +699,10 @@ void ArrayBoundCheckerV2::markPartsInteresting(PathSensitiveBugReport &BR,
}
}
-void ArrayBoundCheckerV2::reportOOB(CheckerContext &C,
- ProgramStateRef ErrorState, Messages Msgs,
- NonLoc Offset, std::optional<NonLoc> Extent,
- bool IsTaintBug /*=false*/) const {
+void ArrayBoundChecker::reportOOB(CheckerContext &C, ProgramStateRef ErrorState,
+ Messages Msgs, NonLoc Offset,
+ std::optional<NonLoc> Extent,
+ bool IsTaintBug /*=false*/) const {
ExplodedNode *ErrorNode = C.generateErrorNode(ErrorState);
if (!ErrorNode)
@@ -725,7 +731,7 @@ void ArrayBoundCheckerV2::reportOOB(CheckerContext &C,
C.emitReport(std::move(BR));
}
-bool ArrayBoundCheckerV2::isFromCtypeMacro(const Stmt *S, ASTContext &ACtx) {
+bool ArrayBoundChecker::isFromCtypeMacro(const Stmt *S, ASTContext &ACtx) {
SourceLocation Loc = S->getBeginLoc();
if (!Loc.isMacroID())
return false;
@@ -744,7 +750,7 @@ bool ArrayBoundCheckerV2::isFromCtypeMacro(const Stmt *S, ASTContext &ACtx) {
(MacroName == "isupper") || (MacroName == "isxdigit"));
}
-bool ArrayBoundCheckerV2::isInAddressOf(const Stmt *S, ASTContext &ACtx) {
+bool ArrayBoundChecker::isInAddressOf(const Stmt *S, ASTContext &ACtx) {
ParentMapContext &ParentCtx = ACtx.getParentMapContext();
do {
const DynTypedNodeList Parents = ParentCtx.getParents(*S);
@@ -756,10 +762,10 @@ bool ArrayBoundCheckerV2::isInAddressOf(const Stmt *S, ASTContext &ACtx) {
return UnaryOp && UnaryOp->getOpcode() == UO_AddrOf;
}
-bool ArrayBoundCheckerV2::isIdiomaticPastTheEndPtr(const Expr *E,
- ProgramStateRef State,
- NonLoc Offset, NonLoc Limit,
- CheckerContext &C) {
+bool ArrayBoundChecker::isIdiomaticPastTheEndPtr(const Expr *E,
+ ProgramStateRef State,
+ NonLoc Offset, NonLoc Limit,
+ CheckerContext &C) {
if (isa<ArraySubscriptExpr>(E) && isInAddressOf(E, C.getASTContext())) {
auto [EqualsToThreshold, NotEqualToThreshold] = compareValueToThreshold(
State, Offset, Limit, C.getSValBuilder(), /*CheckEquality=*/true);
@@ -768,10 +774,10 @@ bool ArrayBoundCheckerV2::isIdiomaticPastTheEndPtr(const Expr *E,
return false;
}
-void ento::registerArrayBoundCheckerV2(CheckerManager &mgr) {
- mgr.registerChecker<ArrayBoundCheckerV2>();
+void ento::registerArrayBoundChecker(CheckerManager &mgr) {
+ mgr.registerChecker<ArrayBoundChecker>();
}
-bool ento::shouldRegisterArrayBoundCheckerV2(const CheckerManager &mgr) {
+bool ento::shouldRegisterArrayBoundChecker(const CheckerManager &mgr) {
return true;
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index fcbe8b864b6e41d..ccff5d0ac3b9649 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -7,7 +7,6 @@ set(LLVM_LINK_COMPONENTS
add_clang_library(clangStaticAnalyzerCheckers
AnalysisOrderChecker.cpp
AnalyzerStatsChecker.cpp
- ArrayBoundChecker.cpp
ArrayBoundCheckerV2.cpp
BasicObjCFoundationChecks.cpp
BitwiseShiftChecker.cpp
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 1a14f38e34f0e19..39dcaf02dbe2584 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -547,8 +547,10 @@ ProgramStateRef CStringChecker::checkInit(CheckerContext &C,
}
return State;
}
-
-// FIXME: This was originally copied from ArrayBoundChecker.cpp. Refactor?
+// FIXME: The root of this logic was copied from the old checker
+// alpha.security.ArrayBound (which is removed within this commit).
+// It should be refactored to use the
diff erent, more sophisticated bounds
+// checking logic used by the new checker ``security.ArrayBound``.
ProgramStateRef CStringChecker::CheckLocation(CheckerContext &C,
ProgramStateRef state,
AnyArgExpr Buffer, SVal Element,
diff --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
index 2c5cd2cf7630f62..c81bb632b83e944 100644
--- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -891,9 +891,8 @@ DefinedOrUnknownSVal MemRegionManager::getStaticSize(const MemRegion *MR,
return Size;
}
- // FIXME: The following are being used in 'SimpleSValBuilder' and in
- // 'ArrayBoundChecker::checkLocation' because there is no symbol to
- // represent the regions more appropriately.
+ // FIXME: The following are being used in 'SimpleSValBuilder' because there
+ // is no symbol to represent the regions more appropriately.
case MemRegion::BlockDataRegionKind:
case MemRegion::BlockCodeRegionKind:
case MemRegion::FunctionCodeRegionKind:
diff --git a/clang/test/Analysis/index-type.c b/clang/test/Analysis/index-type.c
index 997d45c1e5abade..818806c4aff3b5f 100644
--- a/clang/test/Analysis/index-type.c
+++ b/clang/test/Analysis/index-type.c
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,alpha.security.ArrayBoundV2 -Wno-implicit-function-declaration -verify %s
-// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -analyzer-checker=core,alpha.security.ArrayBoundV2 -Wno-implicit-function-declaration -DM32 -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,security.ArrayBound -Wno-implicit-function-declaration -verify %s
+// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -analyzer-checker=core,security.ArrayBound -Wno-implicit-function-declaration -DM32 -verify %s
// expected-no-diagnostics
#define UINT_MAX (~0u)
diff --git a/clang/test/Analysis/misc-ps-region-store.m b/clang/test/Analysis/misc-ps-region-store.m
index a882e7eb0dc909e..bd8b3350ba6da18 100644
--- a/clang/test/Analysis/misc-ps-region-store.m
+++ b/clang/test/Analysis/misc-ps-region-store.m
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -triple i386-apple-darwin9 -analyzer-checker=core,alpha.core.CastToStruct,alpha.security.ReturnPtrRange,alpha.security.ArrayBound -verify -fblocks -Wno-objc-root-class -Wno-strict-prototypes -Wno-error=implicit-function-declaration %s
-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin9 -DTEST_64 -analyzer-checker=core,alpha.core.CastToStruct,alpha.security.ReturnPtrRange,alpha.security.ArrayBound -verify -fblocks -Wno-objc-root-class -Wno-strict-prototypes -Wno-error=implicit-function-declaration %s
+// RUN: %clang_analyze_cc1 -triple i386-apple-darwin9 -analyzer-checker=core,alpha.core.CastToStruct,alpha.security.ReturnPtrRange,security.ArrayBound -verify -fblocks -Wno-objc-root-class -Wno-strict-prototypes -Wno-error=implicit-function-declaration %s
+// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin9 -DTEST_64 -analyzer-checker=core,alpha.core.CastToStruct,alpha.security.ReturnPtrRange,security.ArrayBound -verify -fblocks -Wno-objc-root-class -Wno-strict-prototypes -Wno-error=implicit-function-declaration %s
typedef long unsigned int size_t;
void *memcpy(void *, const void *, size_t);
@@ -928,7 +928,7 @@ void pr6288_pos(int z) {
int *px[1];
int i;
for (i = 0; i < z; i++)
- px[i] = &x[i]; // expected-warning{{Access out-of-bound array element (buffer overflow)}}
+ px[i] = &x[i]; // expected-warning{{Out of bound access to memory after the end of 'px'}}
*(px[0]) = 0; // expected-warning{{Dereference of undefined pointer value}}
}
@@ -976,12 +976,17 @@ - (void) rdar7817800_baz {
}
@end
-// PR 6036 - This test case triggered a crash inside StoreManager::CastRegion because the size
-// of 'unsigned long (*)[0]' is 0.
+// PR 6036 - This test case triggered a crash inside StoreManager::CastRegion
+// because the size of 'unsigned long (*)[0]' is 0.
+// NOTE: This old crash was probably triggered via the old alpha checker
+// `alpha.security.ArrayBound` (which was logic that's
diff erent from the
+// current `security.ArrayBound`). Although that code was removed, it's worth
+// to keep this testcase as a generic example of a zero-sized type.
struct pr6036_a { int pr6036_b; };
struct pr6036_c;
void u132monitk (struct pr6036_c *pr6036_d) {
- (void) ((struct pr6036_a *) (unsigned long (*)[0]) ((char *) pr6036_d - 1))->pr6036_b; // expected-warning{{Casting a non-structure type to a structure type and accessing a field can lead to memory access errors or data corruption}}
+ (void) ((struct pr6036_a *) (unsigned long (*)[0]) ((char *) pr6036_d - 1))->pr6036_b;
+ // expected-warning at -1 {{Casting a non-structure type to a structure type and accessing a field can lead to memory access errors or data corruption}}
}
// ?-expressions used as a base of a member expression should be treated as an lvalue
diff --git a/clang/test/Analysis/no-outofbounds.c b/clang/test/Analysis/no-outofbounds.c
index 15155729067e9b5..c6219ae74ab42ca 100644
--- a/clang/test/Analysis/no-outofbounds.c
+++ b/clang/test/Analysis/no-outofbounds.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,alpha.unix,alpha.security.ArrayBound -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,alpha.unix,security.ArrayBound -verify %s
// expected-no-diagnostics
//===----------------------------------------------------------------------===//
diff --git a/clang/test/Analysis/array-bound-v2-constraint-check.c b/clang/test/Analysis/out-of-bounds-constraint-check.c
similarity index 97%
rename from clang/test/Analysis/array-bound-v2-constraint-check.c
rename to clang/test/Analysis/out-of-bounds-constraint-check.c
index 91f748e655ce2dd..df48c8c17071388 100644
--- a/clang/test/Analysis/array-bound-v2-constraint-check.c
+++ b/clang/test/Analysis/out-of-bounds-constraint-check.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,alpha.security.ArrayBoundV2,debug.ExprInspection \
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,security.ArrayBound,debug.ExprInspection \
// RUN: -analyzer-config eagerly-assume=false -verify %s
void clang_analyzer_eval(int);
diff --git a/clang/test/Analysis/out-of-bounds-diagnostics.c b/clang/test/Analysis/out-of-bounds-diagnostics.c
index 65bc28f58276fd2..1db01251148e14b 100644
--- a/clang/test/Analysis/out-of-bounds-diagnostics.c
+++ b/clang/test/Analysis/out-of-bounds-diagnostics.c
@@ -1,5 +1,5 @@
// RUN: %clang_analyze_cc1 -Wno-array-bounds -analyzer-output=text \
-// RUN: -analyzer-checker=core,alpha.security.ArrayBoundV2,unix.Malloc,optin.taint -verify %s
+// RUN: -analyzer-checker=core,security.ArrayBound,unix.Malloc,optin.taint -verify %s
int TenElements[10];
diff --git a/clang/test/Analysis/out-of-bounds-new.cpp b/clang/test/Analysis/out-of-bounds-new.cpp
index f541bdf810d79ce..4e5442422bff42f 100644
--- a/clang/test/Analysis/out-of-bounds-new.cpp
+++ b/clang/test/Analysis/out-of-bounds-new.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -std=c++11 -Wno-array-bounds -analyzer-checker=unix,core,alpha.security.ArrayBoundV2 -verify %s
+// RUN: %clang_analyze_cc1 -std=c++11 -Wno-array-bounds -analyzer-checker=unix,core,security.ArrayBound -verify %s
// Tests doing an out-of-bounds access after the end of an array using:
// - constant integer index
diff --git a/clang/test/Analysis/out-of-bounds-notes.c b/clang/test/Analysis/out-of-bounds-notes.c
index 391089b6a35d81b..7256beb7e893eb2 100644
--- a/clang/test/Analysis/out-of-bounds-notes.c
+++ b/clang/test/Analysis/out-of-bounds-notes.c
@@ -1,5 +1,5 @@
// RUN: %clang_analyze_cc1 -Wno-array-bounds -analyzer-output=text \
-// RUN: -analyzer-checker=core,alpha.security.ArrayBoundV2,unix.Malloc,optin.taint -verify %s
+// RUN: -analyzer-checker=core,security.ArrayBound,unix.Malloc,optin.taint -verify %s
int TenElements[10];
diff --git a/clang/test/Analysis/out-of-bounds.c b/clang/test/Analysis/out-of-bounds.c
index 1f771c2b3bd1389..734a56602e2aa40 100644
--- a/clang/test/Analysis/out-of-bounds.c
+++ b/clang/test/Analysis/out-of-bounds.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -Wno-array-bounds -analyzer-checker=core,alpha.security.ArrayBoundV2,debug.ExprInspection -verify %s
+// RUN: %clang_analyze_cc1 -Wno-array-bounds -analyzer-checker=core,security.ArrayBound,debug.ExprInspection -verify %s
void clang_analyzer_eval(int);
diff --git a/clang/test/Analysis/outofbound-notwork.c b/clang/test/Analysis/outofbound-notwork.c
index cf2239cee1301cc..1318c07bbf2a88d 100644
--- a/clang/test/Analysis/outofbound-notwork.c
+++ b/clang/test/Analysis/outofbound-notwork.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -Wno-array-bounds -analyzer-checker=core,alpha.security.ArrayBound -verify %s
+// RUN: %clang_analyze_cc1 -Wno-array-bounds -analyzer-checker=core,security.ArrayBound -verify %s
// XFAIL: *
// Once we better handle modeling of sizes of VLAs, we can pull this back
@@ -9,7 +9,7 @@ void sizeof_vla(int a) {
char x[a];
int y[sizeof(x)];
y[4] = 4; // no-warning
- y[5] = 5; // expected-warning{{out-of-bound}}
+ y[5] = 5; // expected-warning{{Out of bounds access}}
}
}
@@ -18,7 +18,7 @@ void sizeof_vla_2(int a) {
char x[a];
int y[sizeof(x) / sizeof(char)];
y[4] = 4; // no-warning
- y[5] = 5; // expected-warning{{out-of-bound}}
+ y[5] = 5; // expected-warning{{Out of bounds access}}
}
}
@@ -27,6 +27,6 @@ void sizeof_vla_3(int a) {
char x[a];
int y[sizeof(*&*&*&x)];
y[4] = 4; // no-warning
- y[5] = 5; // expected-warning{{out-of-bound}}
+ y[5] = 5; // expected-warning{{Out of bounds access}}
}
}
diff --git a/clang/test/Analysis/outofbound.c b/clang/test/Analysis/outofbound.c
index 009cf33f613091d..d3d8ff2b2f0ed6d 100644
--- a/clang/test/Analysis/outofbound.c
+++ b/clang/test/Analysis/outofbound.c
@@ -1,7 +1,7 @@
// RUN: %clang_analyze_cc1 -Wno-array-bounds -verify %s \
// RUN: -analyzer-checker=core \
// RUN: -analyzer-checker=unix \
-// RUN: -analyzer-checker=alpha.security.ArrayBound \
+// RUN: -analyzer-checker=security.ArrayBound \
// RUN: -analyzer-config unix.DynamicMemoryModeling:Optimistic=true
typedef __typeof(sizeof(int)) size_t;
@@ -11,12 +11,12 @@ void *calloc(size_t, size_t);
char f1(void) {
char* s = "abcd";
char c = s[4]; // no-warning
- return s[5] + c; // expected-warning{{Access out-of-bound array element (buffer overflow)}}
+ return s[5] + c; // expected-warning{{Out of bound access to memory after}}
}
void f2(void) {
int *p = malloc(12);
- p[3] = 4; // expected-warning{{Access out-of-bound array element (buffer overflow)}}
+ p[3] = 4; // expected-warning{{Out of bound access to memory after}}
}
struct three_words {
@@ -31,7 +31,7 @@ void f3(void) {
struct three_words a, *p;
p = &a;
p[0] = a; // no-warning
- p[1] = a; // expected-warning{{Access out-of-bound array element (buffer overflow)}}
+ p[1] = a; // expected-warning{{Out of bound access to memory after}}
}
void f4(void) {
@@ -39,31 +39,33 @@ void f4(void) {
struct three_words a, *p = (struct three_words *)&c;
p[0] = a; // no-warning
p[1] = a; // no-warning
- p[2] = a; // expected-warning{{Access out-of-bound array element (buffer overflow)}}
+ p[2] = a; // should warn
+ // FIXME: This is an overflow, but currently security.ArrayBound only checks
+ // that the _beginning_ of the accessed element is within bounds.
}
void f5(void) {
char *p = calloc(2,2);
p[3] = '.'; // no-warning
- p[4] = '!'; // expected-warning{{out-of-bound}}
+ p[4] = '!'; // expected-warning{{Out of bound access}}
}
void f6(void) {
char a[2];
int *b = (int*)a;
- b[1] = 3; // expected-warning{{out-of-bound}}
+ b[1] = 3; // expected-warning{{Out of bound access}}
}
void f7(void) {
struct three_words a;
- a.c[3] = 1; // expected-warning{{out-of-bound}}
+ a.c[3] = 1; // expected-warning{{Out of bound access}}
}
void vla(int a) {
if (a == 5) {
int x[a];
x[4] = 4; // no-warning
- x[5] = 5; // expected-warning{{out-of-bound}}
+ x[5] = 5; // expected-warning{{Out of bound access}}
}
}
@@ -71,14 +73,14 @@ void alloca_region(int a) {
if (a == 5) {
char *x = __builtin_alloca(a);
x[4] = 4; // no-warning
- x[5] = 5; // expected-warning{{out-of-bound}}
+ x[5] = 5; // expected-warning{{Out of bound access}}
}
}
int symbolic_index(int a) {
int x[2] = {1, 2};
if (a == 2) {
- return x[a]; // expected-warning{{out-of-bound}}
+ return x[a]; // expected-warning{{Out of bound access}}
}
return 0;
}
@@ -86,7 +88,7 @@ int symbolic_index(int a) {
int symbolic_index2(int a) {
int x[2] = {1, 2};
if (a < 0) {
- return x[a]; // expected-warning{{out-of-bound}}
+ return x[a]; // expected-warning{{Out of bound access}}
}
return 0;
}
@@ -120,7 +122,7 @@ int overflow_binary_search(double in) {
} else {
eee += 1;
}
- if (in < ins[eee]) { // expected-warning {{Access out-of-bound array element (buffer overflow)}}
+ if (in < ins[eee]) { // expected-warning {{Out of bound access}}
eee -= 1;
}
}
diff --git a/clang/test/Analysis/rdar-6541136-region.c b/clang/test/Analysis/rdar-6541136-region.c
deleted file mode 100644
index f1a3a48a5fe4a8f..000000000000000
--- a/clang/test/Analysis/rdar-6541136-region.c
+++ /dev/null
@@ -1,27 +0,0 @@
-// RUN: %clang_analyze_cc1 -verify -analyzer-checker=core,alpha.security.ArrayBound %s
-
-struct tea_cheese { unsigned magic; };
-typedef struct tea_cheese kernel_tea_cheese_t;
-extern kernel_tea_cheese_t _wonky_gesticulate_cheese;
-
-// This test case exercises the ElementRegion::getRValueType() logic.
-
-void test1( void ) {
- kernel_tea_cheese_t *wonky = &_wonky_gesticulate_cheese;
- struct load_wine *cmd = (void*) &wonky[1];
- cmd = cmd;
- char *p = (void*) &wonky[1];
- kernel_tea_cheese_t *q = &wonky[1];
- // This test case tests both the RegionStore logic (doesn't crash) and
- // the out-of-bounds checking. We don't expect the warning for now since
- // out-of-bound checking is temporarily disabled.
- kernel_tea_cheese_t r = *q; // expected-warning{{Access out-of-bound array element (buffer overflow)}}
-}
-
-void test1_b( void ) {
- kernel_tea_cheese_t *wonky = &_wonky_gesticulate_cheese;
- struct load_wine *cmd = (void*) &wonky[1];
- cmd = cmd;
- char *p = (void*) &wonky[1];
- *p = 1; // expected-warning{{Access out-of-bound array element (buffer overflow)}}
-}
diff --git a/clang/test/Analysis/runtime-regression.c b/clang/test/Analysis/runtime-regression.c
index 55988e9df5ee0f8..9fa0017d5e4701b 100644
--- a/clang/test/Analysis/runtime-regression.c
+++ b/clang/test/Analysis/runtime-regression.c
@@ -1,5 +1,5 @@
// RUN: %clang_analyze_cc1 %s \
-// RUN: -analyzer-checker=core,alpha.security.ArrayBoundV2 \
+// RUN: -analyzer-checker=core,security.ArrayBound \
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -triple x86_64-unknown-linux-gnu \
// RUN: -verify
diff --git a/clang/test/Analysis/taint-diagnostic-visitor.c b/clang/test/Analysis/taint-diagnostic-visitor.c
index 223df9951fd6b9e..afe7117db71507d 100644
--- a/clang/test/Analysis/taint-diagnostic-visitor.c
+++ b/clang/test/Analysis/taint-diagnostic-visitor.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=optin.taint,core,alpha.security.ArrayBoundV2 -analyzer-output=text -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=optin.taint,core,security.ArrayBound -analyzer-output=text -verify %s
// This file is for testing enhanced diagnostics produced by the GenericTaintChecker
diff --git a/clang/test/Analysis/taint-generic.c b/clang/test/Analysis/taint-generic.c
index ad5a99fe8b3a35b..3c520612c5d9baf 100644
--- a/clang/test/Analysis/taint-generic.c
+++ b/clang/test/Analysis/taint-generic.c
@@ -3,7 +3,7 @@
// RUN: -analyzer-checker=optin.taint.GenericTaint \
// RUN: -analyzer-checker=optin.taint.TaintedDiv \
// RUN: -analyzer-checker=core \
-// RUN: -analyzer-checker=alpha.security.ArrayBoundV2 \
+// RUN: -analyzer-checker=security.ArrayBound \
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -analyzer-config \
// RUN: optin.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml
@@ -14,7 +14,7 @@
// RUN: -analyzer-checker=optin.taint.GenericTaint \
// RUN: -analyzer-checker=optin.taint.TaintedDiv \
// RUN: -analyzer-checker=core \
-// RUN: -analyzer-checker=alpha.security.ArrayBoundV2 \
+// RUN: -analyzer-checker=security.ArrayBound \
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -analyzer-config \
// RUN: optin.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml
diff --git a/clang/test/Analysis/taint-generic.cpp b/clang/test/Analysis/taint-generic.cpp
index 881c5baf889f6cd..8836e1d3d2d9865 100644
--- a/clang/test/Analysis/taint-generic.cpp
+++ b/clang/test/Analysis/taint-generic.cpp
@@ -1,5 +1,5 @@
// RUN: %clang_analyze_cc1 -std=c++11 -Wno-format-security \
-// RUN: -analyzer-checker=core,optin.taint,alpha.security.ArrayBoundV2,debug.ExprInspection \
+// RUN: -analyzer-checker=core,optin.taint,security.ArrayBound,debug.ExprInspection \
// RUN: -analyzer-config optin.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml \
// RUN: -verify %s
diff --git a/clang/www/analyzer/open_projects.html b/clang/www/analyzer/open_projects.html
index a7c99c6e25bb3c1..4d6e40f92f82c07 100644
--- a/clang/www/analyzer/open_projects.html
+++ b/clang/www/analyzer/open_projects.html
@@ -35,19 +35,6 @@ <h1>Open Projects</h1>
Such checkers should either be improved
up to a point where they can be enabled by default,
or removed from the analyzer entirely.
-
- <ul>
- <li><code>alpha.security.ArrayBound</code> and
- <code>alpha.security.ArrayBoundV2</code>
- <p>Array bounds checking is a desired feature,
- but having an acceptable rate of false positives might not be possible
- without a proper
- <a href="https://en.wikipedia.org/wiki/Widening_(computer_science)">loop widening</a> support.
- Additionally, it might be more promising to perform index checking based on
- <a href="https://en.wikipedia.org/wiki/Taint_checking">tainted</a> index values.
- <p><i>(Difficulty: Medium)</i></p></p>
- </li>
- </ul>
</li>
<li>Improve C++ support
diff --git a/clang/www/analyzer/potential_checkers.html b/clang/www/analyzer/potential_checkers.html
index ad789b83e71b710..2543db251be127c 100644
--- a/clang/www/analyzer/potential_checkers.html
+++ b/clang/www/analyzer/potential_checkers.html
@@ -965,7 +965,7 @@ <h3>undefined behavior</h3>
(C++03)</span><div class="descr">
Undefined behavior: out-of-bound <code>basic_string</code> access/modification.
<br>Note: possibly an enhancement to <span class="name">
-alpha.security.ArrayBoundV2</span>.
+security.ArrayBound</span>.
<p>Source: C++03 21.3.4p1; C++11 behavior is defined
(21.4.5p2).</p></div></div></td>
<td><div class="exampleContainer expandable">
@@ -1692,24 +1692,6 @@ <h3>
diff erent</h3>
<td class="aligned"></td></tr>
-<tr><td><div class="namedescr expandable"><span class="name">
-
diff erent.ArrayBound</span><span class="lang">
-(C++)</span><div class="descr">
-Out-of-bound dynamic array access.
-<br>Note: possibly an enhancement to <span class="name">
-alpha.security.ArrayBoundV2</span>.</div></div></td>
-<td><div class="exampleContainer expandable">
-<div class="example"><pre>
-void test() {
- int *p = new int[1];
- int i = 1;
- if(p[i]) {}; // warn
- delete[] p;
-}
-</pre></div></div></td>
-<td class="aligned"></td></tr>
-
-
<tr><td><div class="namedescr expandable"><span class="name">
diff erent.StrcpyInputSize</span><span class="lang">
(C)</span><div class="descr">
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 e1c773630de701c..c1bba99be3ba5c3 100644
--- a/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn
+++ b/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn
@@ -16,7 +16,6 @@ static_library("Checkers") {
sources = [
"AnalysisOrderChecker.cpp",
"AnalyzerStatsChecker.cpp",
- "ArrayBoundChecker.cpp",
"ArrayBoundCheckerV2.cpp",
"BasicObjCFoundationChecks.cpp",
"BitwiseShiftChecker.cpp",
More information about the llvm-commits
mailing list