Re: r338259 - [analyzer] Add support for more invalidating functions in InnerPointerChecker.
Thanks very much for the example!
I committed it together with the design correction in r338918.
Alexander Kornienko 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 **) = 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()
> @ 0x5640f9f7e56b336 (anonymous
> namespace)::InnerPointerChecker::isCalledOnStringObject()
> @ 0x5640f9f7e0ef288 (anonymous
> namespace)::InnerPointerChecker::checkPostCall()
> @ 0x5640f9f7e080 48
> clang::ento::check::PostCall::_checkCall<>()
> @ 0x5640fa2d5c12 64 clang::ento::CheckerFn<>::operator()()
> @ 0x5640fa2c82d8208 (anonymous
> namespace)::CheckCallContext::runChecker()
> @ 0x5640fa2c4d6c384 expandGraphWithCheckers<>()
> @ 0x5640fa2c4acb208
> clang::ento::CheckerManager::runCheckersForCallEvent()
> @ 0x5640fa32fdc8 80
> clang::ento::CheckerManager::runCheckersForPostCall()
> @ 0x5640fa332ff3336 clang::ento::ExprEngine::evalCall()
> @ 0x5640fa332e89400
> clang::ento::ExprEngine::VisitCallExpr()
> @ 0x5640fa2ef8cb 4304 clang::ento::ExprEngine::Visit()
> @ 0x5640fa2ec38c464 clang::ento::ExprEngine::ProcessStmt()
> @ 0x5640fa2ec079208
> clang::ento::ExprEngine::processCFGElement()
> @ 0x5640fa31d8ff128
> clang::ento::CoreEngine::HandlePostStmt()
> @ 0x5640fa31d208496
> clang::ento::CoreEngine::dispatchWorkItem()
> @ 0x5640fa31cdbb544
> 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 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=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=338258=338259=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 ) 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 ) const;
+
+ /// Mark
Re: r338259 - [analyzer] Add support for more invalidating functions in InnerPointerChecker.
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 **) = 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()
@ 0x5640f9f7e56b336 (anonymous
namespace)::InnerPointerChecker::isCalledOnStringObject()
@ 0x5640f9f7e0ef288 (anonymous
namespace)::InnerPointerChecker::checkPostCall()
@ 0x5640f9f7e080 48
clang::ento::check::PostCall::_checkCall<>()
@ 0x5640fa2d5c12 64 clang::ento::CheckerFn<>::operator()()
@ 0x5640fa2c82d8208 (anonymous
namespace)::CheckCallContext::runChecker()
@ 0x5640fa2c4d6c384 expandGraphWithCheckers<>()
@ 0x5640fa2c4acb208
clang::ento::CheckerManager::runCheckersForCallEvent()
@ 0x5640fa32fdc8 80
clang::ento::CheckerManager::runCheckersForPostCall()
@ 0x5640fa332ff3336 clang::ento::ExprEngine::evalCall()
@ 0x5640fa332e89400
clang::ento::ExprEngine::VisitCallExpr()
@ 0x5640fa2ef8cb 4304 clang::ento::ExprEngine::Visit()
@ 0x5640fa2ec38c464 clang::ento::ExprEngine::ProcessStmt()
@ 0x5640fa2ec079208
clang::ento::ExprEngine::processCFGElement()
@ 0x5640fa31d8ff128
clang::ento::CoreEngine::HandlePostStmt()
@ 0x5640fa31d208496
clang::ento::CoreEngine::dispatchWorkItem()
@ 0x5640fa31cdbb544
clang::ento::CoreEngine::ExecuteWorkList()
@ 0x5640f9c466b4 80
clang::ento::ExprEngine::ExecuteWorkList()
On Fri, Aug 3, 2018 at 12:28 AM Réka Nikolett Kovács
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 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=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=338258=338259=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 ) 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 ) const;
>>> +
>>> + /// Mark pointer symbols associated with the given memory region
>>> released
>>> + /// in the program state.
>>> + void markPtrSymbolsReleased(const CallEvent , ProgramStateRef
>>> State,
>>> + const MemRegion *ObjRegion,
>>> + CheckerContext ) const;
>>> +
>>> + /// Standard library functions that
Re: r338259 - [analyzer] Add support for more invalidating functions in InnerPointerChecker.
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 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=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=338258=338259=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 ) 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 ) const;
>> +
>> + /// Mark pointer symbols associated with the given memory region
>> released
>> + /// in the program state.
>> + void markPtrSymbolsReleased(const CallEvent , ProgramStateRef
>> State,
>> + const MemRegion *ObjRegion,
>> + CheckerContext ) 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 , ProgramStateRef
>> State,
>> + CheckerContext ) 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 , CheckerContext ) const;
>>
>> - /// Clean up the ProgramState map.
>> + /// Clean up the program state map.
>>void checkDeadSymbols(SymbolReaper , CheckerContext )
>> 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 )
>> const {
>> +bool InnerPointerChecker::isCalledOnStringObject(
>> +const CXXInstanceCall *ICall) const {
>> + const auto *ObjRegion =
>> +
>> dyn_cast_or_null(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
>
Re: r338259 - [analyzer] Add support for more invalidating functions in InnerPointerChecker.
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=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=338258=338259=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 ) 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 ) const;
> +
> + /// Mark pointer symbols associated with the given memory region
> released
> + /// in the program state.
> + void markPtrSymbolsReleased(const CallEvent , ProgramStateRef
> State,
> + const MemRegion *ObjRegion,
> + CheckerContext ) 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 , ProgramStateRef
> State,
> + CheckerContext ) 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 , CheckerContext ) const;
>
> - /// Clean up the ProgramState map.
> + /// Clean up the program state map.
>void checkDeadSymbols(SymbolReaper , CheckerContext ) 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 )
> const {
> +bool InnerPointerChecker::isCalledOnStringObject(
> +const CXXInstanceCall *ICall) const {
> + const auto *ObjRegion =
> +
> dyn_cast_or_null(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
