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

[Ilya Biryukov via cfe-commits]

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 <ibiryukov at 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 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
>


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