[clang] 3e01403 - [analyzer] Improve underflow handling in ArrayBoundV2

[Donát Nagy via cfe-commits]

Author: Donát Nagy
Date: 2023-08-21T17:17:02+02:00
New Revision: 3e014038b373e5a4a96d89d46cea17e4d2456a04

URL: https://github.com/llvm/llvm-project/commit/3e014038b373e5a4a96d89d46cea17e4d2456a04
DIFF: https://github.com/llvm/llvm-project/commit/3e014038b373e5a4a96d89d46cea17e4d2456a04.diff

LOG: [analyzer] Improve underflow handling in ArrayBoundV2

This minor change ensures that underflow errors are reported on memory
regions that are in unknown space but have a well-defined beginning.

As a concrete example, the following test case did not produce a warning
previously, but will produce a warning after this patch:

  typedef struct {
    int id;
    char name[256];
  } user_t;

  user_t *get_symbolic_user(void);
  char test_underflow_symbolic_2() {
    user_t *user = get_symbolic_user();
    return user->name[-1];
  }

Differential Revision: https://reviews.llvm.org/D157104

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
    clang/test/Analysis/out-of-bounds.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
index 269277aaf357f0..93da34ed5da4c5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -172,13 +172,18 @@ void ArrayBoundCheckerV2::checkLocation(SVal location, bool isLoad,
     return;
 
   NonLoc ByteOffset = RawOffset->getByteOffset();
+  const SubRegion *Reg = RawOffset->getRegion();
 
   // CHECK LOWER BOUND
-  const MemSpaceRegion *SR = RawOffset->getRegion()->getMemorySpace();
-  if (!llvm::isa<UnknownSpaceRegion>(SR)) {
-    // A pointer to UnknownSpaceRegion may point to the middle of
-    // an allocated region.
-
+  const MemSpaceRegion *Space = Reg->getMemorySpace();
+  if (!(isa<SymbolicRegion>(Reg) && isa<UnknownSpaceRegion>(Space))) {
+    // A symbolic region in unknown space represents an unknown pointer that
+    // may point into the middle of an array, so we don't look for underflows.
+    // Both conditions are significant because we want to check underflows in
+    // symbolic regions on the heap (which may be introduced by checkers like
+    // MallocChecker that call SValBuilder::getConjuredHeapSymbolVal()) and
+    // non-symbolic regions (e.g. a field subregion of a symbolic region) in
+    // unknown space.
     auto [state_precedesLowerBound, state_withinLowerBound] =
         compareValueToThreshold(state, ByteOffset,
                                 svalBuilder.makeZeroArrayIndex(), svalBuilder);
@@ -194,8 +199,7 @@ void ArrayBoundCheckerV2::checkLocation(SVal location, bool isLoad,
   }
 
   // CHECK UPPER BOUND
-  DefinedOrUnknownSVal Size =
-      getDynamicExtent(state, RawOffset->getRegion(), svalBuilder);
+  DefinedOrUnknownSVal Size = getDynamicExtent(state, Reg, svalBuilder);
   if (auto KnownSize = Size.getAs<NonLoc>()) {
     auto [state_withinUpperBound, state_exceedsUpperBound] =
         compareValueToThreshold(state, ByteOffset, *KnownSize, svalBuilder);

diff  --git a/clang/test/Analysis/out-of-bounds.c b/clang/test/Analysis/out-of-bounds.c
index edf70085e18fae..cf8e60f66ac063 100644
--- a/clang/test/Analysis/out-of-bounds.c
+++ b/clang/test/Analysis/out-of-bounds.c
@@ -151,11 +151,23 @@ void test_assume_after_access(unsigned long x) {
 // Don't warn when indexing below the start of a symbolic region's whose
 // base extent we don't know.
 int *get_symbolic(void);
-void test_index_below_symboloc(void) {
+void test_underflow_symbolic(void) {
   int *buf = get_symbolic();
   buf[-1] = 0; // no-warning;
 }
 
+// But warn if we understand the internal memory layout of a symbolic region.
+typedef struct {
+  int id;
+  char name[256];
+} user_t;
+
+user_t *get_symbolic_user(void);
+char test_underflow_symbolic_2() {
+  user_t *user = get_symbolic_user();
+  return user->name[-1]; // expected-warning{{Out of bound memory access}}
+}
+
 void test_incomplete_struct(void) {
   extern struct incomplete incomplete;
   int *p = (int *)&incomplete;