Ah, the reason why it does not fire on other inheritors is because they're final. Any reason to remove final from NeedsCastLocField?
On Fri, Sep 14, 2018 at 1:17 PM Ilya Biryukov <ibiryu...@google.com> wrote: > 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@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@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > > > -- > Regards, > Ilya Biryukov > -- Regards, Ilya Biryukov
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
