Author: Valeriy Savchenko Date: 2021-04-08T20:30:12+03:00 New Revision: 663ac91ed1d6156e848e5f5f00cd7e7dd6cf867f
URL: https://github.com/llvm/llvm-project/commit/663ac91ed1d6156e848e5f5f00cd7e7dd6cf867f DIFF: https://github.com/llvm/llvm-project/commit/663ac91ed1d6156e848e5f5f00cd7e7dd6cf867f.diff LOG: [analyzer] Fix false positives in inner pointer checker (PR49628) This patch supports std::data and std::addressof functions. rdar://73463300 Differential Revision: https://reviews.llvm.org/D99260 Added: Modified: clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp clang/test/Analysis/inner-pointer.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp index 65e52e139ee4..bcae73378028 100644 --- a/clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp @@ -34,9 +34,9 @@ namespace { class InnerPointerChecker : public Checker<check::DeadSymbols, check::PostCall> { - CallDescription AppendFn, AssignFn, ClearFn, CStrFn, DataFn, EraseFn, - InsertFn, PopBackFn, PushBackFn, ReplaceFn, ReserveFn, ResizeFn, - ShrinkToFitFn, SwapFn; + CallDescription AppendFn, AssignFn, AddressofFn, ClearFn, CStrFn, DataFn, + DataMemberFn, EraseFn, InsertFn, PopBackFn, PushBackFn, ReplaceFn, + ReserveFn, ResizeFn, ShrinkToFitFn, SwapFn; public: class InnerPointerBRVisitor : public BugReporterVisitor { @@ -73,9 +73,10 @@ class InnerPointerChecker InnerPointerChecker() : AppendFn({"std", "basic_string", "append"}), AssignFn({"std", "basic_string", "assign"}), + AddressofFn({"std", "addressof"}), ClearFn({"std", "basic_string", "clear"}), - CStrFn({"std", "basic_string", "c_str"}), - DataFn({"std", "basic_string", "data"}), + CStrFn({"std", "basic_string", "c_str"}), DataFn({"std", "data"}, 1), + DataMemberFn({"std", "basic_string", "data"}), EraseFn({"std", "basic_string", "erase"}), InsertFn({"std", "basic_string", "insert"}), PopBackFn({"std", "basic_string", "pop_back"}), @@ -90,6 +91,9 @@ class InnerPointerChecker /// pointers referring to the container object's inner buffer. bool isInvalidatingMemberFunction(const CallEvent &Call) const; + /// Check whether the called function returns a raw inner pointer. + bool isInnerPointerAccessFunction(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, @@ -130,6 +134,12 @@ bool InnerPointerChecker::isInvalidatingMemberFunction( Call.isCalled(SwapFn)); } +bool InnerPointerChecker::isInnerPointerAccessFunction( + const CallEvent &Call) const { + return (Call.isCalled(CStrFn) || Call.isCalled(DataFn) || + Call.isCalled(DataMemberFn)); +} + void InnerPointerChecker::markPtrSymbolsReleased(const CallEvent &Call, ProgramStateRef State, const MemRegion *MR, @@ -172,6 +182,11 @@ void InnerPointerChecker::checkFunctionArguments(const CallEvent &Call, if (!ArgRegion) continue; + // std::addressof function accepts a non-const reference as an argument, + // but doesn't modify it. + if (Call.isCalled(AddressofFn)) + continue; + markPtrSymbolsReleased(Call, State, ArgRegion, C); } } @@ -195,36 +210,49 @@ void InnerPointerChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const { ProgramStateRef State = C.getState(); + // TODO: Do we need these to be typed? + const TypedValueRegion *ObjRegion = nullptr; + if (const auto *ICall = dyn_cast<CXXInstanceCall>(&Call)) { - // TODO: Do we need these to be typed? - const auto *ObjRegion = dyn_cast_or_null<TypedValueRegion>( + ObjRegion = dyn_cast_or_null<TypedValueRegion>( ICall->getCXXThisVal().getAsRegion()); - if (!ObjRegion) - 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. + // Check [string.require] / second point. + if (isInvalidatingMemberFunction(Call)) { + markPtrSymbolsReleased(Call, State, ObjRegion, C); + return; + } + } - 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); + if (isInnerPointerAccessFunction(Call)) { - State = State->set<RawPtrMap>(ObjRegion, Set); - C.addTransition(State); - } - return; + if (isa<SimpleFunctionCall>(Call)) { + // NOTE: As of now, we only have one free access function: std::data. + // If we add more functions like this in the list, hardcoded + // argument index should be changed. + ObjRegion = + dyn_cast_or_null<TypedValueRegion>(Call.getArgSVal(0).getAsRegion()); } - // Check [string.require] / second point. - if (isInvalidatingMemberFunction(Call)) { - markPtrSymbolsReleased(Call, State, ObjRegion, C); + if (!ObjRegion) return; + + 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] / first point. diff --git a/clang/test/Analysis/inner-pointer.cpp b/clang/test/Analysis/inner-pointer.cpp index d8b011a7aa64..920a1fe8dcc9 100644 --- a/clang/test/Analysis/inner-pointer.cpp +++ b/clang/test/Analysis/inner-pointer.cpp @@ -17,6 +17,11 @@ void func_value(T a); string my_string = "default"; void default_arg(int a = 42, string &b = my_string); +template <class T> +T *addressof(T &arg); + +char *data(std::string &c); + } // end namespace std void consume(const char *) {} @@ -273,6 +278,15 @@ void deref_after_swap() { // expected-note@-1 {{Inner pointer of container used after re/deallocation}} } +void deref_after_std_data() { + const char *c; + std::string s; + c = std::data(s); // expected-note {{Pointer to inner buffer of 'std::string' obtained here}} + s.push_back('c'); // expected-note {{Inner buffer of 'std::string' reallocated by call to 'push_back'}} + consume(c); // expected-warning {{Inner pointer of container used after re/deallocation}} + // expected-note@-1 {{Inner pointer of container used after re/deallocation}} +} + struct S { std::string s; const char *name() { @@ -361,8 +375,24 @@ void func_default_arg() { // expected-note@-1 {{Inner pointer of container used after re/deallocation}} } +void func_addressof() { + const char *c; + std::string s; + c = s.c_str(); + addressof(s); + consume(c); // no-warning +} + +void func_std_data() { + const char *c; + std::string s; + c = std::data(s); + consume(c); // no-warning +} + struct T { std::string to_string() { return s; } + private: std::string s; }; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
