Hi,
I'll try to fix this ASAP!
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? 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
