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
