r356222 - [analyzer] Support C++17 aggregates with bases without constructors.
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 19 09:34:13 PDT 2019
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 <alexfh at google.com>
wrote:
> On Fri, Mar 15, 2019 at 1:21 AM Artem Dergachev via cfe-commits <
> cfe-commits at 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 at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190319/4948c7ed/attachment-0001.html>
More information about the cfe-commits
mailing list