diff --git a/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp +++ b/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(); + auto ConcreteIdx = Idx.getAs(); + + 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(*BT, BT->getDescription(), N); - + auto report = std::make_unique(*BT, SBuf, N); report->addRange(RetE->getSourceRange()); + + if (const auto DeclR = ER->getSuperRegion()->getAs()) { + 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)); } } diff --git a/clang/test/Analysis/return-ptr-range.cpp b/clang/test/Analysis/return-ptr-range.cpp --- a/clang/test/Analysis/return-ptr-range.cpp +++ b/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 @@ -55,17 +68,46 @@ template 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)}} }