Skip to content

Commit

Permalink
[analyzer][ReturnPtrRangeChecker] Fix a false positive on end() iterator
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Szelethus committed Nov 2, 2020
1 parent b3b993a commit 22e7182
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 2 deletions.
Expand Up @@ -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) {
Expand All @@ -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)"));

Expand Down
2 changes: 1 addition & 1 deletion clang/test/Analysis/misc-ps-region-store.m
Expand Up @@ -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}}
}

Expand Down
44 changes: 44 additions & 0 deletions clang/test/Analysis/return-ptr-range.cpp
Expand Up @@ -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();
}

0 comments on commit 22e7182

Please sign in to comment.