<div dir="ltr"><div dir="ltr">Filed this as <a href="https://bugs.llvm.org/show_bug.cgi?id=41142">https://bugs.llvm.org/show_bug.cgi?id=41142</a>. Any hope for a prompt fix here?</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Mar 19, 2019 at 5:34 PM Alexander Kornienko <<a href="mailto:alexfh@google.com">alexfh@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div>A reduced test case:</div><div>$ cat test-RegionStoreManager__bindStruct.cc</div><div>struct a {};</div><div>class b : a {};</div><div>b c() { b d{c()}; }</div><div>$ ./clang-tidy -checks="-*,clang-analyzer*" test-RegionStoreManager__bindStruct.cc -- -std=c++17</div><div>assert.h assertion failed at tools/clang/lib/StaticAnalyzer/Core/RegionStore.cpp:2362 in (anonymous namespace)::RegionBindingsRef (anonym</div><div>ous namespace)::RegionStoreManager::bindStruct(RegionBindingsConstRef, const clang::ento::TypedValueRegion *, clang::ento::SVal): CRD->isAggregate() && "Non-aggregates are constructed with a constructor!"</div><div> @ 0x559908170326 __assert_fail</div><div> @ 0x5599068d4854 (anonymous namespace)::RegionStoreManager::bindStruct()</div><div> @ 0x5599068c93c8 (anonymous namespace)::RegionStoreManager::Bind()</div><div> @ 0x5599068b409f clang::ento::ProgramState::bindLoc()</div><div> @ 0x559906865935 clang::ento::ExprEngine::processPointerEscapedOnBind()</div><div> @ 0x55990685d4b3 clang::ento::ExprEngine::evalBind()</div><div> @ 0x559906872a43 clang::ento::ExprEngine::VisitDeclStmt()</div><div> @ 0x55990685c16f clang::ento::ExprEngine::Visit()</div><div> @ 0x559906858b1f clang::ento::ExprEngine::ProcessStmt()</div><div> @ 0x559906858808 clang::ento::ExprEngine::processCFGElement()</div><div> @ 0x55990684cb65 clang::ento::CoreEngine::HandlePostStmt()</div><div> @ 0x55990684bf5c clang::ento::CoreEngine::ExecuteWorkList()</div><div> @ 0x5599065b635b (anonymous namespace)::AnalysisConsumer::HandleCode()</div><div> @ 0x5599065a0135 (anonymous namespace)::AnalysisConsumer::HandleTranslationUnit()</div><div> @ 0x559906bb7cbc clang::MultiplexConsumer::HandleTranslationUnit()</div><div> @ 0x559906d226d4 clang::ParseAST()</div><div> @ 0x559906b98a83 clang::FrontendAction::Execute()</div><div> @ 0x559906b31cd1 clang::CompilerInstance::ExecuteAction()</div><div> @ 0x559906a9cf61 clang::tooling::FrontendActionFactory::runInvocation()</div><div> @ 0x55990620cc07 clang::tidy::runClangTidy()::ActionFactory::runInvocation()</div><div> @ 0x559906a9ccca clang::tooling::ToolInvocation::runInvocation()</div><div> @ 0x559906a9c646 clang::tooling::ToolInvocation::run()</div><div> @ 0x559906a9ef22 clang::tooling::ClangTool::run()</div><div> @ 0x559906207ecf clang::tidy::runClangTidy()</div><div> @ 0x559902d47c45 main</div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Mar 19, 2019 at 1:14 AM Alexander Kornienko <<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr">On Fri, Mar 15, 2019 at 1:21 AM Artem Dergachev via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: dergachev<br>
Date: Thu Mar 14 17:22:59 2019<br>
New Revision: 356222<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=356222&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=356222&view=rev</a><br>
Log:<br>
[analyzer] Support C++17 aggregates with bases without constructors.<br>
<br>
RegionStore now knows how to bind a nonloc::CompoundVal that represents the<br>
value of an aggregate initializer when it has its initial segment of sub-values<br>
correspond to base classes.<br>
<br>
Additionally, fixes the crash from pr40022.<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D59054" rel="noreferrer" target="_blank">https://reviews.llvm.org/D59054</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=356222&r1=356221&r2=356222&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=356222&r1=356221&r2=356222&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)<br>
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Thu Mar 14 17:22:59 2019<br>
@@ -2334,12 +2334,57 @@ RegionBindingsRef RegionStoreManager::bi<br>
if (V.isUnknown() || !V.getAs<nonloc::CompoundVal>())<br>
return bindAggregate(B, R, UnknownVal());<br>
<br>
+ // The raw CompoundVal is essentially a symbolic InitListExpr: an (immutable)<br>
+ // list of other values. It appears pretty much only when there's an actual<br>
+ // initializer list expression in the program, and the analyzer tries to<br>
+ // unwrap it as soon as possible.<br>
+ // This code is where such unwrap happens: when the compound value is put into<br>
+ // the object that it was supposed to initialize (it's an *initializer* list,<br>
+ // after all), instead of binding the whole value to the whole object, we bind<br>
+ // sub-values to sub-objects. Sub-values may themselves be compound values,<br>
+ // and in this case the procedure becomes recursive.<br>
+ // FIXME: The annoying part about compound values is that they don't carry<br>
+ // any sort of information about which value corresponds to which sub-object.<br>
+ // It's simply a list of values in the middle of nowhere; we expect to match<br>
+ // them to sub-objects, essentially, "by index": first value binds to<br>
+ // the first field, second value binds to the second field, etc.<br>
+ // It would have been much safer to organize non-lazy compound values as<br>
+ // a mapping from fields/bases to values.<br>
const nonloc::CompoundVal& CV = V.castAs<nonloc::CompoundVal>();<br>
nonloc::CompoundVal::iterator VI = CV.begin(), VE = CV.end();<br>
<br>
- RecordDecl::field_iterator FI, FE;<br>
RegionBindingsRef NewB(B);<br>
<br>
+ // In C++17 aggregates may have base classes, handle those as well.<br>
+ // They appear before fields in the initializer list / compound value.<br>
+ if (const auto *CRD = dyn_cast<CXXRecordDecl>(RD)) {<br>
+ assert(CRD->isAggregate() &&<br>
+ "Non-aggregates are constructed with a constructor!");</blockquote><div><br></div><div>Now we see this assertion being triggered on a substantial number of files in our codebase:</div><div>llvm/tools/clang/lib/StaticAnalyzer/Core/RegionStore.cpp:2362 in (anonymous namespace)::RegionBindingsRef (anonymous namespace)::RegionStoreManager::bindStruct(RegionBindingsConstRef, const clang::ento::TypedValueRegion *, clang::ento::SVal): CRD->isAggregate() && "Non-aggregates are constructed with a constructor!"<br></div><div>Stack trace:</div><div><div> @ 0x5596b00a84e6 96 __assert_fail</div><div> @ 0x5596b6e6ea14 304 (anonymous namespace)::RegionStoreManager::bindStruct()</div><div> @ 0x5596afb30228 128 (anonymous namespace)::RegionStoreManager::Bind()</div><div> @ 0x5596af822abf 128 clang::ento::ProgramState::bindLoc()</div><div> @ 0x5596b6e2b657 112 clang::ento::ExprEngine::processPointerEscapedOnBind()</div><div> @ 0x5596b907f65f 512 clang::ento::ExprEngine::evalBind()</div><div> @ 0x5596b6e30ea7 560 clang::ento::ExprEngine::VisitDeclStmt()</div><div> @ 0x5596b8da1fe2 752 clang::ento::ExprEngine::Visit()</div><div> @ 0x5596b8d9cb2f 400 clang::ento::ExprEngine::ProcessStmt()</div><div> @ 0x5596af78431e 112 clang::ento::ExprEngine::processCFGElement()</div><div> @ 0x5596af6578e0 48 clang::ento::CoreEngine::HandlePostStmt()</div><div> @ 0x5596b03b151b 272 clang::ento::CoreEngine::ExecuteWorkList()</div><div> @ 0x5596af6f8efe 1248 (anonymous namespace)::AnalysisConsumer::HandleCode()</div><div> @ 0x5596b6c54a77 448 (anonymous namespace)::AnalysisConsumer::HandleTranslationUnit()</div><div> @ 0x5596b706f08c 48 clang::MultiplexConsumer::HandleTranslationUnit()</div><div> @ 0x5596aff72e24 144 clang::ParseAST()</div><div> @ 0x5596b7053bc3 48 clang::FrontendAction::Execute()</div><div> @ 0x5596b7002ba0 160 clang::CompilerInstance::ExecuteAction()</div><div> @ 0x5596b6f91a61 464 clang::tooling::FrontendActionFactory::runInvocation()</div><div> @ 0x5596b6f917ea 80 clang::tooling::ToolInvocation::runInvocation()</div><div> @ 0x5596afe70af6 2352 clang::tooling::ToolInvocation::run()</div></div><div><br></div><div>Trying to get an isolated test case.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+ for (const auto &B : CRD->bases()) {<br>
+ // (Multiple inheritance is fine though.)<br>
+ assert(!B.isVirtual() && "Aggregates cannot have virtual base classes!");<br>
+<br>
+ if (VI == VE)<br>
+ break;<br>
+<br>
+ QualType BTy = B.getType();<br>
+ assert(BTy->isStructureOrClassType() && "Base classes must be classes!");<br>
+<br>
+ const CXXRecordDecl *BRD = BTy->getAsCXXRecordDecl();<br>
+ assert(BRD && "Base classes must be C++ classes!");<br>
+<br>
+ const CXXBaseObjectRegion *BR =<br>
+ MRMgr.getCXXBaseObjectRegion(BRD, R, /*IsVirtual=*/false);<br>
+<br>
+ NewB = bindStruct(NewB, BR, *VI);<br>
+<br>
+ ++VI;<br>
+ }<br>
+ }<br>
+<br>
+ RecordDecl::field_iterator FI, FE;<br>
+<br>
for (FI = RD->field_begin(), FE = RD->field_end(); FI != FE; ++FI) {<br>
<br>
if (VI == VE)<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=356222&r1=356221&r2=356222&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/array-struct-region.cpp?rev=356222&r1=356221&r2=356222&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Analysis/array-struct-region.cpp (original)<br>
+++ cfe/trunk/test/Analysis/array-struct-region.cpp Thu Mar 14 17:22:59 2019<br>
@@ -1,7 +1,21 @@<br>
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c %s<br>
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c++ -analyzer-config c++-inlining=constructors %s<br>
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -DINLINE -verify -x c %s<br>
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -DINLINE -verify -x c++ -analyzer-config c++-inlining=constructors %s<br>
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\<br>
+// RUN: -analyzer-checker=debug.ExprInspection -verify\<br>
+// RUN: -x c %s<br>
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\<br>
+// RUN: -analyzer-checker=debug.ExprInspection -verify\<br>
+// RUN: -x c++ -std=c++14 %s<br>
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\<br>
+// RUN: -analyzer-checker=debug.ExprInspection -verify\<br>
+// RUN: -x c++ -std=c++17 %s<br>
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\<br>
+// RUN: -analyzer-checker=debug.ExprInspection -verify\<br>
+// RUN: -DINLINE -x c %s<br>
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\<br>
+// RUN: -analyzer-checker=debug.ExprInspection -verify\<br>
+// RUN: -DINLINE -x c++ -std=c++14 %s<br>
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\<br>
+// RUN: -analyzer-checker=debug.ExprInspection -verify\<br>
+// RUN: -DINLINE -x c++ -std=c++17 %s<br>
<br>
void clang_analyzer_eval(int);<br>
<br>
@@ -196,4 +210,49 @@ namespace EmptyClass {<br>
}<br>
}<br>
<br>
+#if __cplusplus >= 201703L<br>
+namespace aggregate_inheritance_cxx17 {<br>
+struct A {<br>
+ int x;<br>
+};<br>
+<br>
+struct B {<br>
+ int y;<br>
+};<br>
+<br>
+struct C: B {<br>
+ int z;<br>
+};<br>
+<br>
+struct D: A, C {<br>
+ int w;<br>
+};<br>
+<br>
+void foo() {<br>
+ D d{1, 2, 3, 4};<br>
+ clang_analyzer_eval(d.x == 1); // expected-warning{{TRUE}}<br>
+ clang_analyzer_eval(d.y == 2); // expected-warning{{TRUE}}<br>
+ clang_analyzer_eval(d.z == 3); // expected-warning{{TRUE}}<br>
+ clang_analyzer_eval(d.w == 4); // expected-warning{{TRUE}}<br>
+}<br>
+} // namespace aggregate_inheritance_cxx17<br>
+#endif<br>
+<br>
+namespace flex_array_inheritance_cxx17 {<br>
+struct A {<br>
+ int flexible_array[];<br>
+};<br>
+<br>
+struct B {<br>
+ long cookie;<br>
+};<br>
+<br>
+struct C : B {<br>
+ A a;<br>
+};<br>
+<br>
+void foo() {<br>
+ C c{}; // no-crash<br>
+}<br>
+} // namespace flex_array_inheritance_cxx17<br>
#endif<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="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div></div></div>
</blockquote></div>
</blockquote></div>