balazske created this revision.
Herald added subscribers: cfe-commits, martong, Charusso, gamesh411, Szelethus,
dkrupp.
Herald added a reviewer: Szelethus.
Herald added a project: clang.
balazske requested review of this revision.
This is a testing version for the checker that is implemented in
D72705 <https://reviews.llvm.org/D72705>. It is extended to make it usable on
large projects.
This change it not intended to land, it is here only for
pre-evaluating the code.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D88312
Files:
clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp
clang/test/Analysis/error-return.c
Index: clang/test/Analysis/error-return.c
===================================================================
--- clang/test/Analysis/error-return.c
+++ clang/test/Analysis/error-return.c
@@ -91,7 +91,8 @@
}
void badcheck(int X) {
- if (X == 0) { } // expected-warning{{Use of return value that was not checked}}
+ if (X == 0) {
+ } // expected-warning{{Use of return value that was not checked}}
}
void test_EOFOrNeg_Call() {
Index: clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp
@@ -54,15 +54,23 @@
public:
/// Test if an encountered binary operator where the return value is involved
/// is a valid check statement. The return value appears in one side of the
- /// operator (`ChildIsLHS` indicates if it is on the LHS). If the other side
- /// contains a known (mostly constant) value, it is already calculated in
- /// `KnownValue`. `RetTy` is the type of the return value (return type of the
- /// function call in the code to check).
+ /// operator (`ChildIsLHS` indicates if it is on the LHS). The other side is
+ /// the value to test against, its "known value" is passed in
+ /// `ValueToTestAgainst. `RetTy` is the return type of the function.
+ // testAppearanceInBinOp
virtual bool testBinOpForCheckStatement(BasicValueFactory &BVF,
const BinaryOperator *BinOp,
- const llvm::APSInt *KnownValue,
+ const llvm::APSInt *ValueToTestAgainst,
QualType RetTy,
bool ChildIsLHS) const = 0;
+
+ virtual bool testUnaryOp(BasicValueFactory &BVF,
+ const UnaryOperator *UnOp,
+ QualType RetTy) const = 0;
+
+ /// Tell if the bare appearance of the return value in place of a condition (for example 'X' in 'if (X)') can be taken as a check for error return value.
+ // testAppearanceInCondition
+ virtual bool testConditionForCheckStatement() const = 0;
};
/// Error return is a -1 or any negative value (both is accepted).
@@ -73,28 +81,28 @@
public:
bool testBinOpForCheckStatement(BasicValueFactory &BVF,
const BinaryOperator *BinOp,
- const llvm::APSInt *KnownValue,
+ const llvm::APSInt *ValueToTestAgainst,
QualType RetTy,
bool ChildIsLHS) const override {
- if (!KnownValue)
- return false;
+ if (!ValueToTestAgainst)
+ return true;
- bool KnownNull = KnownValue->isNullValue();
- bool KnownEOF = ((*KnownValue) == BVF.getValue(-1, RetTy));
+ bool NullFound = ValueToTestAgainst->isNullValue();
+ bool EOFFound = ((*ValueToTestAgainst) == BVF.getValue(-1, RetTy));
if (ChildIsLHS) {
switch (BinOp->getOpcode()) {
case BO_EQ: // 'X == -1'
case BO_NE: // 'X != -1'
- return KnownEOF;
+ return EOFFound;
case BO_LT: // 'X < 0'
- return KnownNull;
+ return NullFound;
case BO_GE: // 'X >= 0'
- return KnownNull;
+ return NullFound;
case BO_LE: // 'X <= -1'
- return KnownEOF;
+ return EOFFound;
case BO_GT: // 'X > -1'
- return KnownEOF;
+ return EOFFound;
default:
return false;
}
@@ -102,21 +110,77 @@
switch (BinOp->getOpcode()) {
case BO_EQ: // '-1 == X'
case BO_NE: // '-1 != X'
- return KnownEOF;
+ return EOFFound;
case BO_GT: // '0 > X'
- return KnownNull;
+ return NullFound;
case BO_LE: // '0 <= X'
- return KnownNull;
+ return NullFound;
case BO_GE: // '-1 >= X'
- return KnownEOF;
+ return EOFFound;
case BO_LT: // '-1 < X'
- return KnownEOF;
+ return EOFFound;
default:
return false;
}
}
return false;
}
+
+ bool testUnaryOp(BasicValueFactory &BVF, const UnaryOperator *UnOp,
+ QualType RetTy) const override {
+ return false;
+ }
+
+ bool testConditionForCheckStatement() const override { return false; }
+};
+
+class NullErrorReturn : public ErrorReturnCheckKind {
+public:
+ bool testBinOpForCheckStatement(BasicValueFactory &BVF,
+ const BinaryOperator *BinOp,
+ const llvm::APSInt *ValueToTestAgainst,
+ QualType RetTy,
+ bool ChildIsLHS) const override {
+ clang::BinaryOperator::Opcode C = BinOp->getOpcode();
+
+ if (C == BO_LAnd || C == BO_LOr)
+ return true;
+
+ if (!ValueToTestAgainst)
+ return false;
+ bool NullFound = ValueToTestAgainst->isNullValue();
+
+ if (C == BO_EQ || C == BO_NE)
+ return NullFound;
+
+ /*if (ChildIsLHS) {
+ switch (BinOp->getOpcode()) {
+ case BO_EQ: // 'X == 0'
+ case BO_NE: // 'X != 0'
+ return KnownNull;
+ default:
+ return false;
+ }
+ } else {
+ switch (BinOp->getOpcode()) {
+ case BO_EQ: // '0 == X'
+ case BO_NE: // '0 != X'
+ return KnownNull;
+ default:
+ return false;
+ }
+ }*/
+ return false;
+ }
+
+ bool testUnaryOp(BasicValueFactory &BVF, const UnaryOperator *UnOp,
+ QualType RetTy) const override {
+ if (UnOp->getOpcode() == UnaryOperator::Opcode::UO_LNot)
+ return true;
+ return false;
+ }
+
+ bool testConditionForCheckStatement() const override { return true; }
};
/// Description of an API function to check.
@@ -171,7 +235,7 @@
check::PointerEscape> {
mutable std::unique_ptr<BuiltinBug> BT_UncheckedUse;
- void checkAccess(CheckerContext &C, ProgramStateRef State, const Stmt *LoadS,
+ ExplodedNode *checkAccess(CheckerContext &C, ProgramStateRef State, const Stmt *LoadS,
SymbolRef CallSym, const CalledFunctionData *CFD) const;
ProgramStateRef processEscapedParams(CheckerContext &C, const CallEvent &Call,
ProgramStateRef State) const;
@@ -190,11 +254,68 @@
private:
EOFOrNegativeErrorReturn CheckForEOFOrNegative;
+ NullErrorReturn CheckForNull;
+ NullErrorReturn CheckForZero;
CallDescriptionMap<FnInfo> CheckedFunctions = {
{{"fputs", 2}, FnInfo{&CheckForEOFOrNegative}},
{{"fputws", 2}, FnInfo{&CheckForEOFOrNegative}},
+
+ {{"aligned_alloc", 2}, FnInfo{&CheckForNull}},
+ {{"bsearch", 5}, FnInfo{&CheckForNull}},
+ {{"bsearch_s", 6}, FnInfo{&CheckForNull}},
+ {{"calloc", 2}, FnInfo{&CheckForNull}},
+ {{"fgets", 3}, FnInfo{&CheckForNull}},
+ {{"fopen", 2}, FnInfo{&CheckForNull}},
+ {{"freopen", 3}, FnInfo{&CheckForNull}},
+ {{"getenv", 1}, FnInfo{&CheckForNull}},
+ {{"getenv_s", 4}, FnInfo{&CheckForNull}},
+ {{"gets_s", 2}, FnInfo{&CheckForNull}},
+ {{"gmtime", 1}, FnInfo{&CheckForNull}},
+ {{"gmtime_s", 2}, FnInfo{&CheckForNull}},
+ {{"localtime", 1}, FnInfo{&CheckForNull}},
+ {{"localtime_s", 2}, FnInfo{&CheckForNull}},
+ {{"malloc", 1}, FnInfo{&CheckForNull}},
+ {{"memchr", 3}, FnInfo{&CheckForNull}},
+ {{"realloc", 2}, FnInfo{&CheckForNull}},
+ {{"setlocale", 2}, FnInfo{&CheckForNull}},
+ {{"strchr", 2}, FnInfo{&CheckForNull}},
+ {{"strpbrk", 2}, FnInfo{&CheckForNull}},
+ {{"strrchr", 2}, FnInfo{&CheckForNull}},
+ {{"strstr", 2}, FnInfo{&CheckForNull}},
+ {{"strtok", 2}, FnInfo{&CheckForNull}},
+ {{"strtok_s", 4}, FnInfo{&CheckForNull}},
+ {{"tmpfile", 0}, FnInfo{&CheckForNull}},
+ {{"tmpnam", 1}, FnInfo{&CheckForNull}},
+ {{"wcschr", 2}, FnInfo{&CheckForNull}},
+ {{"wcspbrk", 2}, FnInfo{&CheckForNull}},
+ {{"wcsrchr", 2}, FnInfo{&CheckForNull}},
+ {{"wcsstr", 2}, FnInfo{&CheckForNull}},
+ {{"wcstok", 3}, FnInfo{&CheckForNull}},
+ {{"wcstok_s", 4}, FnInfo{&CheckForNull}},
+ {{"wmemchr", 3}, FnInfo{&CheckForNull}},
+
+ {{"asctime_s", 3}, FnInfo{&CheckForZero}},
+ {{"at_quick_exit", 1}, FnInfo{&CheckForZero}},
+ {{"atexit", 1}, FnInfo{&CheckForZero}},
+ {{"ctime_s", 3}, FnInfo{&CheckForZero}},
+ {{"fgetpos", 2}, FnInfo{&CheckForZero}},
+ {{"fopen_s", 3}, FnInfo{&CheckForZero}},
+ {{"freopen_s", 4}, FnInfo{&CheckForZero}},
+ {{"fsetpos", 2}, FnInfo{&CheckForZero}},
+ {{"raise", 1}, FnInfo{&CheckForZero}},
+ {{"remove", 1}, FnInfo{&CheckForZero}},
+ {{"rename", 2}, FnInfo{&CheckForZero}},
+ {{"setvbuf", 4}, FnInfo{&CheckForZero}},
+ {{"strerror_s", 3}, FnInfo{&CheckForZero}},
+ {{"strftime", 4}, FnInfo{&CheckForZero}},
+ {{"timespec_get", 2}, FnInfo{&CheckForZero}},
+ {{"tmpfile_s", 1}, FnInfo{&CheckForZero}},
+ {{"tmpnam_s", 2}, FnInfo{&CheckForZero}},
+ {{"wctrans", 1}, FnInfo{&CheckForZero}},
+ {{"wctype", 1}, FnInfo{&CheckForZero}}
};
+
};
/// Result of the ErrorCheckTestStmtVisitor.
@@ -277,6 +398,20 @@
return NoCheckUseFound;
}
+ VisitResult VisitUnaryOperator(const UnaryOperator *UO) {
+ if (CFD->Info->ErrorReturnKind->testUnaryOp(
+ C.getSValBuilder().getBasicValueFactory(), UO,
+ CFD->Info->RetTy))
+ return CheckFound;
+ return NoCheckUseFound;
+ }
+
+ VisitResult VisitAbstractConditionalOperator(const AbstractConditionalOperator *ACO) {
+ if (isChildOf(ACO->getCond()))
+ return CFD->Info->ErrorReturnKind->testConditionForCheckStatement() ? CheckFound : NoCheckUseFound;
+ return ContinueAtParentStmt;
+ }
+
VisitResult VisitCallExpr(const CallExpr *CE) {
const FunctionDecl *CalledF = C.getCalleeDecl(CE);
SourceLocation Loc = CalledF->getLocation();
@@ -302,26 +437,27 @@
VisitResult VisitIfStmt(const IfStmt *S) {
if (isChildOf(S->getCond()))
- return NoCheckUseFound;
+ return CFD->Info->ErrorReturnKind->testConditionForCheckStatement() ? CheckFound : NoCheckUseFound;
return StopExamineNoError;
}
VisitResult VisitForStmt(const ForStmt *S) {
- if (isChildOf(S->getInit()) || isChildOf(S->getCond()) ||
- isChildOf(S->getInc()))
- return NoCheckUseFound;
+// if (isChildOf(S->getInit()) || isChildOf(S->getInc()))
+// return
+ if (isChildOf(S->getCond()))
+ return CFD->Info->ErrorReturnKind->testConditionForCheckStatement() ? CheckFound : NoCheckUseFound;
return StopExamineNoError;
}
VisitResult VisitDoStmt(const DoStmt *S) {
if (isChildOf(S->getCond()))
- return NoCheckUseFound;
+ return CFD->Info->ErrorReturnKind->testConditionForCheckStatement() ? CheckFound : NoCheckUseFound;
return StopExamineNoError;
}
VisitResult VisitWhileStmt(const WhileStmt *S) {
if (isChildOf(S->getCond()))
- return NoCheckUseFound;
+ return CFD->Info->ErrorReturnKind->testConditionForCheckStatement() ? CheckFound : NoCheckUseFound;
return StopExamineNoError;
}
@@ -336,7 +472,7 @@
REGISTER_MAP_WITH_PROGRAMSTATE(CalledFunctionDataMap, SymbolRef,
CalledFunctionData)
-void ErrorReturnChecker::checkAccess(CheckerContext &C, ProgramStateRef State,
+ExplodedNode *ErrorReturnChecker::checkAccess(CheckerContext &C, ProgramStateRef State,
const Stmt *LoadS, SymbolRef CallSym,
const CalledFunctionData *CFD) const {
const ParentMap &PM = C.getLocationContext()->getParentMap();
@@ -359,15 +495,13 @@
SourceRange CallLocation = CFD->CallLocation;
State = State->remove<CalledFunctionDataMap>(CallSym);
- ExplodedNode *N = C.generateNonFatalErrorNode(State);
- if (!N) {
- C.addTransition(State);
- return;
- }
+ ExplodedNode *ErrN = C.generateNonFatalErrorNode(State);
+ if (!ErrN)
+ return C.addTransition(State);
auto Report = std::make_unique<PathSensitiveBugReport>(
- *BT_UncheckedUse, BT_UncheckedUse->getDescription(), N);
- // Report->markInteresting(CallSym);
+ *BT_UncheckedUse, BT_UncheckedUse->getDescription(), ErrN);
+ Report->markInteresting(CallSym);
Report->addRange(CallLocation);
C.emitReport(std::move(Report));
@@ -375,13 +509,12 @@
//Report->addRange(CallLocation);
//C.emitReport(std::move(Report));
- return;
+ return nullptr;
}
case CheckFound:
// A correct error check was found, remove from state.
State = State->remove<CalledFunctionDataMap>(CallSym);
- C.addTransition(State);
- return;
+ return C.addTransition(State);
case ContinueAtParentStmt:
// Continue checking at upper level (check with result of assignment).
@@ -389,10 +522,10 @@
continue;
case StopExamineNoError:
- C.addTransition(State);
- return;
+ return C.addTransition(State);
};
}
+ return C.addTransition(State);
}
ProgramStateRef ErrorReturnChecker::processEscapedParams(
@@ -446,6 +579,21 @@
return Fn;
}
+ struct NoteFn {
+ const CheckerNameRef CheckerName;
+ SymbolRef StreamSym;
+ std::string Message;
+
+ std::string operator()(PathSensitiveBugReport &BR) const {
+ llvm::errs()<<"NoteFn\n";
+ if (BR.isInteresting(StreamSym) &&
+ CheckerName == BR.getBugType().getCheckerName())
+ return Message;
+
+ return "";
+ }
+ };
+
void ErrorReturnChecker::checkPostCall(const CallEvent &Call,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
@@ -463,7 +611,14 @@
CalledFunctionData CFD{Fn, Call.getSourceRange()};
State = State->set<CalledFunctionDataMap>(RetSym, CFD);
- checkAccess(C, State, Call.getOriginExpr(), RetSym, &CFD);
+ //SmallString<100> buf;
+ //llvm::raw_svector_ostream os(buf);
+ //os << "Function \'???\' called here without checked return value";
+ ExplodedNode *N = checkAccess(C, State, Call.getOriginExpr(), RetSym, &CFD);
+ if (!N)
+ return;
+
+ C.addTransition(N->getState(), C.getNoteTag(NoteFn{getCheckerName(), RetSym, "Function \'???\' called here without checked return value"}, N));
}
void ErrorReturnChecker::checkLocation(SVal L, bool IsLoad, const Stmt *S,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
