r336835 - [analyzer] Track multiple raw pointer symbols in DanglingInternalBufferChecker.
Reka Kovacs via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 11 12:08:02 PDT 2018
Author: rkovacs
Date: Wed Jul 11 12:08:02 2018
New Revision: 336835
URL: http://llvm.org/viewvc/llvm-project?rev=336835&view=rev
Log:
[analyzer] Track multiple raw pointer symbols in DanglingInternalBufferChecker.
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.
Differential Revision: https://reviews.llvm.org/D49057
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
cfe/trunk/test/Analysis/dangling-internal-buffer.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp?rev=336835&r1=336834&r2=336835&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp Wed Jul 11 12:08:02 2018
@@ -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<SymbolRef>;
+
+// 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<PtrSet>
+ : public ProgramStatePartialTrait<PtrSet> {
+ static void *GDMIndex() {
+ static int Index = 0;
+ return &Index;
+ }
+};
+} // end namespace ento
+} // end namespace clang
namespace {
@@ -61,7 +74,7 @@ public:
bool isSymbolTracked(ProgramStateRef State, SymbolRef Sym) {
RawPtrMapTy Map = State->get<RawPtrMap>();
for (const auto Entry : Map) {
- if (Entry.second == Sym)
+ if (Entry.second.contains(Sym))
return true;
}
return false;
@@ -88,11 +101,11 @@ void DanglingInternalBufferChecker::chec
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;
@@ -100,20 +113,30 @@ void DanglingInternalBufferChecker::chec
if (Call.isCalled(CStrFn) || Call.isCalled(DataFn)) {
SVal RawPtr = Call.getReturnValue();
- if (!RawPtr.isUnknown()) {
- State = State->set<RawPtrMap>(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<PtrSet>();
+ const PtrSet *SetPtr = State->get<RawPtrMap>(ObjRegion);
+ PtrSet Set = SetPtr ? *SetPtr : F.getEmptySet();
+ assert(C.wasInlined || !Set.contains(Sym));
+ Set = F.add(Set, Sym);
+ State = State->set<RawPtrMap>(ObjRegion, Set);
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 (const PtrSet *PS = State->get<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);
+ 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<RawPtrMap>(ObjRegion);
C.addTransition(State);
return;
}
@@ -123,15 +146,24 @@ void DanglingInternalBufferChecker::chec
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);
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 (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);
}
Modified: cfe/trunk/test/Analysis/dangling-internal-buffer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dangling-internal-buffer.cpp?rev=336835&r1=336834&r2=336835&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/dangling-internal-buffer.cpp (original)
+++ cfe/trunk/test/Analysis/dangling-internal-buffer.cpp Wed Jul 11 12:08:02 2018
@@ -108,6 +108,30 @@ void deref_after_scope_char32_t_data() {
// expected-note at -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 at -1 {{Internal buffer is released because the object was destroyed}}
+ std::string s2;
+ const char *c2 = s2.c_str();
+ if (cond) {
+ // expected-note at -1 {{Assuming 'cond' is not equal to 0}}
+ // expected-note at -2 {{Taking true branch}}
+ // expected-note at -3 {{Assuming 'cond' is 0}}
+ // expected-note at -4 {{Taking false branch}}
+ consume(c1); // expected-warning {{Use of memory after it is freed}}
+ // expected-note at -1 {{Use of memory after it is freed}}
+ } else {
+ consume(d1); // expected-warning {{Use of memory after it is freed}}
+ } // expected-note at -1 {{Use of memory after it is freed}}
+}
+
void deref_after_scope_cstr_ok() {
const char *c;
std::string s;
More information about the cfe-commits
mailing list