rnkovacs created this revision.
rnkovacs added reviewers: NoQ, xazax.hun, george.karpenkov.
Herald added subscribers: mikhail.ramalho, a.sidorin, dkrupp, szepet,
baloghadamsoftware, whisperity.
According to the standard, pointers referring to the elements of a
`basic_string` sequence may also be invalidated if they are used as an argument
to any standard library function taking a reference to non-const `basic_string`
as an argument. This patch makes InnerPointerChecker warn for these cases as
well.
Repository:
rC Clang
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,6 +38,15 @@
typedef basic_string<char16_t> u16string;
typedef basic_string<char32_t> u32string;
+template< class T >
+void func_ref(T& a, T& b);
+
+template< class T >
+void func_const_ref(const T& a, const T& b);
+
+template< class T >
+void func_value(T a, T b);
+
} // end namespace std
void consume(const char *) {}
@@ -277,6 +286,31 @@
// expected-note@-1 {{Use of memory after it is freed}}
}
+void non_member_func_ref() {
+ const char *c;
+ std::string s1, s2;
+ c = s1.c_str(); // expected-note {{Dangling inner pointer obtained here}}
+ std::func_ref(s1, s2); // 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 non_member_func_const_ref() {
+ const char *c;
+ std::string s1, s2;
+ c = s1.c_str();
+ std::func_const_ref(s1, s2);
+ consume(c); // no-warning
+}
+
+void non_member_func_value() {
+ const char *c;
+ std::string s1, s2;
+ c = s1.c_str();
+ std::func_value(s1, s2);
+ consume(c); // no-warning
+}
+
void deref_after_scope_ok(bool cond) {
const char *c, *d;
std::string s;
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2930,6 +2930,8 @@
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)) {
+ OS << CallE->getDirectCallee()->getNameAsString();
}
OS << "'";
}
Index: lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
+++ lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
@@ -45,7 +45,7 @@
namespace {
class InnerPointerChecker
- : public Checker<check::DeadSymbols, check::PostCall> {
+ : public Checker<check::DeadSymbols, check::PostCall, check::PreCall> {
CallDescription AppendFn, AssignFn, ClearFn, CStrFn, DataFn, EraseFn,
InsertFn, PopBackFn, PushBackFn, ReplaceFn, ReserveFn, ResizeFn,
@@ -91,37 +91,33 @@
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;
+ /// Check whether the called member function potentially invalidates
+ /// pointers referring to the container object's inner buffer.
+ bool isInvalidatingMemberFunction(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.
+ /// 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;
+
+ /// 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 pointers potentially invalidated by member functions released.
void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
- /// Clean up the ProgramState map.
+ /// Mark pointers potentially invalidated by functions taking the object
+ /// by non-const reference released.
+ void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
+
+ /// 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::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,53 +133,98 @@
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;
+// [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."
- auto *TypeDecl = ObjRegion->getValueType()->getAsCXXRecordDecl();
- if (TypeDecl->getName() != "basic_string")
- return;
+void InnerPointerChecker::checkPostCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ if (const auto *ICall = dyn_cast<CXXInstanceCall>(&Call)) {
+ SVal Obj = ICall->getCXXThisVal();
+ const auto *ObjRegion =
+ dyn_cast_or_null<TypedValueRegion>(Obj.getAsRegion());
+ if (!ObjRegion)
+ return;
- ProgramStateRef State = C.getState();
+ QualType ObjTy = ObjRegion->getValueType();
+ if (ObjTy.isNull())
+ return;
+ if (ObjTy->getAsCXXRecordDecl()->getName() != "basic_string")
+ return;
+
+ ProgramStateRef State = C.getState();
+ 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;
+ }
- 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);
+ // Check [string.require] / second point.
+ if (isInvalidatingMemberFunction(Call)) {
+ markPtrSymbolsReleased(Call, State, ObjRegion, C);
}
- return;
}
+ 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);
- }
- State = State->remove<RawPtrMap>(ObjRegion);
- C.addTransition(State);
- return;
+void InnerPointerChecker::checkPreCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+
+ // Check [string.require] / first point.
+ if (const auto *FC = dyn_cast<AnyFunctionCall>(&Call)) {
+ const FunctionDecl *FD = FC->getDecl();
+ for (unsigned I = 0, E = FD->getNumParams(); I != E; ++I) {
+ QualType ParamTy = FD->getParamDecl(I)->getType();
+ if (!ParamTy->isReferenceType() ||
+ ParamTy->getPointeeType().isConstQualified())
+ continue;
+ SVal Arg = FC->getArgSVal(I);
+ const auto *ArgRegion =
+ dyn_cast_or_null<TypedValueRegion>(Arg.getAsRegion());
+ if (!ArgRegion)
+ continue;
+ markPtrSymbolsReleased(Call, State, ArgRegion, C);
}
}
+ return;
}
void InnerPointerChecker::checkDeadSymbols(SymbolReaper &SymReaper,
@@ -216,7 +257,6 @@
const ExplodedNode *PrevN,
BugReporterContext &BRC,
BugReport &BR) {
-
if (!isSymbolTracked(N->getState(), PtrToBuf) ||
isSymbolTracked(PrevN->getState(), PtrToBuf))
return nullptr;
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
