<div dir="ltr">On Thu, Aug 29, 2013 at 9:06 AM, Pavel Labath <span dir="ltr"><<a href="mailto:labath@google.com" target="_blank">labath@google.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: labath<br>
Date: Thu Aug 29 11:06:04 2013<br>
New Revision: 189590<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=189590&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=189590&view=rev</a><br>
Log:<br>
[analyzer] Fix handling of "empty" structs with base classes<br>
<br>
Summary:<br>
RegionStoreManager had an optimization which replaces references to empty<br>
structs with UnknownVal. Unfortunately, this check didn't take into account<br>
possible field members in base classes.<br>
<br>
To address this, I changed this test to "is empty and has no base classes". I<br>
don't consider it worth the trouble to go through base classes and check if all<br>
of them are empty.<br></blockquote><div><br></div><div>You could use CXXRecordDecl::isEmpty() (which also checks for virtual functions/virtual bases, and handles the case where the only fields are zero-length bitfields).</div>
<div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Reviewers: jordan_rose<br>
<br>
CC: cfe-commits<br>
<br>
Differential Revision: <a href="http://llvm-reviews.chandlerc.com/D1547" target="_blank">http://llvm-reviews.chandlerc.com/D1547</a><br>
<br>
Modified:<br>
    cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp<br>
    cfe/trunk/test/Analysis/array-struct-region.cpp<br>
<br>
Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=189590&r1=189589&r2=189590&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=189590&r1=189589&r2=189590&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)<br>
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Thu Aug 29 11:06:04 2013<br>
@@ -1832,10 +1832,18 @@ NonLoc RegionStoreManager::createLazyBin<br>
   return svalBuilder.makeLazyCompoundVal(StoreRef(B.asStore(), *this), R);<br>
 }<br>
<br>
+static bool isRecordEmpty(const RecordDecl *RD) {<br>
+  if (!RD->field_empty())<br>
+    return false;<br>
+  if (const CXXRecordDecl *CRD = dyn_cast<CXXRecordDecl>(RD))<br>
+    return CRD->getNumBases() == 0;<br>
+  return true;<br>
+}<br>
+<br>
 SVal RegionStoreManager::getBindingForStruct(RegionBindingsConstRef B,<br>
                                              const TypedValueRegion *R) {<br>
   const RecordDecl *RD = R->getValueType()->castAs<RecordType>()->getDecl();<br>
-  if (RD->field_empty())<br>
+  if (isRecordEmpty(RD))<br>
     return UnknownVal();<br>
<br>
   return createLazyBinding(B, R);<br>
<br>
Modified: cfe/trunk/test/Analysis/array-struct-region.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/array-struct-region.cpp?rev=189590&r1=189589&r2=189590&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/array-struct-region.cpp?rev=189590&r1=189589&r2=189590&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/test/Analysis/array-struct-region.cpp (original)<br>
+++ cfe/trunk/test/Analysis/array-struct-region.cpp Thu Aug 29 11:06:04 2013<br>
@@ -173,4 +173,27 @@ void testImmediateUseOp() {<br>
   clang_analyzer_eval(getConstrainedFieldRefOp(getS()) == 42); // expected-warning{{TRUE}}<br>
 }<br>
<br>
+namespace EmptyClass {<br>
+  struct Base {<br>
+    int& x;<br>
+<br>
+    Base(int& x) : x(x) {}<br>
+  };<br>
+<br>
+  struct Derived : public Base {<br>
+    Derived(int& x) : Base(x) {}<br>
+<br>
+    void operator=(int a) { x = a; }<br>
+  };<br>
+<br>
+  Derived ref(int& a) { return Derived(a); }<br>
+<br>
+  // There used to be a warning here, because analyzer treated Derived as empty.<br>
+  int test() {<br>
+    int a;<br>
+    ref(a) = 42;<br>
+    return a; // no warning<br>
+  }<br>
+}<br>
+<br>
 #endif<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div>