[analyzer] Rename trackNullOrUndefValue to trackExpressionValue

trackNullOrUndefValue is a long and confusing name,
and it does not actually reflect what the function is doing.
Give a function a new name, with a relatively clear semantics.

Also remove some dead code.

Differential Revision: https://reviews.llvm.org/D52758

llvm-svn: 345064
This commit is contained in:
George Karpenkov 2018-10-23 18:24:53 +00:00
parent b4ba467da8
commit b2cf0063d0
17 changed files with 63 additions and 77 deletions

View File

@ -345,12 +345,11 @@ public:
namespace bugreporter {
/// Attempts to add visitors to trace a null or undefined value back to its
/// point of origin, whether it is a symbol constrained to null or an explicit
/// assignment.
/// Attempts to add visitors to track expression value back to its point of
/// origin.
///
/// \param N A node "downstream" from the evaluation of the statement.
/// \param S The statement whose value is null or undefined.
/// \param E The expression value which we are tracking
/// \param R The bug report to which visitors should be attached.
/// \param EnableNullFPSuppression Whether we should employ false positive
/// suppression (inlined defensive checks, returned null).
@ -358,13 +357,10 @@ namespace bugreporter {
/// \return Whether or not the function was able to add visitors for this
/// statement. Note that returning \c true does not actually imply
/// that any visitors were added.
bool trackNullOrUndefValue(const ExplodedNode *N, const Stmt *S, BugReport &R,
bool EnableNullFPSuppression = true);
bool trackExpressionValue(const ExplodedNode *N, const Expr *E, BugReport &R,
bool EnableNullFPSuppression = true);
const Expr *getDerefExpr(const Stmt *S);
const Stmt *GetDenomExpr(const ExplodedNode *N);
const Stmt *GetRetValExpr(const ExplodedNode *N);
bool isDeclRefExprToReference(const Expr *E);
} // namespace bugreporter

View File

@ -214,7 +214,7 @@ void NilArgChecker::generateBugReport(ExplodedNode *N,
auto R = llvm::make_unique<BugReport>(*BT, Msg, N);
R->addRange(Range);
bugreporter::trackNullOrUndefValue(N, E, *R);
bugreporter::trackExpressionValue(N, E, *R);
C.emitReport(std::move(R));
}
@ -578,7 +578,7 @@ void CFRetainReleaseChecker::checkPreCall(const CallEvent &Call,
auto report = llvm::make_unique<BugReport>(BT, OS.str(), N);
report->addRange(Call.getArgSourceRange(0));
bugreporter::trackNullOrUndefValue(N, Call.getArgExpr(0), *report);
bugreporter::trackExpressionValue(N, Call.getArgExpr(0), *report);
C.emitReport(std::move(report));
return;
}

View File

@ -553,7 +553,8 @@ void CStringChecker::emitNullArgBug(CheckerContext &C, ProgramStateRef State,
BuiltinBug *BT = static_cast<BuiltinBug *>(BT_Null.get());
auto Report = llvm::make_unique<BugReport>(*BT, WarningMsg, N);
Report->addRange(S->getSourceRange());
bugreporter::trackNullOrUndefValue(N, S, *Report);
if (const auto *Ex = dyn_cast<Expr>(S))
bugreporter::trackExpressionValue(N, Ex, *Report);
C.emitReport(std::move(Report));
}
}

View File

@ -108,7 +108,7 @@ void CallAndMessageChecker::emitBadCall(BugType *BT, CheckerContext &C,
R->addRange(BadE->getSourceRange());
if (BadE->isGLValue())
BadE = bugreporter::getDerefExpr(BadE);
bugreporter::trackNullOrUndefValue(N, BadE, *R);
bugreporter::trackExpressionValue(N, BadE, *R);
}
C.emitReport(std::move(R));
}
@ -185,9 +185,9 @@ bool CallAndMessageChecker::uninitRefOrPointer(
LazyInit_BT(BD, BT);
auto R = llvm::make_unique<BugReport>(*BT, Os.str(), N);
R->addRange(ArgRange);
if (ArgEx) {
bugreporter::trackNullOrUndefValue(N, ArgEx, *R);
}
if (ArgEx)
bugreporter::trackExpressionValue(N, ArgEx, *R);
C.emitReport(std::move(R));
}
return true;
@ -264,7 +264,7 @@ bool CallAndMessageChecker::PreVisitProcessArg(CheckerContext &C,
R->addRange(ArgRange);
if (ArgEx)
bugreporter::trackNullOrUndefValue(N, ArgEx, *R);
bugreporter::trackExpressionValue(N, ArgEx, *R);
C.emitReport(std::move(R));
}
return true;
@ -307,7 +307,7 @@ bool CallAndMessageChecker::PreVisitProcessArg(CheckerContext &C,
R->addRange(ArgRange);
if (ArgEx)
bugreporter::trackNullOrUndefValue(N, ArgEx, *R);
bugreporter::trackExpressionValue(N, ArgEx, *R);
// FIXME: enhance track back for uninitialized value for arbitrary
// memregions
C.emitReport(std::move(R));
@ -367,7 +367,7 @@ void CallAndMessageChecker::checkPreStmt(const CXXDeleteExpr *DE,
Desc = "Argument to 'delete' is uninitialized";
BugType *BT = BT_cxx_delete_undef.get();
auto R = llvm::make_unique<BugReport>(*BT, Desc, N);
bugreporter::trackNullOrUndefValue(N, DE, *R);
bugreporter::trackExpressionValue(N, DE, *R);
C.emitReport(std::move(R));
return;
}
@ -496,7 +496,7 @@ void CallAndMessageChecker::checkPreObjCMessage(const ObjCMethodCall &msg,
// FIXME: getTrackNullOrUndefValueVisitor can't handle "super" yet.
if (const Expr *ReceiverE = ME->getInstanceReceiver())
bugreporter::trackNullOrUndefValue(N, ReceiverE, *R);
bugreporter::trackExpressionValue(N, ReceiverE, *R);
C.emitReport(std::move(R));
}
return;
@ -537,7 +537,7 @@ void CallAndMessageChecker::emitNilReceiverBug(CheckerContext &C,
report->addRange(ME->getReceiverRange());
// FIXME: This won't track "self" in messages to super.
if (const Expr *receiver = ME->getInstanceReceiver()) {
bugreporter::trackNullOrUndefValue(N, receiver, *report);
bugreporter::trackExpressionValue(N, receiver, *report);
}
C.emitReport(std::move(report));
}

View File

@ -111,6 +111,12 @@ static bool suppressReport(const Expr *E) {
return E->getType().getQualifiers().hasAddressSpace();
}
static bool isDeclRefExprToReference(const Expr *E) {
if (const auto *DRE = dyn_cast<DeclRefExpr>(E))
return DRE->getDecl()->getType()->isReferenceType();
return false;
}
void DereferenceChecker::reportBug(ProgramStateRef State, const Stmt *S,
CheckerContext &C) const {
// Generate an error node.
@ -154,7 +160,7 @@ void DereferenceChecker::reportBug(ProgramStateRef State, const Stmt *S,
}
case Stmt::MemberExprClass: {
const MemberExpr *M = cast<MemberExpr>(S);
if (M->isArrow() || bugreporter::isDeclRefExprToReference(M->getBase())) {
if (M->isArrow() || isDeclRefExprToReference(M->getBase())) {
os << "Access to field '" << M->getMemberNameInfo()
<< "' results in a dereference of a null pointer";
AddDerefSource(os, Ranges, M->getBase()->IgnoreParenCasts(),
@ -177,7 +183,7 @@ void DereferenceChecker::reportBug(ProgramStateRef State, const Stmt *S,
auto report = llvm::make_unique<BugReport>(
*BT_null, buf.empty() ? BT_null->getDescription() : StringRef(buf), N);
bugreporter::trackNullOrUndefValue(N, bugreporter::getDerefExpr(S), *report);
bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *report);
for (SmallVectorImpl<SourceRange>::iterator
I = Ranges.begin(), E = Ranges.end(); I!=E; ++I)
@ -197,8 +203,7 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S,
auto report =
llvm::make_unique<BugReport>(*BT_undef, BT_undef->getDescription(), N);
bugreporter::trackNullOrUndefValue(N, bugreporter::getDerefExpr(S),
*report);
bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *report);
C.emitReport(std::move(report));
}
return;

View File

@ -32,6 +32,13 @@ public:
};
} // end anonymous namespace
static const Expr *getDenomExpr(const ExplodedNode *N) {
const Stmt *S = N->getLocationAs<PreStmt>()->getStmt();
if (const auto *BE = dyn_cast<BinaryOperator>(S))
return BE->getRHS();
return nullptr;
}
void DivZeroChecker::reportBug(
const char *Msg, ProgramStateRef StateZero, CheckerContext &C,
std::unique_ptr<BugReporterVisitor> Visitor) const {
@ -41,7 +48,7 @@ void DivZeroChecker::reportBug(
auto R = llvm::make_unique<BugReport>(*BT, Msg, N);
R->addVisitor(std::move(Visitor));
bugreporter::trackNullOrUndefValue(N, bugreporter::GetDenomExpr(N), *R);
bugreporter::trackExpressionValue(N, getDenomExpr(N), *R);
C.emitReport(std::move(R));
}
}

View File

@ -192,7 +192,7 @@ NonNullParamChecker::genReportNullAttrNonNull(const ExplodedNode *ErrorNode,
*BTAttrNonNull,
"Null pointer passed as an argument to a 'nonnull' parameter", ErrorNode);
if (ArgE)
bugreporter::trackNullOrUndefValue(ErrorNode, ArgE, *R);
bugreporter::trackExpressionValue(ErrorNode, ArgE, *R);
return R;
}
@ -208,9 +208,7 @@ std::unique_ptr<BugReport> NonNullParamChecker::genReportReferenceToNullPointer(
const Expr *ArgEDeref = bugreporter::getDerefExpr(ArgE);
if (!ArgEDeref)
ArgEDeref = ArgE;
bugreporter::trackNullOrUndefValue(ErrorNode,
ArgEDeref,
*R);
bugreporter::trackExpressionValue(ErrorNode, ArgEDeref, *R);
}
return R;

View File

@ -174,7 +174,8 @@ private:
if (Error == ErrorKind::NilAssignedToNonnull ||
Error == ErrorKind::NilPassedToNonnull ||
Error == ErrorKind::NilReturnedToNonnull)
bugreporter::trackNullOrUndefValue(N, ValueExpr, *R);
if (const auto *Ex = dyn_cast<Expr>(ValueExpr))
bugreporter::trackExpressionValue(N, Ex, *R);
}
BR.emitReport(std::move(R));
}

View File

@ -49,7 +49,7 @@ void ObjCAtSyncChecker::checkPreStmt(const ObjCAtSynchronizedStmt *S,
"for @synchronized"));
auto report =
llvm::make_unique<BugReport>(*BT_undef, BT_undef->getDescription(), N);
bugreporter::trackNullOrUndefValue(N, Ex, *report);
bugreporter::trackExpressionValue(N, Ex, *report);
C.emitReport(std::move(report));
}
return;
@ -73,7 +73,7 @@ void ObjCAtSyncChecker::checkPreStmt(const ObjCAtSynchronizedStmt *S,
"(no synchronization will occur)"));
auto report =
llvm::make_unique<BugReport>(*BT_null, BT_null->getDescription(), N);
bugreporter::trackNullOrUndefValue(N, Ex, *report);
bugreporter::trackExpressionValue(N, Ex, *report);
C.emitReport(std::move(report));
return;

View File

@ -87,7 +87,7 @@ static void emitBug(CheckerContext &C, BuiltinBug &BT, const Expr *RetE,
auto Report = llvm::make_unique<BugReport>(BT, BT.getDescription(), N);
Report->addRange(RetE->getSourceRange());
bugreporter::trackNullOrUndefValue(N, TrackingE ? TrackingE : RetE, *Report);
bugreporter::trackExpressionValue(N, TrackingE ? TrackingE : RetE, *Report);
C.emitReport(std::move(Report));
}

View File

@ -98,7 +98,7 @@ void UndefBranchChecker::checkBranchCondition(const Stmt *Condition,
// Emit the bug report.
auto R = llvm::make_unique<BugReport>(*BT, BT->getDescription(), N);
bugreporter::trackNullOrUndefValue(N, Ex, *R);
bugreporter::trackExpressionValue(N, Ex, *R);
R->addRange(Ex->getSourceRange());
Ctx.emitReport(std::move(R));

View File

@ -174,10 +174,10 @@ void UndefResultChecker::checkPostStmt(const BinaryOperator *B,
auto report = llvm::make_unique<BugReport>(*BT, OS.str(), N);
if (Ex) {
report->addRange(Ex->getSourceRange());
bugreporter::trackNullOrUndefValue(N, Ex, *report);
bugreporter::trackExpressionValue(N, Ex, *report);
}
else
bugreporter::trackNullOrUndefValue(N, B, *report);
bugreporter::trackExpressionValue(N, B, *report);
C.emitReport(std::move(report));
}

View File

@ -55,7 +55,7 @@ UndefinedArraySubscriptChecker::checkPreStmt(const ArraySubscriptExpr *A,
// Generate a report for this bug.
auto R = llvm::make_unique<BugReport>(*BT, BT->getName(), N);
R->addRange(A->getIdx()->getSourceRange());
bugreporter::trackNullOrUndefValue(N, A->getIdx(), *R);
bugreporter::trackExpressionValue(N, A->getIdx(), *R);
C.emitReport(std::move(R));
}

View File

@ -112,7 +112,7 @@ void UndefinedAssignmentChecker::checkBind(SVal location, SVal val,
auto R = llvm::make_unique<BugReport>(*BT, OS.str(), N);
if (ex) {
R->addRange(ex->getSourceRange());
bugreporter::trackNullOrUndefValue(N, ex, *R);
bugreporter::trackExpressionValue(N, ex, *R);
}
C.emitReport(std::move(R));
}

View File

@ -314,7 +314,7 @@ bool UnixAPIChecker::ReportZeroByteAllocation(CheckerContext &C,
auto report = llvm::make_unique<BugReport>(*BT_mallocZero, os.str(), N);
report->addRange(arg->getSourceRange());
bugreporter::trackNullOrUndefValue(N, arg, *report);
bugreporter::trackExpressionValue(N, arg, *report);
C.emitReport(std::move(report));
return true;

View File

@ -74,7 +74,7 @@ void VLASizeChecker::reportBug(
auto report = llvm::make_unique<BugReport>(*BT, os.str(), N);
report->addVisitor(std::move(Visitor));
report->addRange(SizeE->getSourceRange());
bugreporter::trackNullOrUndefValue(N, SizeE, *report);
bugreporter::trackExpressionValue(N, SizeE, *report);
C.emitReport(std::move(report));
}

View File

@ -71,12 +71,6 @@ using namespace ento;
// Utility functions.
//===----------------------------------------------------------------------===//
bool bugreporter::isDeclRefExprToReference(const Expr *E) {
if (const auto *DRE = dyn_cast<DeclRefExpr>(E))
return DRE->getDecl()->getType()->isReferenceType();
return false;
}
static const Expr *peelOffPointerArithmetic(const BinaryOperator *B) {
if (B->isAdditiveOp() && B->getType()->isPointerType()) {
if (B->getLHS()->getType()->isPointerType()) {
@ -160,20 +154,6 @@ const Expr *bugreporter::getDerefExpr(const Stmt *S) {
return E;
}
const Stmt *bugreporter::GetDenomExpr(const ExplodedNode *N) {
const Stmt *S = N->getLocationAs<PreStmt>()->getStmt();
if (const auto *BE = dyn_cast<BinaryOperator>(S))
return BE->getRHS();
return nullptr;
}
const Stmt *bugreporter::GetRetValExpr(const ExplodedNode *N) {
const Stmt *S = N->getLocationAs<PostStmt>()->getStmt();
if (const auto *RS = dyn_cast<ReturnStmt>(S))
return RS->getRetValue();
return nullptr;
}
//===----------------------------------------------------------------------===//
// Definitions for bug reporter visitors.
//===----------------------------------------------------------------------===//
@ -884,7 +864,7 @@ public:
RetE = RetE->IgnoreParenCasts();
// If we're returning 0, we should track where that 0 came from.
bugreporter::trackNullOrUndefValue(N, RetE, BR, EnableNullFPSuppression);
bugreporter::trackExpressionValue(N, RetE, BR, EnableNullFPSuppression);
// Build an appropriate message based on the return value.
SmallString<64> Msg;
@ -979,8 +959,7 @@ public:
if (!State->isNull(*ArgV).isConstrainedTrue())
continue;
if (bugreporter::trackNullOrUndefValue(N, ArgE, BR,
EnableNullFPSuppression))
if (bugreporter::trackExpressionValue(N, ArgE, BR, EnableNullFPSuppression))
ShouldInvalidate = false;
// If we /can't/ track the null pointer, we should err on the side of
@ -1258,8 +1237,8 @@ FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ,
V.getAs<loc::ConcreteInt>() || V.getAs<nonloc::ConcreteInt>()) {
if (!IsParam)
InitE = InitE->IgnoreParenCasts();
bugreporter::trackNullOrUndefValue(StoreSite, InitE, BR,
EnableNullFPSuppression);
bugreporter::trackExpressionValue(StoreSite, InitE, BR,
EnableNullFPSuppression);
}
ReturnVisitor::addVisitorIfNecessary(StoreSite, InitE->IgnoreParenCasts(),
BR, EnableNullFPSuppression);
@ -1588,14 +1567,13 @@ static const ExplodedNode* findNodeForExpression(const ExplodedNode *N,
return N;
}
bool bugreporter::trackNullOrUndefValue(const ExplodedNode *InputNode,
const Stmt *InputS,
BugReport &report,
bool EnableNullFPSuppression) {
if (!InputS || !InputNode || !isa<Expr>(InputS))
bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
const Expr *E, BugReport &report,
bool EnableNullFPSuppression) {
if (!E || !InputNode)
return false;
const Expr *Inner = peelOffOuterExpr(cast<Expr>(InputS), InputNode);
const Expr *Inner = peelOffOuterExpr(E, InputNode);
const ExplodedNode *LVNode = findNodeForExpression(InputNode, Inner);
if (!LVNode)
return false;
@ -1606,11 +1584,11 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *InputNode,
// At this point in the path, the receiver should be live since we are at the
// message send expr. If it is nil, start tracking it.
if (const Expr *Receiver = NilReceiverBRVisitor::getNilReceiver(Inner, LVNode))
trackNullOrUndefValue(LVNode, Receiver, report, EnableNullFPSuppression);
trackExpressionValue(LVNode, Receiver, report, EnableNullFPSuppression);
// See if the expression we're interested refers to a variable.
// If so, we can track both its contents and constraints on its value.
if (Inner && ExplodedGraph::isInterestingLValueExpr(Inner)) {
if (ExplodedGraph::isInterestingLValueExpr(Inner)) {
SVal LVal = LVNode->getSVal(Inner);
const MemRegion *RR = getLocationRegionIfReference(Inner, LVNode);
@ -1703,7 +1681,7 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *InputNode,
if (RegionRVal && isa<SymbolicRegion>(RegionRVal)) {
report.markInteresting(RegionRVal);
report.addVisitor(llvm::make_unique<TrackConstraintBRVisitor>(
loc::MemRegionVal(RegionRVal), false));
loc::MemRegionVal(RegionRVal), /*assumption=*/false));
}
}
return true;
@ -1751,8 +1729,8 @@ NilReceiverBRVisitor::VisitNode(const ExplodedNode *N,
// The receiver was nil, and hence the method was skipped.
// Register a BugReporterVisitor to issue a message telling us how
// the receiver was null.
bugreporter::trackNullOrUndefValue(N, Receiver, BR,
/*EnableNullFPSuppression*/ false);
bugreporter::trackExpressionValue(N, Receiver, BR,
/*EnableNullFPSuppression*/ false);
// Issue a message saying that the method was skipped.
PathDiagnosticLocation L(Receiver, BRC.getSourceManager(),
N->getLocationContext());