r338259 - [analyzer] Add support for more invalidating functions in InnerPointerChecker.
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 3 05:36:29 PDT 2018
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/8b933399/attachment.html>
More information about the cfe-commits
mailing list