balazske created this revision.
Herald added subscribers: manas, steakhal, ASDenysPetrov, martong, gamesh411,
dkrupp, donat.nagy, Szelethus, arphaman, 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.
The checker did not work with negative array index at return.
Specially for -1 it did not produce a bug.
`assumeInBound` may not work correct with negative values so a pre-
check is added to the checker for negative array index.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D107960
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
@@ -115,3 +115,24 @@
}
+int *test_negative_index(int I) {
+ static int arr[10]; // expected-note{{Original object declared here}}
+ // expected-note@-1{{'arr' initialized here}}
+ // expected-note@-2{{Original object declared here}}
+ // expected-note@-3{{'arr' initialized here}}
+ if (I == -1) // expected-note{{Assuming the condition is true}}
+ // expected-note@-1{{Taking true branch}}
+ // expected-note@-2{{Assuming the condition is false}}
+ // expected-note@-3{{Taking false branch}}
+ 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 -1}}
+ if (I == -2) // expected-note{{Assuming the condition is true}}
+ // expected-note@-1{{Taking true branch}}
+ 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 -2}}
+ if (I == 0)
+ return arr + I;
+ return arr + I;
+}
Index: clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
@@ -64,61 +64,79 @@
if (Idx == ElementCount)
return;
- ProgramStateRef StInBound = state->assumeInBound(Idx, ElementCount, true);
- ProgramStateRef StOutBound = state->assumeInBound(Idx, ElementCount, false);
- if (StOutBound && !StInBound) {
- ExplodedNode *N = C.generateErrorNode(StOutBound);
-
- if (!N)
+ auto ElementCountNL = ElementCount.getAs<NonLoc>();
+ assert(ElementCountNL && "Array size should not be a pointer");
+ auto IdxNegativeVal =
+ C.getSValBuilder()
+ .evalBinOpNN(state, BO_LT, *ElementCountNL,
+ C.getSValBuilder().makeZeroArrayIndex(),
+ C.getSValBuilder().getConditionType())
+ .castAs<DefinedOrUnknownSVal>();
+ ProgramStateRef IdxNegative, IdxNonNegative;
+ std::tie(IdxNegative, IdxNonNegative) = state->assume(IdxNegativeVal);
+
+ ProgramStateRef StError;
+ if (IdxNegative && !IdxNonNegative) {
+ StError = IdxNegative;
+ } else {
+ ProgramStateRef StInBound = state->assumeInBound(Idx, ElementCount, true);
+ ProgramStateRef StOutBound = state->assumeInBound(Idx, ElementCount, false);
+ if (StOutBound && !StInBound)
+ StError = StOutBound;
+ else
return;
+ }
- // FIXME: This bug correspond to CWE-466. Eventually we should have bug
- // types explicitly reference such exploit categories (when applicable).
- if (!BT)
- BT.reset(new BuiltinBug(
- this, "Buffer overflow",
- "Returned pointer value points outside the original object "
- "(potential buffer overflow)"));
-
- // Generate a report for this bug.
- auto Report =
- std::make_unique<PathSensitiveBugReport>(*BT, BT->getDescription(), N);
- Report->addRange(RetE->getSourceRange());
-
- const auto ConcreteElementCount = ElementCount.getAs<nonloc::ConcreteInt>();
- const auto ConcreteIdx = Idx.getAs<nonloc::ConcreteInt>();
-
- const auto *DeclR = ER->getSuperRegion()->getAs<DeclRegion>();
-
- 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()});
- }
+ ExplodedNode *N = C.generateErrorNode(StError);
- bugreporter::trackExpressionValue(N, RetE, *Report);
+ if (!N)
+ return;
- C.emitReport(std::move(Report));
+ // FIXME: This bug correspond to CWE-466. Eventually we should have bug
+ // types explicitly reference such exploit categories (when applicable).
+ if (!BT)
+ BT.reset(new BuiltinBug(
+ this, "Buffer overflow",
+ "Returned pointer value points outside the original object "
+ "(potential buffer overflow)"));
+
+ // Generate a report for this bug.
+ auto Report =
+ std::make_unique<PathSensitiveBugReport>(*BT, BT->getDescription(), N);
+ Report->addRange(RetE->getSourceRange());
+
+ const auto ConcreteElementCount = ElementCount.getAs<nonloc::ConcreteInt>();
+ const auto ConcreteIdx = Idx.getAs<nonloc::ConcreteInt>();
+
+ const auto *DeclR = ER->getSuperRegion()->getAs<DeclRegion>();
+
+ 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));
}
void ento::registerReturnPointerRangeChecker(CheckerManager &mgr) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
