<div dir="ltr">Ah, the reason why it does not fire on other inheritors is because they're final. Any reason to remove final from NeedsCastLocField?</div><br><div class="gmail_quote"><div dir="ltr">On Fri, Sep 14, 2018 at 1:17 PM Ilya Biryukov <<a href="mailto:ibiryukov@google.com">ibiryukov@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>This introduced revision introduced a new warning:<br></div><div><div><div dir="ltr">../tools/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp:62:7: warning: '(anonymous namespace)::NeedsCastLocField' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor] </div></div><div dir="ltr"><br></div><div>Which is turned into an error on our integrates, so it breaks us. Could you please take a look?</div><div>A simple workaround would be to make FieldNode destructor virtual. Does that make sense?<br></div><br class="m_4827426535669465364gmail-Apple-interchange-newline"></div></div><br><div class="gmail_quote"><div dir="ltr">On Fri, Sep 14, 2018 at 12:19 PM Kristof Umann via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: szelethus<br>
Date: Fri Sep 14 03:18:26 2018<br>
New Revision: 342221<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=342221&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=342221&view=rev</a><br>
Log:<br>
[analyzer][UninitializedObjectChecker] Support for nonloc::LocAsInteger<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D49437" rel="noreferrer" target="_blank">https://reviews.llvm.org/D49437</a><br>
<br>
Modified:<br>
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp<br>
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp<br>
cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp<br>
<br>
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp?rev=342221&r1=342220&r2=342221&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp?rev=342221&r1=342220&r2=342221&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp (original)<br>
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp Fri Sep 14 03:18:26 2018<br>
@@ -274,15 +274,15 @@ bool FindUninitializedFields::isNonUnion<br>
continue;<br>
}<br>
<br>
- if (isDereferencableType(T)) {<br>
+ SVal V = State->getSVal(FieldVal);<br>
+<br>
+ if (isDereferencableType(T) || V.getAs<nonloc::LocAsInteger>()) {<br>
if (isDereferencableUninit(FR, LocalChain))<br>
ContainsUninitField = true;<br>
continue;<br>
}<br>
<br>
if (isPrimitiveType(T)) {<br>
- SVal V = State->getSVal(FieldVal);<br>
-<br>
if (isPrimitiveUninit(V)) {<br>
if (addFieldToUninits(LocalChain.add(RegularField(FR))))<br>
ContainsUninitField = true;<br>
<br>
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp?rev=342221&r1=342220&r2=342221&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp?rev=342221&r1=342220&r2=342221&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp (original)<br>
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp Fri Sep 14 03:18:26 2018<br>
@@ -57,9 +57,9 @@ public:<br>
}<br>
};<br>
<br>
-/// Represents a void* field that needs to be casted back to its dynamic type<br>
-/// for a correct note message.<br>
-class NeedsCastLocField final : public FieldNode {<br>
+/// Represents a nonloc::LocAsInteger or void* field, that point to objects, but<br>
+/// needs to be casted back to its dynamic type for a correct note message.<br>
+class NeedsCastLocField : public FieldNode {<br>
QualType CastBackType;<br>
<br>
public:<br>
@@ -71,7 +71,13 @@ public:<br>
}<br>
<br>
virtual void printPrefix(llvm::raw_ostream &Out) const override {<br>
- Out << "static_cast" << '<' << CastBackType.getAsString() << ">(";<br>
+ // If this object is a nonloc::LocAsInteger.<br>
+ if (getDecl()->getType()->isIntegerType())<br>
+ Out << "reinterpret_cast";<br>
+ // If this pointer's dynamic type is different then it's static type.<br>
+ else<br>
+ Out << "static_cast";<br>
+ Out << '<' << CastBackType.getAsString() << ">(";<br>
}<br>
<br>
virtual void printNode(llvm::raw_ostream &Out) const override {<br>
@@ -106,11 +112,12 @@ static llvm::Optional<DereferenceInfo> d<br>
bool FindUninitializedFields::isDereferencableUninit(<br>
const FieldRegion *FR, FieldChainInfo LocalChain) {<br>
<br>
- assert(isDereferencableType(FR->getDecl()->getType()) &&<br>
- "This method only checks dereferencable objects!");<br>
-<br>
SVal V = State->getSVal(FR);<br>
<br>
+ assert((isDereferencableType(FR->getDecl()->getType()) ||<br>
+ V.getAs<nonloc::LocAsInteger>()) &&<br>
+ "This method only checks dereferencable objects!");<br>
+<br>
if (V.isUnknown() || V.getAs<loc::ConcreteInt>()) {<br>
IsAnyFieldInitialized = true;<br>
return false;<br>
@@ -196,13 +203,15 @@ static llvm::Optional<DereferenceInfo> d<br>
<br>
llvm::SmallSet<const TypedValueRegion *, 5> VisitedRegions;<br>
<br>
- // If the static type of the field is a void pointer, we need to cast it back<br>
- // to the dynamic type before dereferencing.<br>
- bool NeedsCastBack = isVoidPointer(FR->getDecl()->getType());<br>
-<br>
SVal V = State->getSVal(FR);<br>
assert(V.getAsRegion() && "V must have an underlying region!");<br>
<br>
+ // If the static type of the field is a void pointer, or it is a<br>
+ // nonloc::LocAsInteger, we need to cast it back to the dynamic type before<br>
+ // dereferencing.<br>
+ bool NeedsCastBack = isVoidPointer(FR->getDecl()->getType()) ||<br>
+ V.getAs<nonloc::LocAsInteger>();<br>
+<br>
// The region we'd like to acquire.<br>
const auto *R = V.getAsRegion()->getAs<TypedValueRegion>();<br>
if (!R)<br>
<br>
Modified: cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp?rev=342221&r1=342220&r2=342221&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp?rev=342221&r1=342220&r2=342221&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp (original)<br>
+++ cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp Fri Sep 14 03:18:26 2018<br>
@@ -22,6 +22,24 @@ void fConcreteIntLocTest() {<br>
}<br>
<br>
//===----------------------------------------------------------------------===//<br>
+// nonloc::LocAsInteger tests.<br>
+//===----------------------------------------------------------------------===//<br>
+<br>
+using intptr_t = long;<br>
+<br>
+struct LocAsIntegerTest {<br>
+ intptr_t ptr; // expected-note{{uninitialized pointee 'reinterpret_cast<char *>(this->ptr)'}}<br>
+ int dontGetFilteredByNonPedanticMode = 0;<br>
+<br>
+ LocAsIntegerTest(void *ptr) : ptr(reinterpret_cast<intptr_t>(ptr)) {} // expected-warning{{1 uninitialized field}}<br>
+};<br>
+<br>
+void fLocAsIntegerTest() {<br>
+ char c;<br>
+ LocAsIntegerTest t(&c);<br>
+}<br>
+<br>
+//===----------------------------------------------------------------------===//<br>
// Null pointer tests.<br>
//===----------------------------------------------------------------------===//<br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="m_4827426535669465364gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div>Regards,</div><div>Ilya Biryukov</div></div></div></div></div>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div>Regards,</div><div>Ilya Biryukov</div></div></div></div></div>