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:37:20 PDT 2019


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 <alexfh at 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 <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/147d048b/attachment-0001.html>


More information about the cfe-commits mailing list