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

1 files changed, 28 insertions, 22 deletions

diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
index 6422933..6f8d6db 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;
 }