Index: cfe/trunk/docs/analyzer/DebugChecks.rst =================================================================== --- cfe/trunk/docs/analyzer/DebugChecks.rst +++ cfe/trunk/docs/analyzer/DebugChecks.rst @@ -138,6 +138,29 @@ 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 ========== Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h =================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h @@ -639,6 +639,7 @@ } 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. Index: cfe/trunk/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp @@ -17,22 +17,26 @@ using namespace ento; namespace { -class ExprInspectionChecker : public Checker< eval::Call > { +class ExprInspectionChecker : public Checker { mutable std::unique_ptr 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 @@ .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 @@ llvm::make_unique(*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(Sym); + C.addTransition(State); +} + +void ExprInspectionChecker::checkDeadSymbols(SymbolReaper &SymReaper, + CheckerContext &C) const { + ProgramStateRef State = C.getState(); + const MarkedSymbolsTy &Syms = State->get(); + for (auto I = Syms.begin(), E = Syms.end(); I != E; ++I) { + SymbolRef Sym = static_cast(*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(*BT, "SYMBOL DEAD", N)); + C.addTransition(State->remove(Sym), N); + } +} + void ExprInspectionChecker::analyzerCrash(const CallExpr *CE, CheckerContext &C) const { LLVM_BUILTIN_TRAP; Index: cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp @@ -171,10 +171,6 @@ // 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 R = X.getAs()) - SymReaper.markLive(R->getRegion()); - // Mark all symbols in the block expr's value live. RSScaner.scan(X); continue; Index: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -2348,8 +2348,12 @@ if (const SymbolicRegion *SymR = dyn_cast(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 @@ // 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(R)) { Index: cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp @@ -391,6 +391,18 @@ void SymbolReaper::markLive(const MemRegion *region) { RegionRoots.insert(region); + markElementIndicesLive(region); +} + +void SymbolReaper::markElementIndicesLive(const MemRegion *region) { + for (auto SR = dyn_cast(region); SR; + SR = dyn_cast(SR->getSuperRegion())) { + if (auto ER = dyn_cast(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) { Index: cfe/trunk/test/Analysis/return-ptr-range.cpp =================================================================== --- cfe/trunk/test/Analysis/return-ptr-range.cpp +++ cfe/trunk/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)}} +} Index: cfe/trunk/test/Analysis/symbol-reaper.c =================================================================== --- cfe/trunk/test/Analysis/symbol-reaper.c +++ cfe/trunk/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