summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--clang/docs/analyzer/DebugChecks.rst23
-rw-r--r--clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h1
-rw-r--r--clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp46
-rw-r--r--clang/lib/StaticAnalyzer/Core/Environment.cpp4
-rw-r--r--clang/lib/StaticAnalyzer/Core/RegionStore.cpp7
-rw-r--r--clang/lib/StaticAnalyzer/Core/SymbolManager.cpp12
-rw-r--r--clang/test/Analysis/return-ptr-range.cpp27
-rw-r--r--clang/test/Analysis/symbol-reaper.c76
8 files changed, 189 insertions, 7 deletions
diff --git a/clang/docs/analyzer/DebugChecks.rst b/clang/docs/analyzer/DebugChecks.rst
index 14d6ae4c4c9..771e39fc439 100644
--- a/clang/docs/analyzer/DebugChecks.rst
+++ b/clang/docs/analyzer/DebugChecks.rst
@@ -138,6 +138,29 @@ ExprInspection checks
clang_analyzer_warnIfReached(); // no-warning
}
+- void clang_analyzer_warnOnDeadSymbol(int);
+
+ Subscribe for a delayed warning when the symbol that represents the value of
+ the argument is garbage-collected by the analyzer.
+
+ When calling 'clang_analyzer_warnOnDeadSymbol(x)', if value of 'x' is a
+ symbol, then this symbol is marked by the ExprInspection checker. Then,
+ during each garbage collection run, the checker sees if the marked symbol is
+ being collected and issues the 'SYMBOL DEAD' warning if it does.
+ This way you know where exactly, up to the line of code, the symbol dies.
+
+ It is unlikely that you call this function after the symbol is already dead,
+ because the very reference to it as the function argument prevents it from
+ dying. However, if the argument is not a symbol but a concrete value,
+ no warning would be issued.
+
+ Example usage::
+
+ do {
+ int x = generate_some_integer();
+ clang_analyzer_warnOnDeadSymbol(x);
+ } while(0); // expected-warning{{SYMBOL DEAD}}
+
Statistics
==========
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
index 05de02db592..9dbfab24b41 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
@@ -639,6 +639,7 @@ public:
}
void markLive(const MemRegion *region);
+ void markElementIndicesLive(const MemRegion *region);
/// \brief Set to the value of the symbolic store after
/// StoreManager::removeDeadBindings has been called.
diff --git a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
index 60c6aa534d1..8f6c20ab190 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
@@ -17,22 +17,26 @@ using namespace clang;
using namespace ento;
namespace {
-class ExprInspectionChecker : public Checker< eval::Call > {
+class ExprInspectionChecker : public Checker<eval::Call, check::DeadSymbols> {
mutable std::unique_ptr<BugType> BT;
void analyzerEval(const CallExpr *CE, CheckerContext &C) const;
void analyzerCheckInlined(const CallExpr *CE, CheckerContext &C) const;
void analyzerWarnIfReached(const CallExpr *CE, CheckerContext &C) const;
void analyzerCrash(const CallExpr *CE, CheckerContext &C) const;
+ void analyzerWarnOnDeadSymbol(const CallExpr *CE, CheckerContext &C) const;
typedef void (ExprInspectionChecker::*FnCheck)(const CallExpr *,
CheckerContext &C) const;
public:
bool evalCall(const CallExpr *CE, CheckerContext &C) const;
+ void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
};
}
+REGISTER_SET_WITH_PROGRAMSTATE(MarkedSymbols, const void *)
+
bool ExprInspectionChecker::evalCall(const CallExpr *CE,
CheckerContext &C) const {
// These checks should have no effect on the surrounding environment
@@ -42,7 +46,10 @@ bool ExprInspectionChecker::evalCall(const CallExpr *CE,
.Case("clang_analyzer_checkInlined",
&ExprInspectionChecker::analyzerCheckInlined)
.Case("clang_analyzer_crash", &ExprInspectionChecker::analyzerCrash)
- .Case("clang_analyzer_warnIfReached", &ExprInspectionChecker::analyzerWarnIfReached)
+ .Case("clang_analyzer_warnIfReached",
+ &ExprInspectionChecker::analyzerWarnIfReached)
+ .Case("clang_analyzer_warnOnDeadSymbol",
+ &ExprInspectionChecker::analyzerWarnOnDeadSymbol)
.Default(nullptr);
if (!Handler)
@@ -137,6 +144,41 @@ void ExprInspectionChecker::analyzerCheckInlined(const CallExpr *CE,
llvm::make_unique<BugReport>(*BT, getArgumentValueString(CE, C), N));
}
+void ExprInspectionChecker::analyzerWarnOnDeadSymbol(const CallExpr *CE,
+ CheckerContext &C) const {
+ if (CE->getNumArgs() == 0)
+ return;
+ SVal Val = C.getSVal(CE->getArg(0));
+ SymbolRef Sym = Val.getAsSymbol();
+ if (!Sym)
+ return;
+
+ ProgramStateRef State = C.getState();
+ State = State->add<MarkedSymbols>(Sym);
+ C.addTransition(State);
+}
+
+void ExprInspectionChecker::checkDeadSymbols(SymbolReaper &SymReaper,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ const MarkedSymbolsTy &Syms = State->get<MarkedSymbols>();
+ for (auto I = Syms.begin(), E = Syms.end(); I != E; ++I) {
+ SymbolRef Sym = static_cast<SymbolRef>(*I);
+ if (!SymReaper.isDead(Sym))
+ continue;
+
+ if (!BT)
+ BT.reset(new BugType(this, "Checking analyzer assumptions", "debug"));
+
+ ExplodedNode *N = C.generateNonFatalErrorNode();
+ if (!N)
+ return;
+
+ C.emitReport(llvm::make_unique<BugReport>(*BT, "SYMBOL DEAD", N));
+ C.addTransition(State->remove<MarkedSymbols>(Sym), N);
+ }
+}
+
void ExprInspectionChecker::analyzerCrash(const CallExpr *CE,
CheckerContext &C) const {
LLVM_BUILTIN_TRAP;
diff --git a/clang/lib/StaticAnalyzer/Core/Environment.cpp b/clang/lib/StaticAnalyzer/Core/Environment.cpp
index d55858db576..e2cb52cb417 100644
--- a/clang/lib/StaticAnalyzer/Core/Environment.cpp
+++ b/clang/lib/StaticAnalyzer/Core/Environment.cpp
@@ -171,10 +171,6 @@ EnvironmentManager::removeDeadBindings(Environment Env,
// Copy the binding to the new map.
EBMapRef = EBMapRef.add(BlkExpr, X);
- // If the block expr's value is a memory region, then mark that region.
- if (Optional<loc::MemRegionVal> R = X.getAs<loc::MemRegionVal>())
- SymReaper.markLive(R->getRegion());
-
// Mark all symbols in the block expr's value live.
RSScaner.scan(X);
continue;
diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
index 1fe83ef7639..a63f6e49627 100644
--- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2348,8 +2348,12 @@ void removeDeadBindingsWorker::VisitCluster(const MemRegion *baseR,
if (const SymbolicRegion *SymR = dyn_cast<SymbolicRegion>(baseR))
SymReaper.markLive(SymR->getSymbol());
- for (ClusterBindings::iterator I = C->begin(), E = C->end(); I != E; ++I)
+ for (ClusterBindings::iterator I = C->begin(), E = C->end(); I != E; ++I) {
+ // Element index of a binding key is live.
+ SymReaper.markElementIndicesLive(I.getKey().getRegion());
+
VisitBinding(I.getData());
+ }
}
void removeDeadBindingsWorker::VisitBinding(SVal V) {
@@ -2370,6 +2374,7 @@ void removeDeadBindingsWorker::VisitBinding(SVal V) {
// If V is a region, then add it to the worklist.
if (const MemRegion *R = V.getAsRegion()) {
AddToWorkList(R);
+ SymReaper.markLive(R);
// All regions captured by a block are also live.
if (const BlockDataRegion *BR = dyn_cast<BlockDataRegion>(R)) {
diff --git a/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp b/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
index df4d22ad652..99b2e147cb4 100644
--- a/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
@@ -391,6 +391,18 @@ void SymbolReaper::markLive(SymbolRef sym) {
void SymbolReaper::markLive(const MemRegion *region) {
RegionRoots.insert(region);
+ markElementIndicesLive(region);
+}
+
+void SymbolReaper::markElementIndicesLive(const MemRegion *region) {
+ for (auto SR = dyn_cast<SubRegion>(region); SR;
+ SR = dyn_cast<SubRegion>(SR->getSuperRegion())) {
+ if (auto ER = dyn_cast<ElementRegion>(SR)) {
+ SVal Idx = ER->getIndex();
+ for (auto SI = Idx.symbol_begin(), SE = Idx.symbol_end(); SI != SE; ++SI)
+ markLive(*SI);
+ }
+ }
}
void SymbolReaper::markInUse(SymbolRef sym) {
diff --git a/clang/test/Analysis/return-ptr-range.cpp b/clang/test/Analysis/return-ptr-range.cpp
new file mode 100644
index 00000000000..0cc17b05042
--- /dev/null
+++ b/clang/test/Analysis/return-ptr-range.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.security.ReturnPtrRange -verify %s
+
+int arr[10];
+int *ptr;
+
+int conjure_index();
+
+int *test_element_index_lifetime() {
+ do {
+ int x = conjure_index();
+ ptr = arr + x;
+ if (x != 20)
+ return arr; // no-warning
+ } while (0);
+ return ptr; // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow)}}
+}
+
+int *test_element_index_lifetime_with_local_ptr() {
+ int *local_ptr;
+ do {
+ int x = conjure_index();
+ local_ptr = arr + x;
+ if (x != 20)
+ return arr; // no-warning
+ } while (0);
+ return local_ptr; // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow)}}
+}
diff --git a/clang/test/Analysis/symbol-reaper.c b/clang/test/Analysis/symbol-reaper.c
new file mode 100644
index 00000000000..4051c38c2e7
--- /dev/null
+++ b/clang/test/Analysis/symbol-reaper.c
@@ -0,0 +1,76 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
+void clang_analyzer_warnOnDeadSymbol(int);
+
+int conjure_index();
+
+void test_that_expr_inspection_works() {
+ do {
+ int x = conjure_index();
+ clang_analyzer_warnOnDeadSymbol(x);
+ } while(0); // expected-warning{{SYMBOL DEAD}}
+}
+
+// These tests verify the reaping of symbols that are only referenced as
+// index values in element regions. Most of the time, depending on where
+// the element region, as Loc value, is stored, it is possible to
+// recover the index symbol in checker code, which is also demonstrated
+// in the return_ptr_range.c test file.
+
+int arr[3];
+
+int *test_element_index_lifetime_in_environment_values() {
+ int *ptr;
+ do {
+ int x = conjure_index();
+ clang_analyzer_warnOnDeadSymbol(x);
+ ptr = arr + x;
+ } while (0);
+ return ptr;
+}
+
+void test_element_index_lifetime_in_store_keys() {
+ do {
+ int x = conjure_index();
+ clang_analyzer_warnOnDeadSymbol(x);
+ arr[x] = 1;
+ if (x) {}
+ } while (0); // no-warning
+}
+
+int *ptr;
+void test_element_index_lifetime_in_store_values() {
+ do {
+ int x = conjure_index();
+ clang_analyzer_warnOnDeadSymbol(x);
+ ptr = arr + x;
+ } while (0); // no-warning
+}
+
+struct S1 {
+ int field;
+};
+struct S2 {
+ struct S1 array[5];
+} s2;
+
+void test_element_index_lifetime_with_complicated_hierarchy_of_regions() {
+ do {
+ int x = conjure_index();
+ clang_analyzer_warnOnDeadSymbol(x);
+ s2.array[x].field = 1;
+ if (x) {}
+ } while (0); // no-warning
+}
+
+// Test below checks lifetime of SymbolRegionValue in certain conditions.
+
+int **ptrptr;
+void test_region_lifetime_as_store_value(int *x) {
+ clang_analyzer_warnOnDeadSymbol((int) x);
+ *x = 1;
+ ptrptr = &x;
+ (void)0; // No-op; make sure the environment forgets things and the GC runs.
+ clang_analyzer_eval(**ptrptr); // expected-warning{{TRUE}}
+} // no-warning
OpenPOWER on IntegriCloud