aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h19
-rw-r--r--lib/StaticAnalyzer/Core/BugReporter.cpp73
-rw-r--r--lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp5
-rw-r--r--lib/StaticAnalyzer/Core/PathDiagnostic.cpp82
-rw-r--r--test/Analysis/dtor.cpp69
5 files changed, 184 insertions, 64 deletions
diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h b/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
index 730a558b74..73958a6ef4 100644
--- a/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
+++ b/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
@@ -148,6 +148,15 @@ public:
assert(Range.isValid());
}
+ /// Create a location at an explicit offset in the source.
+ ///
+ /// This should only be used if there are no more appropriate constructors.
+ PathDiagnosticLocation(SourceLocation loc, const SourceManager &sm)
+ : K(SingleLocK), S(0), D(0), SM(&sm), Loc(loc, sm), Range(genRange()) {
+ assert(Loc.isValid());
+ assert(Range.isValid());
+ }
+
/// Create a location corresponding to the given declaration.
static PathDiagnosticLocation create(const Decl *D,
const SourceManager &SM) {
@@ -163,6 +172,14 @@ public:
const SourceManager &SM,
const LocationOrAnalysisDeclContext LAC);
+ /// Create a location for the end of the statement.
+ ///
+ /// If the statement is a CompoundStatement, the location will point to the
+ /// closing brace instead of following it.
+ static PathDiagnosticLocation createEnd(const Stmt *S,
+ const SourceManager &SM,
+ const LocationOrAnalysisDeclContext LAC);
+
/// Create the location for the operator of the binary expression.
/// Assumes the statement has a valid location.
static PathDiagnosticLocation createOperatorLoc(const BinaryOperator *BO,
@@ -637,6 +654,8 @@ public:
void pushActivePath(PathPieces *p) { pathStack.push_back(p); }
void popActivePath() { if (!pathStack.empty()) pathStack.pop_back(); }
+
+ bool isWithinCall() const { return !pathStack.empty(); }
// PathDiagnostic();
PathDiagnostic(const Decl *DeclWithIssue,
diff --git a/lib/StaticAnalyzer/Core/BugReporter.cpp b/lib/StaticAnalyzer/Core/BugReporter.cpp
index 08588f60ab..12479ca53a 100644
--- a/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -441,21 +441,23 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD,
}
if (const CallEnter *CE = dyn_cast<CallEnter>(&P)) {
+ // Flush all locations, and pop the active path.
+ bool VisitedEntireCall = PD.isWithinCall();
PD.popActivePath();
- // The current active path should never be empty. Either we
- // just added a bunch of stuff to the top-level path, or
- // we have a previous CallExitEnd. If the front of the active
- // path is not a PathDiagnosticCallPiece, it means that the
+
+ // Either we just added a bunch of stuff to the top-level path, or
+ // we have a previous CallExitEnd. If the former, it means that the
// path terminated within a function call. We must then take the
// current contents of the active path and place it within
// a new PathDiagnosticCallPiece.
- assert(!PD.getActivePath().empty());
- PathDiagnosticCallPiece *C =
- dyn_cast<PathDiagnosticCallPiece>(PD.getActivePath().front());
- if (!C) {
+ PathDiagnosticCallPiece *C;
+ if (VisitedEntireCall) {
+ C = cast<PathDiagnosticCallPiece>(PD.getActivePath().front());
+ } else {
const Decl *Caller = CE->getLocationContext()->getDecl();
C = PathDiagnosticCallPiece::construct(PD.getActivePath(), Caller);
}
+
C->setCallee(*CE, SMgr);
if (!CallStack.empty()) {
assert(CallStack.back().first == C);
@@ -868,6 +870,7 @@ public:
void rawAddEdge(PathDiagnosticLocation NewLoc);
void addContext(const Stmt *S);
+ void addContext(const PathDiagnosticLocation &L);
void addExtendedContext(const Stmt *S);
};
} // end anonymous namespace
@@ -1035,7 +1038,10 @@ void EdgeBuilder::addContext(const Stmt *S) {
return;
PathDiagnosticLocation L(S, PDB.getSourceManager(), PDB.LC);
+ addContext(L);
+}
+void EdgeBuilder::addContext(const PathDiagnosticLocation &L) {
while (!CLocs.empty()) {
const PathDiagnosticLocation &TopContextLoc = CLocs.back();
@@ -1151,25 +1157,22 @@ static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD,
}
if (const CallExitEnd *CE = dyn_cast<CallExitEnd>(&P)) {
- const StackFrameContext *LCtx =
- CE->getLocationContext()->getCurrentStackFrame();
- // FIXME: This needs to handle implicit calls.
- if (const Stmt *S = CE->getCalleeContext()->getCallSite()) {
- if (const Expr *Ex = dyn_cast<Expr>(S))
+ const Stmt *S = CE->getCalleeContext()->getCallSite();
+ if (const Expr *Ex = dyn_cast_or_null<Expr>(S)) {
reversePropagateIntererstingSymbols(*PDB.getBugReport(), IE,
N->getState().getPtr(), Ex,
N->getLocationContext());
- PathDiagnosticLocation Loc(S,
- PDB.getSourceManager(),
- LCtx);
- EB.addEdge(Loc, true);
- EB.flushLocations();
- PathDiagnosticCallPiece *C =
- PathDiagnosticCallPiece::construct(N, *CE, SM);
- PD.getActivePath().push_front(C);
- PD.pushActivePath(&C->path);
- CallStack.push_back(StackDiagPair(C, N));
}
+
+ PathDiagnosticCallPiece *C =
+ PathDiagnosticCallPiece::construct(N, *CE, SM);
+
+ EB.addEdge(C->callReturn, true);
+ EB.flushLocations();
+
+ PD.getActivePath().push_front(C);
+ PD.pushActivePath(&C->path);
+ CallStack.push_back(StackDiagPair(C, N));
break;
}
@@ -1183,30 +1186,26 @@ static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD,
EB.addEdge(pos);
// Flush all locations, and pop the active path.
+ bool VisitedEntireCall = PD.isWithinCall();
EB.flushLocations();
PD.popActivePath();
- assert(!PD.getActivePath().empty());
PDB.LC = N->getLocationContext();
- // The current active path should never be empty. Either we
- // just added a bunch of stuff to the top-level path, or
- // we have a previous CallExitEnd. If the front of the active
- // path is not a PathDiagnosticCallPiece, it means that the
+ // Either we just added a bunch of stuff to the top-level path, or
+ // we have a previous CallExitEnd. If the former, it means that the
// path terminated within a function call. We must then take the
// current contents of the active path and place it within
// a new PathDiagnosticCallPiece.
- PathDiagnosticCallPiece *C =
- dyn_cast<PathDiagnosticCallPiece>(PD.getActivePath().front());
- if (!C) {
- const Decl * Caller = CE->getLocationContext()->getDecl();
+ PathDiagnosticCallPiece *C;
+ if (VisitedEntireCall) {
+ C = cast<PathDiagnosticCallPiece>(PD.getActivePath().front());
+ } else {
+ const Decl *Caller = CE->getLocationContext()->getDecl();
C = PathDiagnosticCallPiece::construct(PD.getActivePath(), Caller);
}
- // FIXME: We still need a location for implicit calls.
- if (CE->getCallExpr()) {
- C->setCallee(*CE, SM);
- EB.addContext(CE->getCallExpr());
- }
+ C->setCallee(*CE, SM);
+ EB.addContext(C->getLocation());
if (!CallStack.empty()) {
assert(CallStack.back().first == C);
diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 4431c03d49..89d1e27c3d 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -293,6 +293,11 @@ bool ExprEngine::inlineCall(const CallEvent &Call,
if (!ADC->getCFGBuildOptions().AddImplicitDtors ||
!ADC->getCFGBuildOptions().AddInitializers)
return false;
+ // FIXME: This is a hack. We only process VarDecl destructors right now,
+ // so we should only inline VarDecl constructors.
+ if (const CXXConstructorCall *Ctor = dyn_cast<CXXConstructorCall>(&Call))
+ if (!isa<VarRegion>(Ctor->getCXXThisVal().getAsRegion()))
+ return false;
break;
}
case CE_CXXAllocator:
diff --git a/lib/StaticAnalyzer/Core/PathDiagnostic.cpp b/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
index bfb8bf6432..5496df0f8f 100644
--- a/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ b/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -16,6 +16,7 @@
#include "clang/Basic/SourceManager.h"
#include "clang/AST/Expr.h"
#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/ParentMap.h"
#include "clang/AST/StmtCXX.h"
@@ -242,13 +243,14 @@ PathDiagnosticConsumer::FlushDiagnostics(SmallVectorImpl<std::string> *Files) {
//===----------------------------------------------------------------------===//
static SourceLocation getValidSourceLocation(const Stmt* S,
- LocationOrAnalysisDeclContext LAC) {
- SourceLocation L = S->getLocStart();
+ LocationOrAnalysisDeclContext LAC,
+ bool UseEnd = false) {
+ SourceLocation L = UseEnd ? S->getLocEnd() : S->getLocStart();
assert(!LAC.isNull() && "A valid LocationContext or AnalysisDeclContext should "
"be passed to PathDiagnosticLocation upon creation.");
// S might be a temporary statement that does not have a location in the
- // source code, so find an enclosing statement and use it's location.
+ // source code, so find an enclosing statement and use its location.
if (!L.isValid()) {
ParentMap *PM = 0;
@@ -259,7 +261,7 @@ static SourceLocation getValidSourceLocation(const Stmt* S,
while (!L.isValid()) {
S = PM->getParent(S);
- L = S->getLocStart();
+ L = UseEnd ? S->getLocEnd() : S->getLocStart();
}
}
@@ -280,6 +282,17 @@ PathDiagnosticLocation
SM, SingleLocK);
}
+
+PathDiagnosticLocation
+PathDiagnosticLocation::createEnd(const Stmt *S,
+ const SourceManager &SM,
+ LocationOrAnalysisDeclContext LAC) {
+ if (const CompoundStmt *CS = dyn_cast<CompoundStmt>(S))
+ return createEndBrace(CS, SM);
+ return PathDiagnosticLocation(getValidSourceLocation(S, LAC, /*End=*/true),
+ SM, SingleLocK);
+}
+
PathDiagnosticLocation
PathDiagnosticLocation::createOperatorLoc(const BinaryOperator *BO,
const SourceManager &SM) {
@@ -339,9 +352,6 @@ PathDiagnosticLocation
else if (const PostStmt *PS = dyn_cast<PostStmt>(&P)) {
S = PS->getStmt();
}
- else if (const CallExitEnd *CEE = dyn_cast<CallExitEnd>(&P)) {
- S = CEE->getCalleeContext()->getCallSite();
- }
return PathDiagnosticLocation(S, SMng, P.getLocationContext());
}
@@ -498,21 +508,36 @@ PathDiagnosticLocation PathDiagnostic::getLocation() const {
// Manipulation of PathDiagnosticCallPieces.
//===----------------------------------------------------------------------===//
-static PathDiagnosticLocation getLastStmtLoc(const ExplodedNode *N,
- const SourceManager &SM) {
- while (N) {
- ProgramPoint PP = N->getLocation();
- if (const StmtPoint *SP = dyn_cast<StmtPoint>(&PP))
- return PathDiagnosticLocation(SP->getStmt(), SM, PP.getLocationContext());
- // FIXME: Handle implicit calls.
- if (const CallExitEnd *CEE = dyn_cast<CallExitEnd>(&PP))
- if (const Stmt *CallSite = CEE->getCalleeContext()->getCallSite())
- return PathDiagnosticLocation(CallSite, SM, PP.getLocationContext());
- if (N->pred_empty())
- break;
- N = *N->pred_begin();
+static PathDiagnosticLocation
+getLocationForCaller(const StackFrameContext *SFC,
+ const LocationContext *CallerCtx,
+ const SourceManager &SM) {
+ const CFGBlock &Block = *SFC->getCallSiteBlock();
+ CFGElement Source = Block[SFC->getIndex()];
+
+ switch (Source.getKind()) {
+ case CFGElement::Invalid:
+ llvm_unreachable("Invalid CFGElement");
+ case CFGElement::Statement:
+ return PathDiagnosticLocation(cast<CFGStmt>(Source).getStmt(),
+ SM, CallerCtx);
+ case CFGElement::Initializer: {
+ const CFGInitializer &Init = cast<CFGInitializer>(Source);
+ return PathDiagnosticLocation(Init.getInitializer()->getInit(),
+ SM, CallerCtx);
}
- return PathDiagnosticLocation();
+ case CFGElement::AutomaticObjectDtor: {
+ const CFGAutomaticObjDtor &Dtor = cast<CFGAutomaticObjDtor>(Source);
+ return PathDiagnosticLocation::createEnd(Dtor.getTriggerStmt(),
+ SM, CallerCtx);
+ }
+ case CFGElement::BaseDtor:
+ case CFGElement::MemberDtor:
+ case CFGElement::TemporaryDtor:
+ llvm_unreachable("not yet implemented!");
+ }
+
+ llvm_unreachable("Unknown CFGElement kind");
}
PathDiagnosticCallPiece *
@@ -520,7 +545,9 @@ PathDiagnosticCallPiece::construct(const ExplodedNode *N,
const CallExitEnd &CE,
const SourceManager &SM) {
const Decl *caller = CE.getLocationContext()->getDecl();
- PathDiagnosticLocation pos = getLastStmtLoc(N, SM);
+ PathDiagnosticLocation pos = getLocationForCaller(CE.getCalleeContext(),
+ CE.getLocationContext(),
+ SM);
return new PathDiagnosticCallPiece(caller, pos);
}
@@ -535,11 +562,11 @@ PathDiagnosticCallPiece::construct(PathPieces &path,
void PathDiagnosticCallPiece::setCallee(const CallEnter &CE,
const SourceManager &SM) {
- const Decl *D = CE.getCalleeContext()->getDecl();
- Callee = D;
- callEnter = PathDiagnosticLocation(CE.getCallExpr(), SM,
- CE.getLocationContext());
- callEnterWithin = PathDiagnosticLocation::createBegin(D, SM);
+ const StackFrameContext *CalleeCtx = CE.getCalleeContext();
+ Callee = CalleeCtx->getDecl();
+
+ callEnterWithin = PathDiagnosticLocation::createBegin(Callee, SM);
+ callEnter = getLocationForCaller(CalleeCtx, CE.getLocationContext(), SM);
}
IntrusiveRefCntPtr<PathDiagnosticEventPiece>
@@ -677,6 +704,7 @@ std::string StackHintGeneratorForSymbol::getMessage(const ExplodedNode *N){
const CallExitEnd *CExit = dyn_cast<CallExitEnd>(&P);
assert(CExit && "Stack Hints should be constructed at CallExitEnd points.");
+ // FIXME: Use CallEvent to abstract this over all calls.
const Stmt *CallSite = CExit->getCalleeContext()->getCallSite();
const CallExpr *CE = dyn_cast_or_null<CallExpr>(CallSite);
if (!CE)
diff --git a/test/Analysis/dtor.cpp b/test/Analysis/dtor.cpp
index 5a14f3025d..8d67f78a4a 100644
--- a/test/Analysis/dtor.cpp
+++ b/test/Analysis/dtor.cpp
@@ -34,3 +34,72 @@ void testSmartPointer() {
}
*mem = 0; // expected-warning{{Use of memory after it is freed}}
}
+
+
+void doSomething();
+void testSmartPointer2() {
+ char *mem = (char*)malloc(4);
+ {
+ SmartPointer Deleter(mem);
+ // Remove dead bindings...
+ doSomething();
+ // destructor called here
+ }
+ *mem = 0; // expected-warning{{Use of memory after it is freed}}
+}
+
+
+class Subclass : public SmartPointer {
+public:
+ Subclass(void *x) : SmartPointer(x) {}
+};
+
+void testSubclassSmartPointer() {
+ char *mem = (char*)malloc(4);
+ {
+ Subclass Deleter(mem);
+ // Remove dead bindings...
+ doSomething();
+ // destructor called here
+ }
+ *mem = 0; // expected-warning{{Use of memory after it is freed}}
+}
+
+
+class MultipleInheritance : public Subclass, public SmartPointer {
+public:
+ MultipleInheritance(void *a, void *b) : Subclass(a), SmartPointer(b) {}
+};
+
+void testMultipleInheritance1() {
+ char *mem = (char*)malloc(4);
+ {
+ MultipleInheritance Deleter(mem, 0);
+ // Remove dead bindings...
+ doSomething();
+ // destructor called here
+ }
+ *mem = 0; // expected-warning{{Use of memory after it is freed}}
+}
+
+void testMultipleInheritance2() {
+ char *mem = (char*)malloc(4);
+ {
+ MultipleInheritance Deleter(0, mem);
+ // Remove dead bindings...
+ doSomething();
+ // destructor called here
+ }
+ *mem = 0; // expected-warning{{Use of memory after it is freed}}
+}
+
+void testMultipleInheritance3() {
+ char *mem = (char*)malloc(4);
+ {
+ MultipleInheritance Deleter(mem, mem);
+ // Remove dead bindings...
+ doSomething();
+ // destructor called here
+ // expected-warning@25 {{Attempt to free released memory}}
+ }
+}