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