rnkovacs created this revision.
rnkovacs added reviewers: NoQ, xazax.hun, george.karpenkov.
Herald added subscribers: mikhail.ramalho, a.sidorin, dkrupp, szepet,
baloghadamsoftware, whisperity.
Previously, the checker only tracked one raw pointer symbol for each container
object. But member functions returning a pointer to the object's inner buffer
may be called on the object several times.
These pointer symbols are now collected in a set inside the program state map
and thus all of them is checked for use-after-free problems.
Repository:
rC Clang
https://reviews.llvm.org/D49057
Files:
lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
test/Analysis/dangling-internal-buffer.cpp
Index: test/Analysis/dangling-internal-buffer.cpp
===================================================================
--- test/Analysis/dangling-internal-buffer.cpp
+++ test/Analysis/dangling-internal-buffer.cpp
@@ -108,6 +108,28 @@
// 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 'Pred' is not equal to 0}} // expected-note@-1 {{Taking true branch}}
+ // expected-note@-2 {{Assuming 'Pred' is 0}} // expected-note@-2 {{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;
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)
+typedef llvm::ImmutableSet<SymbolRef> PtrSet;
+
+// 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<PtrSet>
+ : public ProgramStatePartialTrait<PtrSet> {
+ 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<RawPtrMap>();
- for (const auto Entry : Map) {
- if (Entry.second == Sym)
+ for (const auto &Entry : Map) {
+ if (Entry.second.contains(Sym))
return true;
}
return false;
@@ -88,32 +100,45 @@
return;
SVal Obj = ICall->getCXXThisVal();
- const auto *TypedR = dyn_cast_or_null<TypedValueRegion>(Obj.getAsRegion());
- if (!TypedR)
+ const auto *ObjRegion = dyn_cast_or_null<TypedValueRegion>(Obj.getAsRegion());
+ if (!ObjRegion)
return;
- auto *TypeDecl = TypedR->getValueType()->getAsCXXRecordDecl();
+ auto *TypeDecl = ObjRegion->getValueType()->getAsCXXRecordDecl();
if (TypeDecl->getName() != "basic_string")
return;
ProgramStateRef State = C.getState();
if (Call.isCalled(CStrFn) || Call.isCalled(DataFn)) {
SVal RawPtr = Call.getReturnValue();
if (!RawPtr.isUnknown()) {
- State = State->set<RawPtrMap>(TypedR, RawPtr.getAsSymbol());
- C.addTransition(State);
+ // 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>();
+ PtrSet NewSet = F.getEmptySet();
+ if (State->contains<RawPtrMap>(ObjRegion)) {
+ NewSet = *State->get<RawPtrMap>(ObjRegion);
+ if (NewSet.contains(RawPtr.getAsSymbol()))
+ return;
+ }
+ NewSet = F.add(NewSet, RawPtr.getAsSymbol());
+ if (!NewSet.isEmpty()) {
+ State = State->set<RawPtrMap>(ObjRegion, NewSet);
+ C.addTransition(State);
+ }
}
return;
}
if (isa<CXXDestructorCall>(ICall)) {
- if (State->contains<RawPtrMap>(TypedR)) {
- const SymbolRef *StrBufferPtr = State->get<RawPtrMap>(TypedR);
- // FIXME: What if Origin is null?
+ if (State->contains<RawPtrMap>(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<RawPtrMap>(TypedR);
+ const PtrSet *PS = State->get<RawPtrMap>(ObjRegion);
+ for (const auto &Symbol : *PS)
+ State = allocation_state::markReleased(State, Symbol, Origin);
+ State = State->remove<RawPtrMap>(ObjRegion);
C.addTransition(State);
return;
}
@@ -123,15 +148,25 @@
void DanglingInternalBufferChecker::checkDeadSymbols(SymbolReaper &SymReaper,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
+ PtrSet::Factory &F = State->getStateManager().get_context<PtrSet>();
RawPtrMapTy RPM = State->get<RawPtrMap>();
- for (const auto Entry : RPM) {
- if (!SymReaper.isLive(Entry.second))
- State = State->remove<RawPtrMap>(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<RawPtrMap>(Entry.first);
}
+ if (State->contains<RawPtrMap>(Entry.first)) {
+ const PtrSet *OldSet = State->get<RawPtrMap>(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<RawPtrMap>(Entry.first)
+ : State->set<RawPtrMap>(Entry.first, CleanedUpSet);
+ }
}
C.addTransition(State);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
