[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.

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 different, simpler and less accurate algorithm) is removed.
+
 .. _release-notes-sanitizers:
 
 Sanitizers

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)

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 ArrayBoundChecker : Checker<"ArrayBound">,
+                          HelpText<"Warn about out of bounds access to memory">,
+                          Documentation<HasDocumentation>;
 
-def MmapWriteExecChecker : Checker<"MmapWriteExec">,
-  HelpText<"Warn on mmap() calls with both writable and executable access">,
-  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 PointerSubChecker : Checker<"PointerSub">,
-  HelpText<"Check for pointer subtractions on two pointers pointing to "
-           "different memory chunks">,
-  Documentation<HasDocumentation>;
+  def MmapWriteExecChecker
+      : Checker<"MmapWriteExec">,
+        HelpText<
+            "Warn on mmap() calls with both writable and executable access">,
+        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 PointerSubChecker
+      : Checker<"PointerSub">,
+        HelpText<"Check for pointer subtractions on two pointers pointing to "
+                 "different memory chunks">,
+        Documentation<HasDocumentation>;
 
-def SetgidSetuidOrderChecker : Checker<"SetgidSetuidOrder">,
-  HelpText<"Warn on possible reversed order of 'setgid(getgid()))' and "
-           "'setuid(getuid())' (CERT: POS36-C)">,
-  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>;

clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp

@@ -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;
-}

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;
 }

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

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 different, more sophisticated bounds
+// checking logic used by the new checker ``security.ArrayBound``.
 ProgramStateRef CStringChecker::CheckLocation(CheckerContext &C,
                                               ProgramStateRef state,
                                               AnyArgExpr Buffer, SVal Element,

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:

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)

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_qux(void*);
 }
 @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 different 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@-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

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
 
 //===----------------------------------------------------------------------===//

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);

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];
 

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

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];
 

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);
 

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}}
   }
 }

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;
     }
   }

clang/test/Analysis/rdar-6541136-region.c

@@ -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)}}
-}

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

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
 

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

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
 

clang/www/analyzer/open_projects.html

@@ -35,19 +35,6 @@ mailing list</a> to notify other members of the community.</p>
     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

clang/www/analyzer/potential_checkers.html

@@ -965,7 +965,7 @@ undefbehavior.BasicStringOutOfBound</span><span class="lang">
 (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 @@ bool test() {
 <td class="aligned"></td></tr>
 
 
-<tr><td><div class="namedescr expandable"><span class="name">
-different.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">
 different.StrcpyInputSize</span><span class="lang">
 (C)</span><div class="descr">

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",