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