r342221 - [analyzer][UninitializedObjectChecker] Support for nonloc::LocAsInteger

[Ilya Biryukov via cfe-commits]

This introduced revision introduced a new warning:
../tools/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp:62:7:
warning: '(anonymous namespace)::NeedsCastLocField' has virtual functions
but non-virtual destructor [-Wnon-virtual-dtor]

Which is turned into an error on our integrates, so it breaks us. Could you
please take a look?
A simple workaround would be to make FieldNode destructor virtual.
Does that make sense?


On Fri, Sep 14, 2018 at 12:19 PM Kristof Umann via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: szelethus
> Date: Fri Sep 14 03:18:26 2018
> New Revision: 342221
>
> URL: http://llvm.org/viewvc/llvm-project?rev=342221&view=rev
> Log:
> [analyzer][UninitializedObjectChecker] Support for nonloc::LocAsInteger
>
> Differential Revision: https://reviews.llvm.org/D49437
>
> Modified:
>
> cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
>
> cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
>     cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
>
> Modified:
> cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp?rev=342221&r1=342220&r2=342221&view=diff
>
> ==============================================================================
> ---
> cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
> (original)
> +++
> cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
> Fri Sep 14 03:18:26 2018
> @@ -274,15 +274,15 @@ bool FindUninitializedFields::isNonUnion
>        continue;
>      }
>
> -    if (isDereferencableType(T)) {
> +    SVal V = State->getSVal(FieldVal);
> +
> +    if (isDereferencableType(T) || V.getAs<nonloc::LocAsInteger>()) {
>        if (isDereferencableUninit(FR, LocalChain))
>          ContainsUninitField = true;
>        continue;
>      }
>
>      if (isPrimitiveType(T)) {
> -      SVal V = State->getSVal(FieldVal);
> -
>        if (isPrimitiveUninit(V)) {
>          if (addFieldToUninits(LocalChain.add(RegularField(FR))))
>            ContainsUninitField = true;
>
> Modified:
> cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp?rev=342221&r1=342220&r2=342221&view=diff
>
> ==============================================================================
> ---
> cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
> (original)
> +++
> cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
> Fri Sep 14 03:18:26 2018
> @@ -57,9 +57,9 @@ public:
>    }
>  };
>
> -/// Represents a void* field that needs to be casted back to its dynamic
> type
> -/// for a correct note message.
> -class NeedsCastLocField final : public FieldNode {
> +/// Represents a nonloc::LocAsInteger or void* field, that point to
> objects, but
> +/// needs to be casted back to its dynamic type for a correct note
> message.
> +class NeedsCastLocField : public FieldNode {
>    QualType CastBackType;
>
>  public:
> @@ -71,7 +71,13 @@ public:
>    }
>
>    virtual void printPrefix(llvm::raw_ostream &Out) const override {
> -    Out << "static_cast" << '<' << CastBackType.getAsString() << ">(";
> +    // If this object is a nonloc::LocAsInteger.
> +    if (getDecl()->getType()->isIntegerType())
> +      Out << "reinterpret_cast";
> +    // If this pointer's dynamic type is different then it's static type.
> +    else
> +      Out << "static_cast";
> +    Out << '<' << CastBackType.getAsString() << ">(";
>    }
>
>    virtual void printNode(llvm::raw_ostream &Out) const override {
> @@ -106,11 +112,12 @@ static llvm::Optional<DereferenceInfo> d
>  bool FindUninitializedFields::isDereferencableUninit(
>      const FieldRegion *FR, FieldChainInfo LocalChain) {
>
> -  assert(isDereferencableType(FR->getDecl()->getType()) &&
> -         "This method only checks dereferencable objects!");
> -
>    SVal V = State->getSVal(FR);
>
> +  assert((isDereferencableType(FR->getDecl()->getType()) ||
> +          V.getAs<nonloc::LocAsInteger>()) &&
> +         "This method only checks dereferencable objects!");
> +
>    if (V.isUnknown() || V.getAs<loc::ConcreteInt>()) {
>      IsAnyFieldInitialized = true;
>      return false;
> @@ -196,13 +203,15 @@ static llvm::Optional<DereferenceInfo> d
>
>    llvm::SmallSet<const TypedValueRegion *, 5> VisitedRegions;
>
> -  // If the static type of the field is a void pointer, we need to cast
> it back
> -  // to the dynamic type before dereferencing.
> -  bool NeedsCastBack = isVoidPointer(FR->getDecl()->getType());
> -
>    SVal V = State->getSVal(FR);
>    assert(V.getAsRegion() && "V must have an underlying region!");
>
> +  // If the static type of the field is a void pointer, or it is a
> +  // nonloc::LocAsInteger, we need to cast it back to the dynamic type
> before
> +  // dereferencing.
> +  bool NeedsCastBack = isVoidPointer(FR->getDecl()->getType()) ||
> +                       V.getAs<nonloc::LocAsInteger>();
> +
>    // The region we'd like to acquire.
>    const auto *R = V.getAsRegion()->getAs<TypedValueRegion>();
>    if (!R)
>
> Modified: cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp?rev=342221&r1=342220&r2=342221&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp (original)
> +++ cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp Fri Sep
> 14 03:18:26 2018
> @@ -22,6 +22,24 @@ void fConcreteIntLocTest() {
>  }
>
>
>  //===----------------------------------------------------------------------===//
> +// nonloc::LocAsInteger tests.
>
> +//===----------------------------------------------------------------------===//
> +
> +using intptr_t = long;
> +
> +struct LocAsIntegerTest {
> +  intptr_t ptr; // expected-note{{uninitialized pointee
> 'reinterpret_cast<char *>(this->ptr)'}}
> +  int dontGetFilteredByNonPedanticMode = 0;
> +
> +  LocAsIntegerTest(void *ptr) : ptr(reinterpret_cast<intptr_t>(ptr)) {}
> // expected-warning{{1 uninitialized field}}
> +};
> +
> +void fLocAsIntegerTest() {
> +  char c;
> +  LocAsIntegerTest t(&c);
> +}
> +
>
> +//===----------------------------------------------------------------------===//
>  // Null pointer tests.
>
>  //===----------------------------------------------------------------------===//
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>


-- 
Regards,
Ilya Biryukov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180914/0d290182/attachment.html>