https://github.com/NagyDonat created https://github.com/llvm/llvm-project/pull/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. ----- The effects of enabling this checker (compared to a run where only the other stable checkers are running): | Project | New Reports | Resolved Reports | |---------|-------------|------------------| | memcached | [2 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=memcached_1.6.8_edonnag_llvm-main_1883de3&newcheck=memcached_1.6.8_edonnag_llvm-main_f8aa6f1&diff-type=New) | [0 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=memcached_1.6.8_edonnag_llvm-main_1883de3&newcheck=memcached_1.6.8_edonnag_llvm-main_f8aa6f1&diff-type=Resolved) | tmux | [1 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=tmux_2.6_edonnag_llvm-main_1883de3&newcheck=tmux_2.6_edonnag_llvm-main_f8aa6f1&diff-type=New) | [0 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=tmux_2.6_edonnag_llvm-main_1883de3&newcheck=tmux_2.6_edonnag_llvm-main_f8aa6f1&diff-type=Resolved) | curl | [5 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=curl_curl-7_66_0_edonnag_llvm-main_1883de3&newcheck=curl_curl-7_66_0_edonnag_llvm-main_f8aa6f1&diff-type=New) | [2 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=curl_curl-7_66_0_edonnag_llvm-main_1883de3&newcheck=curl_curl-7_66_0_edonnag_llvm-main_f8aa6f1&diff-type=Resolved) | twin | [15 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=twin_v0.8.1_edonnag_llvm-main_1883de3&newcheck=twin_v0.8.1_edonnag_llvm-main_f8aa6f1&diff-type=New) | [2 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=twin_v0.8.1_edonnag_llvm-main_1883de3&newcheck=twin_v0.8.1_edonnag_llvm-main_f8aa6f1&diff-type=Resolved) | vim | [57 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=vim_v8.2.1920_edonnag_llvm-main_1883de3&newcheck=vim_v8.2.1920_edonnag_llvm-main_f8aa6f1&diff-type=New) | [1 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=vim_v8.2.1920_edonnag_llvm-main_1883de3&newcheck=vim_v8.2.1920_edonnag_llvm-main_f8aa6f1&diff-type=Resolved) | openssl | [21 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=openssl_openssl-3.0.0-alpha7_edonnag_llvm-main_1883de3&newcheck=openssl_openssl-3.0.0-alpha7_edonnag_llvm-main_f8aa6f1&diff-type=New) | [0 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=openssl_openssl-3.0.0-alpha7_edonnag_llvm-main_1883de3&newcheck=openssl_openssl-3.0.0-alpha7_edonnag_llvm-main_f8aa6f1&diff-type=Resolved) | sqlite | [16 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=sqlite_version-3.33.0_edonnag_llvm-main_1883de3&newcheck=sqlite_version-3.33.0_edonnag_llvm-main_f8aa6f1&diff-type=New) | [3 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=sqlite_version-3.33.0_edonnag_llvm-main_1883de3&newcheck=sqlite_version-3.33.0_edonnag_llvm-main_f8aa6f1&diff-type=Resolved) | ffmpeg | [187 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=ffmpeg_n4.3.1_edonnag_llvm-main_1883de3&newcheck=ffmpeg_n4.3.1_edonnag_llvm-main_f8aa6f1&diff-type=New) | [27 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=ffmpeg_n4.3.1_edonnag_llvm-main_1883de3&newcheck=ffmpeg_n4.3.1_edonnag_llvm-main_f8aa6f1&diff-type=Resolved) | postgres | [66 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=postgres_REL_13_0_edonnag_llvm-main_1883de3&newcheck=postgres_REL_13_0_edonnag_llvm-main_f8aa6f1&diff-type=New) | [6 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=postgres_REL_13_0_edonnag_llvm-main_1883de3&newcheck=postgres_REL_13_0_edonnag_llvm-main_f8aa6f1&diff-type=Resolved) | tinyxml2 | [1 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=tinyxml2_8.0.0_edonnag_llvm-main_1883de3&newcheck=tinyxml2_8.0.0_edonnag_llvm-main_f8aa6f1&diff-type=New) | [0 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=tinyxml2_8.0.0_edonnag_llvm-main_1883de3&newcheck=tinyxml2_8.0.0_edonnag_llvm-main_f8aa6f1&diff-type=Resolved) | libwebm | [23 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=libwebm_libwebm-1.0.0.27_edonnag_llvm-main_1883de3&newcheck=libwebm_libwebm-1.0.0.27_edonnag_llvm-main_f8aa6f1&diff-type=New) | [0 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=libwebm_libwebm-1.0.0.27_edonnag_llvm-main_1883de3&newcheck=libwebm_libwebm-1.0.0.27_edonnag_llvm-main_f8aa6f1&diff-type=Resolved) | xerces | [4 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=xerces_v3.2.3_edonnag_llvm-main_1883de3&newcheck=xerces_v3.2.3_edonnag_llvm-main_f8aa6f1&diff-type=New) | [2 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=xerces_v3.2.3_edonnag_llvm-main_1883de3&newcheck=xerces_v3.2.3_edonnag_llvm-main_f8aa6f1&diff-type=Resolved) | bitcoin | [9 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=bitcoin_v0.20.1_edonnag_llvm-main_1883de3&newcheck=bitcoin_v0.20.1_edonnag_llvm-main_f8aa6f1&diff-type=New) | [0 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=bitcoin_v0.20.1_edonnag_llvm-main_1883de3&newcheck=bitcoin_v0.20.1_edonnag_llvm-main_f8aa6f1&diff-type=Resolved) | protobuf | [10 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=protobuf_v3.13.0_edonnag_llvm-main_1883de3&newcheck=protobuf_v3.13.0_edonnag_llvm-main_f8aa6f1&diff-type=New) | [2 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=protobuf_v3.13.0_edonnag_llvm-main_1883de3&newcheck=protobuf_v3.13.0_edonnag_llvm-main_f8aa6f1&diff-type=Resolved) | qtbase | [69 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=qtbase_v6.2.0_edonnag_llvm-main_1883de3&newcheck=qtbase_v6.2.0_edonnag_llvm-main_f8aa6f1&diff-type=New) | [5 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=qtbase_v6.2.0_edonnag_llvm-main_1883de3&newcheck=qtbase_v6.2.0_edonnag_llvm-main_f8aa6f1&diff-type=Resolved) From 5f64fad88822dae0fd767a5d6d1531ffcbf656bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com> Date: Wed, 7 Feb 2024 12:13:26 +0100 Subject: [PATCH] [analyzer] Consolidate array bound checkers 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 | 5 + clang/docs/analyzer/checkers.rst | 133 ++++++++---------- .../clang/StaticAnalyzer/Checkers/Checkers.td | 67 ++++----- .../Checkers/ArrayBoundChecker.cpp | 92 ------------ .../Checkers/ArrayBoundCheckerV2.cpp | 50 ++++--- .../StaticAnalyzer/Checkers/CMakeLists.txt | 1 - .../Checkers/CStringChecker.cpp | 6 +- clang/lib/StaticAnalyzer/Core/MemRegion.cpp | 5 +- clang/test/Analysis/index-type.c | 4 +- clang/test/Analysis/misc-ps-region-store.m | 17 ++- clang/test/Analysis/no-outofbounds.c | 2 +- ...eck.c => out-of-bounds-constraint-check.c} | 2 +- .../test/Analysis/out-of-bounds-diagnostics.c | 2 +- clang/test/Analysis/out-of-bounds-new.cpp | 2 +- clang/test/Analysis/out-of-bounds-notes.c | 2 +- clang/test/Analysis/out-of-bounds.c | 2 +- clang/test/Analysis/outofbound-notwork.c | 8 +- clang/test/Analysis/outofbound.c | 28 ++-- clang/test/Analysis/rdar-6541136-region.c | 27 ---- clang/test/Analysis/runtime-regression.c | 2 +- .../test/Analysis/taint-diagnostic-visitor.c | 2 +- clang/test/Analysis/taint-generic.c | 4 +- clang/test/Analysis/taint-generic.cpp | 2 +- clang/www/analyzer/open_projects.html | 13 -- clang/www/analyzer/potential_checkers.html | 20 +-- .../lib/StaticAnalyzer/Checkers/BUILD.gn | 1 - 26 files changed, 178 insertions(+), 321 deletions(-) delete mode 100644 clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp rename clang/test/Analysis/{array-bound-v2-constraint-check.c => out-of-bounds-constraint-check.c} (97%) delete mode 100644 clang/test/Analysis/rdar-6541136-region.c diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index a220e57d0b3222..a3aad565472eb4 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -231,6 +231,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 diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index e093b2d672a74e..707067358fdfe3 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 1361da46c3c81d..9bf491eac1e0eb 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 " - "different 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 " + "different 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 c990ad138f8905..00000000000000 --- 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 6422933c8828a9..6f8d6dbd573f40 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 fcbe8b864b6e41..ccff5d0ac3b964 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 1a14f38e34f0e1..39dcaf02dbe258 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 different, 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 2c5cd2cf7630f6..c81bb632b83e94 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 997d45c1e5abad..818806c4aff3b5 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 a882e7eb0dc909..bd8b3350ba6da1 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 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 diff --git a/clang/test/Analysis/no-outofbounds.c b/clang/test/Analysis/no-outofbounds.c index 15155729067e9b..c6219ae74ab42c 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 91f748e655ce2d..df48c8c1707138 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 65bc28f58276fd..1db01251148e14 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 f541bdf810d79c..4e5442422bff42 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 391089b6a35d81..7256beb7e893eb 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 1f771c2b3bd138..734a56602e2aa4 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 cf2239cee1301c..1318c07bbf2a88 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 009cf33f613091..d3d8ff2b2f0ed6 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 f1a3a48a5fe4a8..00000000000000 --- 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 55988e9df5ee0f..9fa0017d5e4701 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 223df9951fd6b9..afe7117db71507 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 ad5a99fe8b3a35..3c520612c5d9ba 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 881c5baf889f6c..8836e1d3d2d986 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 a7c99c6e25bb3c..4d6e40f92f82c0 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 ad789b83e71b71..2543db251be127 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>different</h3> <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"> 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 e1c773630de701..c1bba99be3ba5c 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", _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
