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

[Kristóf Umann via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG22e7182002b5: [analyzer][ReturnPtrRangeChecker] Fix a false positive on end() iterator (authored by Szelethus).

Changed prior to commit:
  https://reviews.llvm.org/D83678?vs=287881&id=302279#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83678/new/

https://reviews.llvm.org/D83678

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


Index: clang/test/Analysis/return-ptr-range.cpp
===================================================================
--- clang/test/Analysis/return-ptr-range.cpp
+++ clang/test/Analysis/return-ptr-range.cpp
@@ -25,3 +25,47 @@
   } 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();
+}
Index: clang/test/Analysis/misc-ps-region-store.m
===================================================================
--- clang/test/Analysis/misc-ps-region-store.m
+++ clang/test/Analysis/misc-ps-region-store.m
@@ -463,7 +463,7 @@
 
 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}}
 }
 
Index: clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
@@ -58,6 +58,11 @@
   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 @@
     // 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)"));
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D83678.302279.patch
Type: text/x-patch
Size: 3080 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201102/e6618938/attachment.bin>