r338259 - [analyzer] Add support for more invalidating functions in InnerPointerChecker.

[Réka Nikolett Kovács via cfe-commits]

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/20180802/eddbca8d/attachment-0001.html>