balazske created this revision.
Herald added subscribers: manas, steakhal, ASDenysPetrov, martong, gamesh411,
dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet,
baloghadamsoftware, xazax.hun, whisperity.
Herald added a reviewer: Szelethus.
balazske requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Add some notes and track of bad return value.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D107051
Files:
clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
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
@@ -1,29 +1,42 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.security.ReturnPtrRange -verify %s
+// RUN1: %clang_analyze_cc1 -analyzer-checker=alpha.security.ReturnPtrRange -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.security.ReturnPtrRange -analyzer-output text -verify=notes %s
-int arr[10];
+
+#include "Inputs/system-header-simulator.h"
+void *malloc(size_t);
+
+int arr1[10]; // notes-note{{Memory object declared here}}
+int arr2[10]; // notes-note{{Memory object declared here}}
+int arr3[10]; // notes-note{{Memory object declared here}}
int *ptr;
int conjure_index();
int *test_element_index_lifetime() {
- do {
+ do { // notes-note{{Loop condition is false. Exiting loop}}
int x = conjure_index();
- ptr = arr + x;
- if (x != 20)
- return arr; // no-warning
+ ptr = arr1 + x; // notes-note{{Value assigned to 'ptr'}}
+ if (x != 20) // notes-note{{Assuming 'x' is equal to 20}}
+ // notes-note@-1{{Taking false branch}}
+ return arr1; // no-warning
} while (0);
- return ptr; // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow)}}
+ return ptr; // expected-warning{{Returned pointer value points outside the original object with size of 10 'int' objects (potential buffer overflow) [alpha.security.ReturnPtrRange]}}
+ // notes-warning@-1{{Returned pointer value points outside the original object with size of 10 'int' objects (potential buffer overflow) [alpha.security.ReturnPtrRange]}}
+ // notes-note@-2{{Returned pointer value points outside the original object with size of 10 'int' objects (potential buffer overflow)}}
}
int *test_element_index_lifetime_with_local_ptr() {
int *local_ptr;
- do {
+ do { // notes-note{{Loop condition is false. Exiting loop}}
int x = conjure_index();
- local_ptr = arr + x;
- if (x != 20)
- return arr; // no-warning
+ local_ptr = arr2 + x; // notes-note{{Value assigned to 'local_ptr'}}
+ if (x != 20) // notes-note{{Assuming 'x' is equal to 20}}
+ // notes-note@-1{{Taking false branch}}
+ return arr2; // no-warning
} while (0);
- return local_ptr; // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow)}}
+ return local_ptr; // expected-warning{{Returned pointer value points outside the original object with size of 10 'int' objects (potential buffer overflow) [alpha.security.ReturnPtrRange]}}
+ // notes-warning@-1{{Returned pointer value points outside the original object with size of 10 'int' objects (potential buffer overflow) [alpha.security.ReturnPtrRange]}}
+ // notes-note@-2{{Returned pointer value points outside the original object with size of 10 'int' objects (potential buffer overflow)}}
}
template <typename T, int N>
@@ -55,17 +68,46 @@
template <int N>
class BadIterable {
- int buffer[N];
+ int buffer[N]; // notes-note{{Memory object declared here}}
int *start, *finish;
public:
- BadIterable() : start(buffer), finish(buffer + N) {}
+ BadIterable() : start(buffer), finish(buffer + N) {} // notes-note{{Value assigned to 'iter.finish'}}
int* begin() { return start; }
- int* end() { return finish + 1; } // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow)}}
+ int* end() { return finish + 1; } // expected-warning{{Returned pointer value with index 21 points outside the original object with size of 20 'int' objects (potential buffer overflow) [alpha.security.ReturnPtrRange]}}
+ // notes-warning@-1{{Returned pointer value with index 21 points outside the original object with size of 20 'int' objects (potential buffer overflow) [alpha.security.ReturnPtrRange]}}
+ // notes-note@-2{{Returned pointer value with index 21 points outside the original object with size of 20 'int' objects (potential buffer overflow)}}
};
void use_bad_iterable_object() {
- BadIterable<20> iter;
- iter.end();
+ BadIterable<20> iter; // notes-note{{Calling default constructor for 'BadIterable<20>'}}
+ // notes-note@-1{{Returning from default constructor for 'BadIterable<20>'}}
+ iter.end(); // notes-note{{Calling 'BadIterable::end'}}
+}
+
+int *test_idx_sym(int I) {
+ if (I != 11) // notes-note{{Assuming 'I' is equal to 11}}
+ // notes-note@-1{{Taking false branch}}
+ return arr3;
+ return arr3 + I; // expected-warning{{Returned pointer value with index 11 points outside the original object with size of 10 'int' objects (potential buffer overflow) [alpha.security.ReturnPtrRange]}}
+ // notes-warning@-1{{Returned pointer value with index 11 points outside the original object with size of 10 'int' objects (potential buffer overflow) [alpha.security.ReturnPtrRange]}}
+ // notes-note@-2{{Returned pointer value with index 11 points outside the original object with size of 10 'int' objects (potential buffer overflow)}}
+}
+
+struct Data {
+ int A;
+ char *B;
+};
+
+Data DataArr[10]; // notes-note{{Memory object declared here}}
+
+Data *test_struct_array() {
+ int I = conjure_index();
+ if (I != 11) // notes-note{{Assuming 'I' is equal to 11}}
+ // notes-note@-1{{Taking false branch}}
+ return DataArr;
+ return DataArr + I; // expected-warning{{Returned pointer value with index 11 points outside the original object with size of 10 'Data' objects (potential buffer overflow) [alpha.security.ReturnPtrRange]}}
+ // notes-warning@-1{{Returned pointer value with index 11 points outside the original object with size of 10 'Data' objects (potential buffer overflow) [alpha.security.ReturnPtrRange]}}
+ // notes-note@-2{{Returned pointer value with index 11 points outside the original object with size of 10 'Data' objects (potential buffer overflow)}}
}
Index: clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
@@ -12,6 +12,7 @@
//===----------------------------------------------------------------------===//
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
@@ -79,15 +80,36 @@
"Returned pointer value points outside the original object "
"(potential buffer overflow)"));
- // 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.
+ auto ConcreteElementCount = ElementCount.getAs<nonloc::ConcreteInt>();
+ auto ConcreteIdx = Idx.getAs<nonloc::ConcreteInt>();
+
+ SmallString<128> SBuf;
+ llvm::raw_svector_ostream OS(SBuf);
+ OS << "Returned pointer value";
+ if (ConcreteIdx) {
+ OS << " with index " << ConcreteIdx->getValue();
+ }
+ OS << " points outside the original object";
+ if (ConcreteElementCount) {
+ OS << " with size of " << ConcreteElementCount->getValue() << " '";
+ ER->getValueType().print(OS,
+ PrintingPolicy(C.getASTContext().getLangOpts()));
+ OS << "' objects";
+ }
+ OS << " (potential buffer overflow)";
// Generate a report for this bug.
- auto report =
- std::make_unique<PathSensitiveBugReport>(*BT, BT->getDescription(), N);
-
+ auto report = std::make_unique<PathSensitiveBugReport>(*BT, SBuf, N);
report->addRange(RetE->getSourceRange());
+
+ if (const auto DeclR = ER->getSuperRegion()->getAs<DeclRegion>()) {
+ const Decl *RegD = DeclR->getDecl();
+ PathDiagnosticLocation L{RegD, C.getSourceManager()};
+ report->addNote("Memory object declared here", L);
+ }
+
+ bugreporter::trackExpressionValue(N, RetE, *report);
+
C.emitReport(std::move(report));
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
