rnkovacs updated this revision to Diff 157278.
rnkovacs marked 2 inline comments as done.
rnkovacs added a comment.
Fix note for function pointers & handle argument counting in member operator
calls.
I also refactored the code a little, because after moving things from
`checkPreCall` to `checkPostCall`, the structure was a bit confusing.
https://reviews.llvm.org/D49656
Files:
lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
test/Analysis/inner-pointer.cpp
Index: test/Analysis/inner-pointer.cpp
===================================================================
--- test/Analysis/inner-pointer.cpp
+++ test/Analysis/inner-pointer.cpp
@@ -38,13 +38,26 @@
typedef basic_string<char16_t> u16string;
typedef basic_string<char32_t> u32string;
+template <typename T>
+void func_ref(T &a);
+
+template <typename T>
+void func_const_ref(const T &a);
+
+template <typename T>
+void func_value(T a);
+
} // end namespace std
void consume(const char *) {}
void consume(const wchar_t *) {}
void consume(const char16_t *) {}
void consume(const char32_t *) {}
+//=--------------------------------------=//
+// `std::string` member functions //
+//=--------------------------------------=//
+
void deref_after_scope_char(bool cond) {
const char *c, *d;
{
@@ -151,6 +164,19 @@
} // expected-note@-1 {{Use of memory after it is freed}}
}
+void deref_after_scope_ok(bool cond) {
+ const char *c, *d;
+ std::string s;
+ {
+ c = s.c_str();
+ d = s.data();
+ }
+ if (cond)
+ consume(c); // no-warning
+ else
+ consume(d); // no-warning
+}
+
void deref_after_equals() {
const char *c;
std::string s = "hello";
@@ -277,15 +303,49 @@
// expected-note@-1 {{Use of memory after it is freed}}
}
-void deref_after_scope_ok(bool cond) {
- const char *c, *d;
+//=---------------------------=//
+// Other STL functions //
+//=---------------------------=//
+
+void STL_func_ref() {
+ const char *c;
std::string s;
- {
- c = s.c_str();
- d = s.data();
- }
- if (cond)
- consume(c); // no-warning
- else
- consume(d); // no-warning
+ c = s.c_str(); // expected-note {{Dangling inner pointer obtained here}}
+ std::func_ref(s); // expected-note {{Inner pointer invalidated by call to 'func_ref'}}
+ consume(c); // expected-warning {{Use of memory after it is freed}}
+ // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void STL_func_const_ref() {
+ const char *c;
+ std::string s;
+ c = s.c_str();
+ std::func_const_ref(s);
+ consume(c); // no-warning
+}
+
+void STL_func_value() {
+ const char *c;
+ std::string s;
+ c = s.c_str();
+ std::func_value(s);
+ consume(c); // no-warning
+}
+
+void func_ptr_known() {
+ const char *c;
+ std::string s;
+ void (*func_ptr)(std::string &) = std::func_ref<std::string>;
+ c = s.c_str(); // expected-note {{Dangling inner pointer obtained here}}
+ func_ptr(s); // expected-note {{Inner pointer invalidated by call to 'func_ref'}}
+ consume(c); // expected-warning {{Use of memory after it is freed}}
+ // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void func_ptr_unknown(void (*func_ptr)(std::string &)) {
+ const char *c;
+ std::string s;
+ c = s.c_str();
+ func_ptr(s);
+ consume(c); // no-warning
}
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2930,6 +2930,11 @@
OS << MemCallE->getMethodDecl()->getNameAsString();
} else if (const auto *OpCallE = dyn_cast<CXXOperatorCallExpr>(S)) {
OS << OpCallE->getDirectCallee()->getNameAsString();
+ } else if (const auto *CallE = dyn_cast<CallExpr>(S)) {
+ auto &CEMgr = BRC.getStateManager().getCallEventManager();
+ CallEventRef<> Call = CEMgr.getSimpleCall(CallE, state, CurrentLC);
+ const auto *D = dyn_cast_or_null<NamedDecl>(Call->getDecl());
+ OS << (D ? D->getNameAsString() : "unknown");
}
OS << "'";
}
Index: lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
+++ lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
@@ -91,37 +91,53 @@
ReserveFn("reserve"), ResizeFn("resize"),
ShrinkToFitFn("shrink_to_fit"), SwapFn("swap") {}
- /// Check whether the function called on the container object is a
- /// member function that potentially invalidates pointers referring
- /// to the objects's internal buffer.
- bool mayInvalidateBuffer(const CallEvent &Call) const;
-
- /// Record the connection between the symbol returned by c_str() and the
- /// corresponding string object region in the ProgramState. Mark the symbol
- /// released if the string object is destroyed.
+ /// Check if the object of this member function call is a `basic_string`.
+ bool isCalledOnStringObject(const CXXInstanceCall *ICall) const;
+
+ /// Check whether the called member function potentially invalidates
+ /// pointers referring to the container object's inner buffer.
+ bool isInvalidatingMemberFunction(const CallEvent &Call) const;
+
+ /// Mark pointer symbols associated with the given memory region released
+ /// in the program state.
+ void markPtrSymbolsReleased(const CallEvent &Call, ProgramStateRef State,
+ const MemRegion *ObjRegion,
+ CheckerContext &C) const;
+
+ /// Standard library functions that take a non-const `basic_string` argument by
+ /// reference may invalidate its inner pointers. Check for these cases and
+ /// mark the pointers released.
+ void checkFunctionArguments(const CallEvent &Call, ProgramStateRef State,
+ CheckerContext &C) const;
+
+ /// Record the connection between raw pointers referring to a container
+ /// object's inner buffer and the object's memory region in the program state.
+ /// Mark potentially invalidated pointers released.
void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
- /// Clean up the ProgramState map.
+ /// Clean up the program state map.
void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
};
} // end anonymous namespace
-// [string.require]
-//
-// "References, pointers, and iterators referring to the elements of a
-// basic_string sequence may be invalidated by the following uses of that
-// basic_string object:
-//
-// -- TODO: As an argument to any standard library function taking a reference
-// to non-const basic_string as an argument. For example, as an argument to
-// non-member functions swap(), operator>>(), and getline(), or as an argument
-// to basic_string::swap().
-//
-// -- Calling non-const member functions, except operator[], at, front, back,
-// begin, rbegin, end, and rend."
-//
-bool InnerPointerChecker::mayInvalidateBuffer(const CallEvent &Call) const {
+bool InnerPointerChecker::isCalledOnStringObject(
+ const CXXInstanceCall *ICall) const {
+ const auto *ObjRegion =
+ dyn_cast_or_null<TypedValueRegion>(ICall->getCXXThisVal().getAsRegion());
+ if (!ObjRegion)
+ return false;
+
+ QualType ObjTy = ObjRegion->getValueType();
+ if (ObjTy.isNull() ||
+ ObjTy->getAsCXXRecordDecl()->getName() != "basic_string")
+ return false;
+
+ return true;
+}
+
+bool InnerPointerChecker::isInvalidatingMemberFunction(
+ const CallEvent &Call) const {
if (const auto *MemOpCall = dyn_cast<CXXMemberOperatorCall>(&Call)) {
OverloadedOperatorKind Opc = MemOpCall->getOriginExpr()->getOperator();
if (Opc == OO_Equal || Opc == OO_PlusEqual)
@@ -137,52 +153,110 @@
Call.isCalled(SwapFn));
}
-void InnerPointerChecker::checkPostCall(const CallEvent &Call,
- CheckerContext &C) const {
- const auto *ICall = dyn_cast<CXXInstanceCall>(&Call);
- if (!ICall)
+void InnerPointerChecker::markPtrSymbolsReleased(const CallEvent &Call,
+ ProgramStateRef State,
+ const MemRegion *MR,
+ CheckerContext &C) const {
+ if (const PtrSet *PS = State->get<RawPtrMap>(MR)) {
+ const Expr *Origin = Call.getOriginExpr();
+ 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>(MR);
+ C.addTransition(State);
return;
+ }
+}
- SVal Obj = ICall->getCXXThisVal();
- const auto *ObjRegion = dyn_cast_or_null<TypedValueRegion>(Obj.getAsRegion());
- if (!ObjRegion)
- return;
+void InnerPointerChecker::checkFunctionArguments(const CallEvent &Call,
+ ProgramStateRef State,
+ CheckerContext &C) const {
+ if (const auto *FC = dyn_cast<AnyFunctionCall>(&Call)) {
+ const FunctionDecl *FD = FC->getDecl();
+ if (!FD || !FD->isInStdNamespace())
+ return;
- auto *TypeDecl = ObjRegion->getValueType()->getAsCXXRecordDecl();
- if (TypeDecl->getName() != "basic_string")
- return;
+ for (unsigned I = 0, E = FD->getNumParams(); I != E; ++I) {
+ QualType ParamTy = FD->getParamDecl(I)->getType();
+ if (!ParamTy->isReferenceType() ||
+ ParamTy->getPointeeType().isConstQualified())
+ continue;
- ProgramStateRef State = C.getState();
+ // In case of member operator calls, `this` is counted as an
+ // argument but not as a parameter.
+ const auto *MemberOpCall = dyn_cast<CXXMemberOperatorCall>(FC);
+ unsigned ArgI = MemberOpCall ? I+1 : I;
- if (Call.isCalled(CStrFn) || Call.isCalled(DataFn)) {
- SVal RawPtr = Call.getReturnValue();
- 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);
+ SVal Arg = FC->getArgSVal(ArgI);
+ const auto *ArgRegion =
+ dyn_cast_or_null<TypedValueRegion>(Arg.getAsRegion());
+ if (!ArgRegion)
+ continue;
+
+ markPtrSymbolsReleased(Call, State, ArgRegion, C);
}
- return;
}
+}
- if (mayInvalidateBuffer(Call)) {
- if (const PtrSet *PS = State->get<RawPtrMap>(ObjRegion)) {
- // Mark all pointer symbols associated with the deleted object released.
- const Expr *Origin = Call.getOriginExpr();
- 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);
+// [string.require]
+//
+// "References, pointers, and iterators referring to the elements of a
+// basic_string sequence may be invalidated by the following uses of that
+// basic_string object:
+//
+// -- As an argument to any standard library function taking a reference
+// to non-const basic_string as an argument. For example, as an argument to
+// non-member functions swap(), operator>>(), and getline(), or as an argument
+// to basic_string::swap().
+//
+// -- Calling non-const member functions, except operator[], at, front, back,
+// begin, rbegin, end, and rend."
+
+void InnerPointerChecker::checkPostCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+
+ if (const auto *ICall = dyn_cast<CXXInstanceCall>(&Call)) {
+
+ if (isCalledOnStringObject(ICall)) {
+ const auto *ObjRegion = dyn_cast_or_null<TypedValueRegion>(
+ ICall->getCXXThisVal().getAsRegion());
+
+ if (Call.isCalled(CStrFn) || Call.isCalled(DataFn)) {
+ SVal RawPtr = Call.getReturnValue();
+ 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;
+ }
+
+ // Check [string.require] / second point.
+ if (isInvalidatingMemberFunction(Call)) {
+ markPtrSymbolsReleased(Call, State, ObjRegion, C);
+ return;
}
- State = State->remove<RawPtrMap>(ObjRegion);
- C.addTransition(State);
+
+ } else {
+ // Check [string.require] / first point for member functions.
+ checkFunctionArguments(Call, State, C);
return;
}
+
+ } else {
+ // Check [string.require] / first point for non-member functions.
+ checkFunctionArguments(Call, State, C);
}
}
@@ -216,7 +290,6 @@
const ExplodedNode *PrevN,
BugReporterContext &BRC,
BugReport &BR) {
-
if (!isSymbolTracked(N->getState(), PtrToBuf) ||
isSymbolTracked(PrevN->getState(), PtrToBuf))
return nullptr;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
