r338259 - [analyzer] Add support for more invalidating functions in InnerPointerChecker.
Réka Nikolett Kovács via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 3 13:47:27 PDT 2018
Thanks very much for the example!
I committed it together with the design correction in r338918.
Alexander Kornienko <alexfh at 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 <
> rekanikolett at 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 <alexfh at 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 at 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).
>>>
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180803/87259d77/attachment-0001.html>
More information about the cfe-commits
mailing list