Skip to content

Commit

Permalink
[analyzer] NFC: Assert that our fix for noreturn destructors keeps wo…
Browse files Browse the repository at this point in the history
…rking.

Massive false positives were known to be caused by continuing the analysis
after a destructor with a noreturn attribute has been executed in the program
but not modeled in the analyzer due to being missing in the CFG.

Now that work is being done on enabling the modeling of temporary constructors
and destructors in the CFG, we need to make sure that the heuristic that
suppresses these false positives keeps working when such modeling is disabled.
In particular, different code paths open up when the corresponding constructor
is being inlined during analysis.

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

llvm-svn: 324802
  • Loading branch information
haoNoQ committed Feb 10, 2018
1 parent b73028b commit 3da7205
Showing 1 changed file with 15 additions and 5 deletions.
20 changes: 15 additions & 5 deletions clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
Expand Up @@ -356,20 +356,30 @@ void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
// paths when no-return temporary destructors are used for assertions.
const AnalysisDeclContext *ADC = LCtx->getAnalysisDeclContext();
if (!ADC->getCFGBuildOptions().AddTemporaryDtors) {
const MemRegion *Target = Call->getCXXThisVal().getAsRegion();
if (Target && isa<CXXTempObjectRegion>(Target) &&
Call->getDecl()->getParent()->isAnyDestructorNoReturn()) {
const MemRegion *Target = Call->getCXXThisVal().getAsRegion();
if (Target && isa<CXXTempObjectRegion>(Target) &&
Call->getDecl()->getParent()->isAnyDestructorNoReturn()) {

// If we've inlined the constructor, then DstEvaluated would be empty.
// In this case we still want a sink, which could be implemented
// in processCallExit. But we don't have that implemented at the moment,
// so if you hit this assertion, see if you can avoid inlining
// the respective constructor when analyzer-config cfg-temporary-dtors
// is set to false.
// Otherwise there's nothing wrong with inlining such constructor.
assert(!DstEvaluated.empty() &&
"We should not have inlined this constructor!");

for (ExplodedNode *N : DstEvaluated) {
Bldr.generateSink(CE, N, N->getState());
}

// There is no need to run the PostCall and PostStmtchecker
// There is no need to run the PostCall and PostStmt checker
// callbacks because we just generated sinks on all nodes in th
// frontier.
return;
}
}
}

ExplodedNodeSet DstPostCall;
getCheckerManager().runCheckersForPostCall(DstPostCall, DstEvaluated,
Expand Down

0 comments on commit 3da7205

Please sign in to comment.