Index: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp +++ lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp @@ -24,10 +24,22 @@ using namespace clang; using namespace ento; -// FIXME: member functions that return a pointer to the container's internal -// buffer may be called on the object many times, so the object's memory -// region should have a list of pointer symbols associated with it. -REGISTER_MAP_WITH_PROGRAMSTATE(RawPtrMap, const MemRegion *, SymbolRef) +using PtrSet = llvm::ImmutableSet; + +// Associate container objects with a set of raw pointer symbols. +REGISTER_MAP_WITH_PROGRAMSTATE(RawPtrMap, const MemRegion *, PtrSet) + +namespace clang { +namespace ento { +template<> struct ProgramStateTrait + : public ProgramStatePartialTrait { + static void *GDMIndex() { + static int Index = 0; + return &Index; + } +}; +} // end namespace ento +} // end namespace clang namespace { @@ -60,8 +72,8 @@ // lookup by region. bool isSymbolTracked(ProgramStateRef State, SymbolRef Sym) { RawPtrMapTy Map = State->get(); - for (const auto Entry : Map) { - if (Entry.second == Sym) + for (const auto &Entry : Map) { + if (Entry.second.contains(Sym)) return true; } return false; @@ -88,11 +100,11 @@ return; SVal Obj = ICall->getCXXThisVal(); - const auto *TypedR = dyn_cast_or_null(Obj.getAsRegion()); - if (!TypedR) + const auto *ObjRegion = dyn_cast_or_null(Obj.getAsRegion()); + if (!ObjRegion) return; - auto *TypeDecl = TypedR->getValueType()->getAsCXXRecordDecl(); + auto *TypeDecl = ObjRegion->getValueType()->getAsCXXRecordDecl(); if (TypeDecl->getName() != "basic_string") return; @@ -101,19 +113,30 @@ if (Call.isCalled(CStrFn) || Call.isCalled(DataFn)) { SVal RawPtr = Call.getReturnValue(); if (!RawPtr.isUnknown()) { - State = State->set(TypedR, RawPtr.getAsSymbol()); + // Start tracking this raw pointer by adding it to the set of symbols + // associated with this container object in the program state map. + PtrSet::Factory &F = State->getStateManager().get_context(); + PtrSet NewSet = F.getEmptySet(); + if (State->contains(ObjRegion)) { + NewSet = *State->get(ObjRegion); + if (NewSet.contains(RawPtr.getAsSymbol())) + return; + } + NewSet = F.add(NewSet, RawPtr.getAsSymbol()); + assert(!NewSet.isEmpty()); + State = State->set(ObjRegion, NewSet); C.addTransition(State); } return; } if (isa(ICall)) { - if (State->contains(TypedR)) { - const SymbolRef *StrBufferPtr = State->get(TypedR); - // FIXME: What if Origin is null? + if (const PtrSet *PS = State->get(ObjRegion)) { + // Mark all pointer symbols associated with the deleted object released. const Expr *Origin = Call.getOriginExpr(); - State = allocation_state::markReleased(State, *StrBufferPtr, Origin); - State = State->remove(TypedR); + for (const auto &Symbol : *PS) + State = allocation_state::markReleased(State, Symbol, Origin); + State = State->remove(ObjRegion); C.addTransition(State); return; } @@ -123,15 +146,24 @@ void DanglingInternalBufferChecker::checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const { ProgramStateRef State = C.getState(); + PtrSet::Factory &F = State->getStateManager().get_context(); RawPtrMapTy RPM = State->get(); - for (const auto Entry : RPM) { - if (!SymReaper.isLive(Entry.second)) - State = State->remove(Entry.first); + for (const auto &Entry : RPM) { if (!SymReaper.isLiveRegion(Entry.first)) { - // Due to incomplete destructor support, some dead regions might still + // Due to incomplete destructor support, some dead regions might // remain in the program state map. Clean them up. State = State->remove(Entry.first); } + if (const PtrSet *OldSet = State->get(Entry.first)) { + PtrSet CleanedUpSet = *OldSet; + for (const auto &Symbol : Entry.second) { + if (!SymReaper.isLive(Symbol)) + CleanedUpSet = F.remove(CleanedUpSet, Symbol); + } + State = CleanedUpSet.isEmpty() + ? State->remove(Entry.first) + : State->set(Entry.first, CleanedUpSet); + } } C.addTransition(State); } Index: test/Analysis/dangling-internal-buffer.cpp =================================================================== --- test/Analysis/dangling-internal-buffer.cpp +++ test/Analysis/dangling-internal-buffer.cpp @@ -108,6 +108,30 @@ // expected-note@-1 {{Use of memory after it is freed}} } +void multiple_symbols(bool cond) { + const char *c1, *d1; + { + std::string s1; + c1 = s1.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}} + d1 = s1.data(); // expected-note {{Pointer to dangling buffer was obtained here}} + const char *local = s1.c_str(); + consume(local); // no-warning + } // expected-note {{Internal buffer is released because the object was destroyed}} + // expected-note@-1 {{Internal buffer is released because the object was destroyed}} + std::string s2; + const char *c2 = s2.c_str(); + if (cond) { + // expected-note@-1 {{Assuming 'cond' is not equal to 0}} + // expected-note@-2 {{Taking true branch}} + // expected-note@-3 {{Assuming 'cond' is 0}} + // expected-note@-4 {{Taking false branch}} + consume(c1); // expected-warning {{Use of memory after it is freed}} + // expected-note@-1 {{Use of memory after it is freed}} + } else { + consume(d1); // expected-warning {{Use of memory after it is freed}} + } // expected-note@-1 {{Use of memory after it is freed}} +} + void deref_after_scope_cstr_ok() { const char *c; std::string s;