summaryrefslogtreecommitdiffstats
path: root/clang
diff options
context:
space:
mode:
Diffstat (limited to 'clang')
-rw-r--r--clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h11
-rw-r--r--clang/lib/StaticAnalyzer/Core/ExprEngine.cpp4
-rw-r--r--clang/lib/StaticAnalyzer/Core/SymbolManager.cpp10
-rw-r--r--clang/test/Analysis/diagnostics/dtors.cpp19
-rw-r--r--clang/test/Analysis/symbol-reaper.cpp60
-rw-r--r--clang/unittests/StaticAnalyzer/CMakeLists.txt1
-rw-r--r--clang/unittests/StaticAnalyzer/SymbolReaperTest.cpp121
7 files changed, 213 insertions, 13 deletions
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 86b776afb82..c9a65d92b16 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -131,6 +131,9 @@ private:
/// SymMgr - Object that manages the symbol information.
SymbolManager &SymMgr;
+ /// MRMgr - MemRegionManager object that creates memory regions.
+ MemRegionManager &MRMgr;
+
/// svalBuilder - SValBuilder object that creates SVals from expressions.
SValBuilder &svalBuilder;
@@ -180,6 +183,10 @@ public:
AnalysisManager &getAnalysisManager() override { return AMgr; }
+ AnalysisDeclContextManager &getAnalysisDeclContextManager() {
+ return AMgr.getAnalysisDeclContextManager();
+ }
+
CheckerManager &getCheckerManager() const {
return *AMgr.getCheckerManager();
}
@@ -387,9 +394,9 @@ public:
return StateMgr.getBasicVals();
}
- // FIXME: Remove when we migrate over to just using ValueManager.
SymbolManager &getSymbolManager() { return SymMgr; }
- const SymbolManager &getSymbolManager() const { return SymMgr; }
+ MemRegionManager &getRegionManager() { return MRMgr; }
+
// Functions for external checking of whether we have unfinished work
bool wasBlocksExhausted() const { return Engine.wasBlocksExhausted(); }
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 151eef56fec..cbc022f49aa 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -198,7 +198,9 @@ ExprEngine::ExprEngine(cross_tu::CrossTranslationUnitContext &CTU,
mgr.getConstraintManagerCreator(), G.getAllocator(),
this),
SymMgr(StateMgr.getSymbolManager()),
- svalBuilder(StateMgr.getSValBuilder()), ObjCNoRet(mgr.getASTContext()),
+ MRMgr(StateMgr.getRegionManager()),
+ svalBuilder(StateMgr.getSValBuilder()),
+ ObjCNoRet(mgr.getASTContext()),
BR(mgr, *this),
VisitedCallees(VisitedCalleesIn), HowToInline(HowToInlineIn) {
unsigned TrimInterval = mgr.options.GraphTrimInterval;
diff --git a/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp b/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
index 66273f099a3..c60c3d0f376 100644
--- a/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
@@ -405,7 +405,7 @@ void SymbolReaper::markLive(SymbolRef sym) {
}
void SymbolReaper::markLive(const MemRegion *region) {
- RegionRoots.insert(region);
+ RegionRoots.insert(region->getBaseRegion());
markElementIndicesLive(region);
}
@@ -426,11 +426,15 @@ void SymbolReaper::markInUse(SymbolRef sym) {
}
bool SymbolReaper::isLiveRegion(const MemRegion *MR) {
+ // TODO: For now, liveness of a memory region is equivalent to liveness of its
+ // base region. In fact we can do a bit better: say, if a particular FieldDecl
+ // is not used later in the path, we can diagnose a leak of a value within
+ // that field earlier than, say, the variable that contains the field dies.
+ MR = MR->getBaseRegion();
+
if (RegionRoots.count(MR))
return true;
- MR = MR->getBaseRegion();
-
if (const auto *SR = dyn_cast<SymbolicRegion>(MR))
return isLive(SR->getSymbol());
diff --git a/clang/test/Analysis/diagnostics/dtors.cpp b/clang/test/Analysis/diagnostics/dtors.cpp
index 094917e432f..b3fe7ec803a 100644
--- a/clang/test/Analysis/diagnostics/dtors.cpp
+++ b/clang/test/Analysis/diagnostics/dtors.cpp
@@ -1,9 +1,11 @@
-// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,cplusplus -verify %s
-
-// expected-no-diagnostics
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,cplusplus -analyzer-output=text -verify %s
namespace no_crash_on_delete_dtor {
-// We were crashing when producing diagnostics for this code.
+// We were crashing when producing diagnostics for this code, but not for the
+// report that it currently emits. Instead, Static Analyzer was thinking that
+// p.get()->foo() is a null dereference because it was dropping
+// constraints over x too early and took a different branch next time
+// we call .get().
struct S {
void foo();
~S();
@@ -14,12 +16,15 @@ struct smart_ptr {
S *s;
smart_ptr(S *);
S *get() {
- return (x || 0) ? nullptr : s;
+ return (x || 0) ? nullptr : s; // expected-note{{Left side of '||' is false}}
+ // expected-note@-1{{'?' condition is false}}
+ // expected-warning@-2{{Use of memory after it is freed}}
+ // expected-note@-3{{Use of memory after it is freed}}
}
};
void bar(smart_ptr p) {
- delete p.get();
- p.get()->foo();
+ delete p.get(); // expected-note{{Memory is released}}
+ p.get()->foo(); // expected-note{{Calling 'smart_ptr::get'}}
}
} // namespace no_crash_on_delete_dtor
diff --git a/clang/test/Analysis/symbol-reaper.cpp b/clang/test/Analysis/symbol-reaper.cpp
new file mode 100644
index 00000000000..f3f6cb382e0
--- /dev/null
+++ b/clang/test/Analysis/symbol-reaper.cpp
@@ -0,0 +1,60 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
+void clang_analyzer_warnOnDeadSymbol(int);
+
+namespace test_dead_region_with_live_subregion_in_environment {
+int glob;
+
+struct A {
+ int x;
+
+ void foo() {
+ // FIXME: Maybe just let clang_analyzer_eval() work within callees already?
+ // The glob variable shouldn't keep our symbol alive because
+ // 'x != 0' is concrete 'true'.
+ glob = (x != 0);
+ }
+};
+
+void test_A(A a) {
+ if (a.x == 0)
+ return;
+
+ clang_analyzer_warnOnDeadSymbol(a.x);
+
+ // What we're testing is that a.x is alive until foo() exits.
+ a.foo(); // no-warning // (i.e., no 'SYMBOL DEAD' yet)
+
+ // Let's see if constraints on a.x were known within foo().
+ clang_analyzer_eval(glob); // expected-warning{{TRUE}}
+ // expected-warning@-1{{SYMBOL DEAD}}
+}
+
+struct B {
+ A a;
+ int y;
+};
+
+A &noop(A &a) {
+ // This function ensures that the 'b' expression within its argument
+ // would be cleaned up before its call, so that only 'b.a' remains
+ // in the Environment.
+ return a;
+}
+
+
+void test_B(B b) {
+ if (b.a.x == 0)
+ return;
+
+ clang_analyzer_warnOnDeadSymbol(b.a.x);
+
+ // What we're testing is that b.a.x is alive until foo() exits.
+ noop(b.a).foo(); // no-warning // (i.e., no 'SYMBOL DEAD' yet)
+
+ // Let's see if constraints on a.x were known within foo().
+ clang_analyzer_eval(glob); // expected-warning{{TRUE}}
+ // expected-warning@-1{{SYMBOL DEAD}}
+}
+} // namespace test_dead_region_with_live_subregion_in_environment
diff --git a/clang/unittests/StaticAnalyzer/CMakeLists.txt b/clang/unittests/StaticAnalyzer/CMakeLists.txt
index 3036dec1676..8afc670f89c 100644
--- a/clang/unittests/StaticAnalyzer/CMakeLists.txt
+++ b/clang/unittests/StaticAnalyzer/CMakeLists.txt
@@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS
add_clang_unittest(StaticAnalysisTests
AnalyzerOptionsTest.cpp
RegisterCustomCheckersTest.cpp
+ SymbolReaperTest.cpp
)
target_link_libraries(StaticAnalysisTests
diff --git a/clang/unittests/StaticAnalyzer/SymbolReaperTest.cpp b/clang/unittests/StaticAnalyzer/SymbolReaperTest.cpp
new file mode 100644
index 00000000000..9f74a644a69
--- /dev/null
+++ b/clang/unittests/StaticAnalyzer/SymbolReaperTest.cpp
@@ -0,0 +1,121 @@
+//===- unittests/StaticAnalyzer/SymbolReaperTest.cpp ----------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/CrossTU/CrossTranslationUnit.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
+#include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace ento {
+namespace {
+
+using namespace ast_matchers;
+
+// A re-usable consumer that constructs ExprEngine out of CompilerInvocation.
+// TODO: Actually re-use it when we write our second test.
+class ExprEngineConsumer : public ASTConsumer {
+protected:
+ CompilerInstance &C;
+
+private:
+ // We need to construct all of these in order to construct ExprEngine.
+ CheckerManager ChkMgr;
+ cross_tu::CrossTranslationUnitContext CTU;
+ PathDiagnosticConsumers Consumers;
+ AnalysisManager AMgr;
+ SetOfConstDecls VisitedCallees;
+ FunctionSummariesTy FS;
+
+protected:
+ ExprEngine Eng;
+
+ // Find a declaration in the current AST by name. This has nothing to do
+ // with ExprEngine but turns out to be handy.
+ // TODO: There's probably a better place for it.
+ template <typename T>
+ const T *findDeclByName(const Decl *Where, StringRef Name) {
+ auto Matcher = decl(hasDescendant(namedDecl(hasName(Name)).bind("d")));
+ auto Matches = match(Matcher, *Where, Eng.getContext());
+ assert(Matches.size() == 1 && "Ambiguous name!");
+ const T *Node = selectFirst<T>("d", Matches);
+ assert(Node && "Name not found!");
+ return Node;
+ }
+
+public:
+ ExprEngineConsumer(CompilerInstance &C)
+ : C(C), ChkMgr(C.getASTContext(), *C.getAnalyzerOpts()), CTU(C),
+ Consumers(),
+ AMgr(C.getASTContext(), C.getDiagnostics(), Consumers,
+ CreateRegionStoreManager, CreateRangeConstraintManager, &ChkMgr,
+ *C.getAnalyzerOpts()),
+ VisitedCallees(), FS(),
+ Eng(CTU, AMgr, &VisitedCallees, &FS, ExprEngine::Inline_Regular) {}
+};
+
+class SuperRegionLivenessConsumer : public ExprEngineConsumer {
+ void performTest(const Decl *D) {
+ const auto *FD = findDeclByName<FieldDecl>(D, "x");
+ const auto *VD = findDeclByName<VarDecl>(D, "s");
+ assert(FD && VD);
+
+ // The variable must belong to a stack frame,
+ // otherwise SymbolReaper would think it's a global.
+ const StackFrameContext *SFC =
+ Eng.getAnalysisDeclContextManager().getStackFrame(D);
+
+ // Create regions for 's' and 's.x'.
+ const VarRegion *VR = Eng.getRegionManager().getVarRegion(VD, SFC);
+ const FieldRegion *FR = Eng.getRegionManager().getFieldRegion(FD, VR);
+
+ // Pass a null location context to the SymbolReaper so that
+ // it was thinking that the variable is dead.
+ SymbolReaper SymReaper((StackFrameContext *)nullptr, (Stmt *)nullptr,
+ Eng.getSymbolManager(), Eng.getStoreManager());
+
+ SymReaper.markLive(FR);
+ EXPECT_TRUE(SymReaper.isLiveRegion(VR));
+ }
+
+public:
+ SuperRegionLivenessConsumer(CompilerInstance &C) : ExprEngineConsumer(C) {}
+ ~SuperRegionLivenessConsumer() override {}
+
+ bool HandleTopLevelDecl(DeclGroupRef DG) override {
+ for (const auto *D : DG)
+ performTest(D);
+ return true;
+ }
+};
+
+class SuperRegionLivenessAction: public ASTFrontendAction {
+public:
+ SuperRegionLivenessAction() {}
+ std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &Compiler,
+ StringRef File) override {
+ auto Consumer = llvm::make_unique<SuperRegionLivenessConsumer>(Compiler);
+ return Consumer;
+ }
+};
+
+// Test that marking s.x as live would also make s live.
+TEST(SymbolReaper, SuperRegionLiveness) {
+ EXPECT_TRUE(tooling::runToolOnCode(new SuperRegionLivenessAction,
+ "void foo() { struct S { int x; } s; }"));
+}
+
+} // namespace
+} // namespace ento
+} // namespace clang
OpenPOWER on IntegriCloud