Index: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp +++ lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp @@ -24,10 +24,23 @@ 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) + +// This is a trick to gain access to PtrSet's Factory. +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 { @@ -61,7 +74,7 @@ bool isSymbolTracked(ProgramStateRef State, SymbolRef Sym) { RawPtrMapTy Map = State->get(); for (const auto Entry : Map) { - if (Entry.second == Sym) + if (Entry.second.contains(Sym)) return true; } return false; @@ -88,11 +101,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; @@ -100,20 +113,30 @@ if (Call.isCalled(CStrFn) || Call.isCalled(DataFn)) { SVal RawPtr = Call.getReturnValue(); - if (!RawPtr.isUnknown()) { - State = State->set(TypedR, RawPtr.getAsSymbol()); + if (SymbolRef Sym = RawPtr.getAsSymbol(/*IncludeBaseRegions=*/true)) { + // 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(); + const PtrSet *SetPtr = State->get(ObjRegion); + PtrSet Set = SetPtr ? *SetPtr : F.getEmptySet(); + assert(C.wasInlined || !Set.contains(Sym)); + Set = F.add(Set, Sym); + State = State->set(ObjRegion, Set); 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) { + // NOTE: `Origin` may be null, and will be stored so in the symbol's + // `RefState` in MallocChecker's `RegionState` program state map. + 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); 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;