On Tue, Mar 19, 2019 at 6:24 PM Artem Dergachev <noqnoq...@gmail.com> wrote:
> Hi, > > I'll try to fix this ASAP! > Thanks! > It sounds as if i don't have nearly enough C++17 codebases for testing, > are there any obvious open-source projects that you could recommend? No, but this particular example comes from C++11 code that just has subtly different semantics in C++17. I guess, just adding -std=c++17 on existing code (LLVM, for example ;) could help uncover some of the issues. I believe, we can also continue providing reports in case CSA fails assertions on our code. > I'd > love to try to reincarnate > > http://green.lab.llvm.org/green/view/Experimental/job/StaticAnalyzerBenchmarks/ > with those. > > On 3/19/19 9:37 AM, Alexander Kornienko wrote: > > Filed this as https://bugs.llvm.org/show_bug.cgi?id=41142. Any hope > > for a prompt fix here? > > > > On Tue, Mar 19, 2019 at 5:34 PM Alexander Kornienko <ale...@google.com > > <mailto:ale...@google.com>> wrote: > > > > 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 <mailto:ale...@google.com>> wrote: > > > > On Fri, Mar 15, 2019 at 1:21 AM Artem Dergachev via > > cfe-commits <cfe-commits@lists.llvm.org > > <mailto: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 <mailto: > 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
