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

[Alexander Kornienko via cfe-commits]

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/56732cac/attachment.html>