Skip to content

Commit

Permalink
[analyzer] [NFC] Minor refactoring of trackNullOrUndefValue
Browse files Browse the repository at this point in the history 
Simple refactoring attempt: factor out some code, remove some
repetition, use auto where appropriate.

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

llvm-svn: 322151
  • Loading branch information
George Karpenkov committed Jan 10, 2018
1 parent 57dc6b7 commit c41f379
Showing 1 changed file with 97 additions and 80 deletions.
177 changes: 97 additions & 80 deletions clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
View file
Expand Up @@ -1003,59 +1003,89 @@ static const Expr *peelOffOuterExpr(const Expr *Ex,
return Ex;
}

/// Walk through nodes until we get one that matches the statement exactly.
/// Alternately, if we hit a known lvalue for the statement, we know we've
/// gone too far (though we can likely track the lvalue better anyway).
static const ExplodedNode* findNodeForStatement(const ExplodedNode *N,
const Stmt *S,
const Expr *Inner) {
do {
const ProgramPoint &pp = N->getLocation();
if (auto ps = pp.getAs<StmtPoint>()) {
if (ps->getStmt() == S || ps->getStmt() == Inner)
break;
} else if (auto CEE = pp.getAs<CallExitEnd>()) {
if (CEE->getCalleeContext()->getCallSite() == S ||
CEE->getCalleeContext()->getCallSite() == Inner)
break;
}
N = N->getFirstPred();
} while (N);
return N;
}

/// Find the ExplodedNode where the lvalue (the value of 'Ex')
/// was computed.
static const ExplodedNode* findNodeForExpression(const ExplodedNode *N,
const Expr *Inner) {
while (N) {
if (auto P = N->getLocation().getAs<PostStmt>()) {
if (P->getStmt() == Inner)
break;
}
N = N->getFirstPred();
}
assert(N && "Unable to find the lvalue node.");
return N;

}

/// Performing operator `&' on an lvalue expression is essentially a no-op.
/// Then, if we are taking addresses of fields or elements, these are also
/// unlikely to matter.
static const Expr* peelOfOuterAddrOf(const Expr* Ex) {
Ex = Ex->IgnoreParenCasts();

// FIXME: There's a hack in our Store implementation that always computes
// field offsets around null pointers as if they are always equal to 0.
// The idea here is to report accesses to fields as null dereferences
// even though the pointer value that's being dereferenced is actually
// the offset of the field rather than exactly 0.
// See the FIXME in StoreManager's getLValueFieldOrIvar() method.
// This code interacts heavily with this hack; otherwise the value
// would not be null at all for most fields, so we'd be unable to track it.
if (const auto *Op = dyn_cast<UnaryOperator>(Ex))
if (Op->getOpcode() == UO_AddrOf && Op->getSubExpr()->isLValue())
if (const Expr *DerefEx = bugreporter::getDerefExpr(Op->getSubExpr()))
return DerefEx;
return Ex;

}

bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N,
const Stmt *S,
BugReport &report, bool IsArg,
bool EnableNullFPSuppression) {
if (!S || !N)
return false;

if (const Expr *Ex = dyn_cast<Expr>(S))
if (const auto *Ex = dyn_cast<Expr>(S))
S = peelOffOuterExpr(Ex, N);

const Expr *Inner = nullptr;
if (const Expr *Ex = dyn_cast<Expr>(S)) {
if (const auto *Ex = dyn_cast<Expr>(S)) {
Ex = peelOfOuterAddrOf(Ex);
Ex = Ex->IgnoreParenCasts();

// Performing operator `&' on an lvalue expression is essentially a no-op.
// Then, if we are taking addresses of fields or elements, these are also
// unlikely to matter.
// FIXME: There's a hack in our Store implementation that always computes
// field offsets around null pointers as if they are always equal to 0.
// The idea here is to report accesses to fields as null dereferences
// even though the pointer value that's being dereferenced is actually
// the offset of the field rather than exactly 0.
// See the FIXME in StoreManager's getLValueFieldOrIvar() method.
// This code interacts heavily with this hack; otherwise the value
// would not be null at all for most fields, so we'd be unable to track it.
if (const auto *Op = dyn_cast<UnaryOperator>(Ex))
if (Op->getOpcode() == UO_AddrOf && Op->getSubExpr()->isLValue())
if (const Expr *DerefEx = getDerefExpr(Op->getSubExpr()))
Ex = DerefEx;

if (Ex && (ExplodedGraph::isInterestingLValueExpr(Ex) || CallEvent::isCallStmt(Ex)))
if (Ex && (ExplodedGraph::isInterestingLValueExpr(Ex)
|| CallEvent::isCallStmt(Ex)))
Inner = Ex;
}

if (IsArg && !Inner) {
assert(N->getLocation().getAs<CallEnter>() && "Tracking arg but not at call");
} else {
// Walk through nodes until we get one that matches the statement exactly.
// Alternately, if we hit a known lvalue for the statement, we know we've
// gone too far (though we can likely track the lvalue better anyway).
do {
const ProgramPoint &pp = N->getLocation();
if (Optional<StmtPoint> ps = pp.getAs<StmtPoint>()) {
if (ps->getStmt() == S || ps->getStmt() == Inner)
break;
} else if (Optional<CallExitEnd> CEE = pp.getAs<CallExitEnd>()) {
if (CEE->getCalleeContext()->getCallSite() == S ||
CEE->getCalleeContext()->getCallSite() == Inner)
break;
}
N = N->getFirstPred();
} while (N);

N = findNodeForStatement(N, S, Inner);
if (!N)
return false;
}
Expand All @@ -1066,48 +1096,35 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N,
// 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(S, N))
trackNullOrUndefValue(N, Receiver, report, false, EnableNullFPSuppression);

trackNullOrUndefValue(N, Receiver, report, /* IsArg=*/ false,
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)) {
const MemRegion *R = nullptr;

// Find the ExplodedNode where the lvalue (the value of 'Ex')
// was computed. We need this for getting the location value.
const ExplodedNode *LVNode = N;
while (LVNode) {
if (Optional<PostStmt> P = LVNode->getLocation().getAs<PostStmt>()) {
if (P->getStmt() == Inner)
break;
}
LVNode = LVNode->getFirstPred();
}
assert(LVNode && "Unable to find the lvalue node.");
const ExplodedNode *LVNode = findNodeForExpression(N, Inner);
ProgramStateRef LVState = LVNode->getState();
SVal LVal = LVState->getSVal(Inner, LVNode->getLocationContext());

if (LVState->isNull(LVal).isConstrainedTrue()) {
// In case of C++ references, we want to differentiate between a null
// reference and reference to null pointer.
// If the LVal is null, check if we are dealing with null reference.
// For those, we want to track the location of the reference.
if (const MemRegion *RR = getLocationRegionIfReference(Inner, N))
R = RR;
} else {
R = LVState->getSVal(Inner, LVNode->getLocationContext()).getAsRegion();

// If this is a C++ reference to a null pointer, we are tracking the
// pointer. In addition, we should find the store at which the reference
// got initialized.
if (const MemRegion *RR = getLocationRegionIfReference(Inner, N)) {
if (Optional<KnownSVal> KV = LVal.getAs<KnownSVal>())
report.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>(
const MemRegion *RR = getLocationRegionIfReference(Inner, N);
bool LVIsNull = LVState->isNull(LVal).isConstrainedTrue();

// If this is a C++ reference to a null pointer, we are tracking the
// pointer. In addition, we should find the store at which the reference
// got initialized.
if (RR && !LVIsNull) {
if (auto KV = LVal.getAs<KnownSVal>())
report.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>(
*KV, RR, EnableNullFPSuppression));
}
}

// In case of C++ references, we want to differentiate between a null
// reference and reference to null pointer.
// If the LVal is null, check if we are dealing with null reference.
// For those, we want to track the location of the reference.
const MemRegion *R = (RR && LVIsNull) ? RR :
LVState->getSVal(Inner, LVNode->getLocationContext()).getAsRegion();

if (R) {
// Mark both the variable region and its contents as interesting.
SVal V = LVState->getRawSVal(loc::MemRegionVal(R));
Expand All @@ -1119,21 +1136,21 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N,
// If the contents are symbolic, find out when they became null.
if (V.getAsLocSymbol(/*IncludeBaseRegions*/ true))
report.addVisitor(llvm::make_unique<TrackConstraintBRVisitor>(
V.castAs<DefinedSVal>(), false));
V.castAs<DefinedSVal>(), false));

// Add visitor, which will suppress inline defensive checks.
if (Optional<DefinedSVal> DV = V.getAs<DefinedSVal>()) {
if (auto DV = V.getAs<DefinedSVal>()) {
if (!DV->isZeroConstant() && LVState->isNull(*DV).isConstrainedTrue() &&
EnableNullFPSuppression) {
report.addVisitor(
llvm::make_unique<SuppressInlineDefensiveChecksVisitor>(*DV,
LVNode));
LVNode));
}
}

if (Optional<KnownSVal> KV = V.getAs<KnownSVal>())
if (auto KV = V.getAs<KnownSVal>())
report.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>(
*KV, R, EnableNullFPSuppression));
*KV, R, EnableNullFPSuppression));
return true;
}
}
Expand All @@ -1144,7 +1161,7 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N,

// If the value came from an inlined function call, we should at least make
// sure that function isn't pruned in our output.
if (const Expr *E = dyn_cast<Expr>(S))
if (const auto *E = dyn_cast<Expr>(S))
S = E->IgnoreParenCasts();

ReturnVisitor::addVisitorIfNecessary(N, S, report, EnableNullFPSuppression);
Expand All @@ -1153,29 +1170,29 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N,
// base value that was dereferenced.
// assert(!V.isUnknownOrUndef());
// Is it a symbolic value?
if (Optional<loc::MemRegionVal> L = V.getAs<loc::MemRegionVal>()) {
if (auto L = V.getAs<loc::MemRegionVal>()) {
report.addVisitor(llvm::make_unique<UndefOrNullArgVisitor>(L->getRegion()));

// At this point we are dealing with the region's LValue.
// However, if the rvalue is a symbolic region, we should track it as well.
// Try to use the correct type when looking up the value.
SVal RVal;
if (const Expr *E = dyn_cast<Expr>(S))
if (const auto *E = dyn_cast<Expr>(S))
RVal = state->getRawSVal(L.getValue(), E->getType());
else
RVal = state->getSVal(L->getRegion());

report.addVisitor(llvm::make_unique<UndefOrNullArgVisitor>(L->getRegion()));
if (Optional<KnownSVal> KV = RVal.getAs<KnownSVal>())
if (auto KV = RVal.getAs<KnownSVal>())
report.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>(
*KV, L->getRegion(), EnableNullFPSuppression));
*KV, L->getRegion(), EnableNullFPSuppression));

const MemRegion *RegionRVal = RVal.getAsRegion();
if (RegionRVal && isa<SymbolicRegion>(RegionRVal)) {
report.markInteresting(RegionRVal);
report.addVisitor(llvm::make_unique<TrackConstraintBRVisitor>(
loc::MemRegionVal(RegionRVal), false));
loc::MemRegionVal(RegionRVal), false));
}
}

return true;
}

Expand Down

0 comments on commit c41f379

Please sign in to comment.