Thanks very much for the example! I committed it together with the design correction in r338918.
Alexander Kornienko <ale...@google.com> ezt írta (időpont: 2018. aug. 3., P, 5:36): > Thanks for the prompt fix, it solved the problem. > > In case you want to add a regression test, this is what I came up with: > $ cat test-isCalledOnStringObject.cc > char *c(); > class B {}; > void d() { > B g, *h; > *(void **)&h = c() + 1; > *h = g; > } > $ ./clang-tidy -checks=-*,clang-analyzer* test-isCalledOnStringObject.cc > -- -std=c++11 > @ 0x5640f3e56b85 32 clang::DeclarationName::isIdentifier() > @ 0x5640f3ebaca3 80 clang::NamedDecl::getName() > @ 0x5640f9f7e56b 336 (anonymous > namespace)::InnerPointerChecker::isCalledOnStringObject() > @ 0x5640f9f7e0ef 288 (anonymous > namespace)::InnerPointerChecker::checkPostCall() > @ 0x5640f9f7e080 48 > clang::ento::check::PostCall::_checkCall<>() > @ 0x5640fa2d5c12 64 clang::ento::CheckerFn<>::operator()() > @ 0x5640fa2c82d8 208 (anonymous > namespace)::CheckCallContext::runChecker() > @ 0x5640fa2c4d6c 384 expandGraphWithCheckers<>() > @ 0x5640fa2c4acb 208 > clang::ento::CheckerManager::runCheckersForCallEvent() > @ 0x5640fa32fdc8 80 > clang::ento::CheckerManager::runCheckersForPostCall() > @ 0x5640fa332ff3 336 clang::ento::ExprEngine::evalCall() > @ 0x5640fa332e89 400 > clang::ento::ExprEngine::VisitCallExpr() > @ 0x5640fa2ef8cb 4304 clang::ento::ExprEngine::Visit() > @ 0x5640fa2ec38c 464 clang::ento::ExprEngine::ProcessStmt() > @ 0x5640fa2ec079 208 > clang::ento::ExprEngine::processCFGElement() > @ 0x5640fa31d8ff 128 > clang::ento::CoreEngine::HandlePostStmt() > @ 0x5640fa31d208 496 > clang::ento::CoreEngine::dispatchWorkItem() > @ 0x5640fa31cdbb 544 > clang::ento::CoreEngine::ExecuteWorkList() > @ 0x5640f9c466b4 80 > clang::ento::ExprEngine::ExecuteWorkList() > > > On Fri, Aug 3, 2018 at 12:28 AM Réka Nikolett Kovács < > rekanikol...@gmail.com> wrote: > >> Thanks for the notice. Have you managed to get the repro? I haven't >> succeeded in constructing one yet, but I've committed a check for the >> CXXRecordDecl. I hope that fixes the crashes. >> >> Alexander Kornienko <ale...@google.com> ezt írta (időpont: 2018. aug. >> 2., Cs, 11:07): >> >>> >>> >>> On Mon, Jul 30, 2018 at 5:44 PM Reka Kovacs via cfe-commits < >>> cfe-commits@lists.llvm.org> wrote: >>> >>>> Author: rkovacs >>>> Date: Mon Jul 30 08:43:45 2018 >>>> New Revision: 338259 >>>> >>>> URL: http://llvm.org/viewvc/llvm-project?rev=338259&view=rev >>>> Log: >>>> [analyzer] Add support for more invalidating functions in >>>> InnerPointerChecker. >>>> >>>> According to the standard, pointers referring to the elements of a >>>> `basic_string` may 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. >>>> >>>> Differential Revision: https://reviews.llvm.org/D49656 >>>> >>>> Modified: >>>> cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp >>>> cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp >>>> cfe/trunk/test/Analysis/inner-pointer.cpp >>>> >>>> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp >>>> URL: >>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp?rev=338259&r1=338258&r2=338259&view=diff >>>> >>>> ============================================================================== >>>> --- cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp >>>> (original) >>>> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp Mon >>>> Jul 30 08:43:45 2018 >>>> @@ -91,37 +91,53 @@ public: >>>> 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") >>>> >>> >>> We observe crashes on this line. I'm working on an isolated repro, but >>> you could probably try to construct one yourself (I >>> suppose, getAsCXXRecordDecl() can return null here). >>> >>> >>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits