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,16 +80,44 @@ "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. - // Generate a report for this bug. - auto report = + auto Report = std::make_unique(*BT, BT->getDescription(), N); - - report->addRange(RetE->getSourceRange()); - C.emitReport(std::move(report)); + Report->addRange(RetE->getSourceRange()); + + const auto ConcreteElementCount = ElementCount.getAs(); + const auto ConcreteIdx = Idx.getAs(); + + const auto *DeclR = ER->getSuperRegion()->getAs(); + + if (DeclR) + Report->addNote("Original object declared here", + {DeclR->getDecl(), C.getSourceManager()}); + + if (ConcreteElementCount) { + SmallString<128> SBuf; + llvm::raw_svector_ostream OS(SBuf); + OS << "Original object "; + if (DeclR) { + OS << "'"; + DeclR->getDecl()->printName(OS); + OS << "' "; + } + OS << "is an array of " << ConcreteElementCount->getValue() << " '"; + ER->getValueType().print(OS, + PrintingPolicy(C.getASTContext().getLangOpts())); + OS << "' objects"; + if (ConcreteIdx) { + OS << ", returned pointer points at index " << ConcreteIdx->getValue(); + } + + Report->addNote(SBuf, + {RetE, C.getSourceManager(), C.getLocationContext()}); + } + + bugreporter::trackExpressionValue(N, RetE, *Report); + + C.emitReport(std::move(Report)); } } diff --git a/clang/test/Analysis/misc-ps-region-store.m b/clang/test/Analysis/misc-ps-region-store.m --- a/clang/test/Analysis/misc-ps-region-store.m +++ b/clang/test/Analysis/misc-ps-region-store.m @@ -461,10 +461,11 @@ // Test returning an out-of-bounds pointer (CWE-466) //===----------------------------------------------------------------------===// -static int test_cwe466_return_outofbounds_pointer_a[10]; +static int test_cwe466_return_outofbounds_pointer_a[10]; // expected-note{{Original object declared here}} int *test_cwe466_return_outofbounds_pointer() { int *p = test_cwe466_return_outofbounds_pointer_a+11; return p; // expected-warning{{Returned pointer value points outside the original object}} + // expected-note@-1{{Original object 'test_cwe466_return_outofbounds_pointer_a' is an array of 10 'int' objects, returned pointer points at index 11}} } //===----------------------------------------------------------------------===// 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,39 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.security.ReturnPtrRange -verify %s - -int arr[10]; -int *ptr; +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.security.ReturnPtrRange -analyzer-output text -verify %s int conjure_index(); -int *test_element_index_lifetime() { - do { +namespace test_element_index_lifetime { + +int arr[10]; // expected-note{{Original object declared here}} expected-note{{Original object declared here}} +int *ptr; + +int *test_global_ptr() { + do { // expected-note{{Loop condition is false. Exiting loop}} int x = conjure_index(); - ptr = arr + x; - if (x != 20) + ptr = arr + x; // expected-note{{Value assigned to 'ptr'}} + if (x != 20) // expected-note{{Assuming 'x' is equal to 20}} + // expected-note@-1{{Taking false branch}} return arr; // no-warning - } while (0); - return ptr; // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow)}} + } while(0); + return ptr; // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow) [alpha.security.ReturnPtrRange]}} + // expected-note@-1{{Returned pointer value points outside the original object (potential buffer overflow)}} + // expected-note@-2{{Original object 'arr' is an array of 10 'int' objects}} } -int *test_element_index_lifetime_with_local_ptr() { +int *test_local_ptr() { int *local_ptr; - do { + do { // expected-note{{Loop condition is false. Exiting loop}} int x = conjure_index(); - local_ptr = arr + x; - if (x != 20) + local_ptr = arr + x; // expected-note{{Value assigned to 'local_ptr'}} + if (x != 20) // expected-note{{Assuming 'x' is equal to 20}} + // expected-note@-1{{Taking false branch}} return arr; // no-warning - } while (0); - return local_ptr; // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow)}} + } while(0); + return local_ptr; // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow) [alpha.security.ReturnPtrRange]}} + // expected-note@-1{{Returned pointer value points outside the original object (potential buffer overflow)}} + // expected-note@-2{{Original object 'arr' is an array of 10 'int' objects}} +} + } template @@ -55,17 +65,53 @@ template class BadIterable { - int buffer[N]; + int buffer[N]; // expected-note{{Original object declared here}} int *start, *finish; public: - BadIterable() : start(buffer), finish(buffer + N) {} + BadIterable() : start(buffer), finish(buffer + N) {} // expected-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 points outside the original object}} + // expected-note@-1{{Returned pointer value points outside the original object}} + // expected-note@-2{{Original object 'buffer' is an array of 20 'int' objects, returned pointer points at index 21}} }; void use_bad_iterable_object() { - BadIterable<20> iter; - iter.end(); + BadIterable<20> iter; // expected-note{{Calling default constructor for 'BadIterable<20>'}} + // expected-note@-1{{Returning from default constructor for 'BadIterable<20>'}} + iter.end(); // expected-note{{Calling 'BadIterable::end'}} } + +int *test_idx_sym(int I) { + static int arr[10]; // expected-note{{Original object declared here}} expected-note{{'arr' initialized here}} + + if (I != 11) // expected-note{{Assuming 'I' is equal to 11}} + // expected-note@-1{{Taking false branch}} + return arr; + return arr + I; // expected-warning{{Returned pointer value points outside the original object}} + // expected-note@-1{{Returned pointer value points outside the original object}} + // expected-note@-2{{Original object 'arr' is an array of 10 'int' objects, returned pointer points at index 11}} +} + +namespace test_array_of_struct { + +struct Data { + int A; + char *B; +}; + +Data DataArr[10]; // expected-note{{Original object declared here}} + +Data *test_struct_array() { + int I = conjure_index(); + if (I != 11) // expected-note{{Assuming 'I' is equal to 11}} + // expected-note@-1{{Taking false branch}} + return DataArr; + return DataArr + I; // expected-warning{{Returned pointer value points outside the original object}} + // expected-note@-1{{Returned pointer value points outside the original object}} + // expected-note@-2{{Original object 'DataArr' is an array of 10 'test_array_of_struct::Data' objects, returned pointer points at index 11}} +} + +} +