[clang] 22e7182 - [analyzer][ReturnPtrRangeChecker] Fix a false positive on end() iterator

[Kirstóf Umann via cfe-commits]

Author: Kirstóf Umann
Date: 2020-11-02T16:41:17+01:00
New Revision: 22e7182002b5396539c69603b1c8c924b5f661e7

URL: https://github.com/llvm/llvm-project/commit/22e7182002b5396539c69603b1c8c924b5f661e7
DIFF: https://github.com/llvm/llvm-project/commit/22e7182002b5396539c69603b1c8c924b5f661e7.diff

LOG: [analyzer][ReturnPtrRangeChecker] Fix a false positive on end() iterator

ReturnPtrRange checker emits a report if a function returns a pointer which
points out of the buffer. However, end() iterator of containers is always such
a pointer, so this always results a false positive report. This false positive
case is now eliminated.

This patch resolves these tickets:
https://bugs.llvm.org/show_bug.cgi?id=20929
https://bugs.llvm.org/show_bug.cgi?id=25226
https://bugs.llvm.org/show_bug.cgi?id=27701

Patch by Tibor Brunner!

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

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
    clang/test/Analysis/misc-ps-region-store.m
    clang/test/Analysis/return-ptr-range.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
index 599d4f306aa1..1a94ccdc2825 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
@@ -58,6 +58,11 @@ void ReturnPointerRangeChecker::checkPreStmt(const ReturnStmt *RS,
   DefinedOrUnknownSVal ElementCount = getDynamicElementCount(
       state, ER->getSuperRegion(), C.getSValBuilder(), ER->getValueType());
 
+  // We assume that the location after the last element in the array is used as
+  // end() iterator. Reporting on these would return too many false positives.
+  if (Idx == ElementCount)
+    return;
+
   ProgramStateRef StInBound = state->assumeInBound(Idx, ElementCount, true);
   ProgramStateRef StOutBound = state->assumeInBound(Idx, ElementCount, false);
   if (StOutBound && !StInBound) {
@@ -70,7 +75,7 @@ void ReturnPointerRangeChecker::checkPreStmt(const ReturnStmt *RS,
     // types explicitly reference such exploit categories (when applicable).
     if (!BT)
       BT.reset(new BuiltinBug(
-          this, "Return of pointer value outside of expected range",
+          this, "Buffer overflow",
           "Returned pointer value points outside the original object "
           "(potential buffer overflow)"));
 

diff  --git a/clang/test/Analysis/misc-ps-region-store.m b/clang/test/Analysis/misc-ps-region-store.m
index b8710ff6f638..9c31324e7e59 100644
--- a/clang/test/Analysis/misc-ps-region-store.m
+++ b/clang/test/Analysis/misc-ps-region-store.m
@@ -463,7 +463,7 @@ void element_region_with_symbolic_superregion(int* p) {
 
 static int test_cwe466_return_outofbounds_pointer_a[10];
 int *test_cwe466_return_outofbounds_pointer() {
-  int *p = test_cwe466_return_outofbounds_pointer_a+10;
+  int *p = test_cwe466_return_outofbounds_pointer_a+11;
   return p; // expected-warning{{Returned pointer value points outside the original object}}
 }
 

diff  --git a/clang/test/Analysis/return-ptr-range.cpp b/clang/test/Analysis/return-ptr-range.cpp
index dd5dcd5d5d19..9763b51264e6 100644
--- a/clang/test/Analysis/return-ptr-range.cpp
+++ b/clang/test/Analysis/return-ptr-range.cpp
@@ -25,3 +25,47 @@ int *test_element_index_lifetime_with_local_ptr() {
   } while (0);
   return local_ptr; // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow)}}
 }
+
+template <typename T, int N>
+T* end(T (&arr)[N]) {
+  return arr + N; // no-warning, because we want to avoid false positives on returning the end() iterator of a container.
+}
+
+void get_end_of_array() {
+  static int arr[10];
+  end(arr);
+}
+
+template <int N>
+class Iterable {
+  int buffer[N];
+  int *start, *finish;
+
+public:
+  Iterable() : start(buffer), finish(buffer + N) {}
+
+  int* begin() { return start; }
+  int* end() { return finish; }
+};
+
+void use_iterable_object() {
+  Iterable<20> iter;
+  iter.end();
+}
+
+template <int N>
+class BadIterable {
+  int buffer[N];
+  int *start, *finish;
+
+public:
+  BadIterable() : start(buffer), finish(buffer + N) {}
+
+  int* begin() { return start; }
+  int* end() { return finish + 1; } // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow)}}
+};
+
+void use_bad_iterable_object() {
+  BadIterable<20> iter;
+  iter.end();
+}