From 219a4bf52bae030e51ec6c07f933937845c07dc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <1.int32@gmail.com> Date: Mon, 28 Mar 2022 08:57:11 +0200 Subject: [PATCH] [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr. The "in-class initializer" expression should be set in the field of a default initialization expression before this expression node is created. The `CXXDefaultInitExpr` objects are created after the AST is loaded and at import not present in the "To" AST. And the in-class initializers of the used fields can be missing too, these must be set at import. This fixes a github issue #54061. Reviewed By: martong Differential Revision: https://reviews.llvm.org/D120824 --- clang/lib/AST/ASTImporter.cpp | 45 ++++-- .../Inputs/ctu-cxxdefaultinitexpr-import.cpp | 26 +++ ...xpr-import.cpp.externalDefMap.ast-dump.txt | 4 + .../test/Analysis/ctu-cxxdefaultinitexpr.cpp | 35 ++++ clang/unittests/AST/ASTImporterTest.cpp | 152 ++++++++++++++++++ 5 files changed, 249 insertions(+), 13 deletions(-) create mode 100644 clang/test/Analysis/Inputs/ctu-cxxdefaultinitexpr-import.cpp create mode 100644 clang/test/Analysis/Inputs/ctu-cxxdefaultinitexpr-import.cpp.externalDefMap.ast-dump.txt create mode 100644 clang/test/Analysis/ctu-cxxdefaultinitexpr.cpp diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 457465e87d9353..dc25c662bf6a6c 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -3637,19 +3637,23 @@ ExpectedDecl ASTNodeImporter::VisitFieldDecl(FieldDecl *D) { // initializer of a FieldDecl might not had been instantiated in the // "To" context. However, the "From" context might instantiated that, // thus we have to merge that. + // Note: `hasInClassInitializer()` is not the same as non-null + // `getInClassInitializer()` value. if (Expr *FromInitializer = D->getInClassInitializer()) { - // We don't have yet the initializer set. - if (FoundField->hasInClassInitializer() && - !FoundField->getInClassInitializer()) { - if (ExpectedExpr ToInitializerOrErr = import(FromInitializer)) + if (ExpectedExpr ToInitializerOrErr = import(FromInitializer)) { + // Import of the FromInitializer may result in the setting of + // InClassInitializer. If not, set it here. + assert(FoundField->hasInClassInitializer() && + "Field should have an in-class initializer if it has an " + "expression for it."); + if (!FoundField->getInClassInitializer()) FoundField->setInClassInitializer(*ToInitializerOrErr); - else { - // We can't return error here, - // since we already mapped D as imported. - // FIXME: warning message? - consumeError(ToInitializerOrErr.takeError()); - return FoundField; - } + } else { + // We can't return error here, + // since we already mapped D as imported. + // FIXME: warning message? + consumeError(ToInitializerOrErr.takeError()); + return FoundField; } } return FoundField; @@ -8118,8 +8122,23 @@ ExpectedStmt ASTNodeImporter::VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E) { if (!UsedContextOrErr) return UsedContextOrErr.takeError(); - return CXXDefaultInitExpr::Create( - Importer.getToContext(), *ToBeginLocOrErr, *ToFieldOrErr, *UsedContextOrErr); + FieldDecl *ToField = *ToFieldOrErr; + assert(ToField->hasInClassInitializer() && + "Field should have in-class initializer if there is a default init " + "expression that uses it."); + if (!ToField->getInClassInitializer()) { + // The in-class initializer may be not yet set in "To" AST even if the + // field is already there. This must be set here to make construction of + // CXXDefaultInitExpr work. + auto ToInClassInitializerOrErr = + import(E->getField()->getInClassInitializer()); + if (!ToInClassInitializerOrErr) + return ToInClassInitializerOrErr.takeError(); + ToField->setInClassInitializer(*ToInClassInitializerOrErr); + } + + return CXXDefaultInitExpr::Create(Importer.getToContext(), *ToBeginLocOrErr, + ToField, *UsedContextOrErr); } ExpectedStmt ASTNodeImporter::VisitCXXNamedCastExpr(CXXNamedCastExpr *E) { diff --git a/clang/test/Analysis/Inputs/ctu-cxxdefaultinitexpr-import.cpp b/clang/test/Analysis/Inputs/ctu-cxxdefaultinitexpr-import.cpp new file mode 100644 index 00000000000000..e0e356ff555cf2 --- /dev/null +++ b/clang/test/Analysis/Inputs/ctu-cxxdefaultinitexpr-import.cpp @@ -0,0 +1,26 @@ +namespace QHashPrivate { +template int b; +struct Data; +} // namespace QHashPrivate + +struct QDomNodePrivate {}; +template struct QMultiHash { + QHashPrivate::Data *d = nullptr; +}; + +struct QDomNamedNodeMapPrivate { + QMultiHash<> map; +}; +struct QDomElementPrivate : QDomNodePrivate { + QDomElementPrivate(); + void importee(); + QMultiHash<> *m_attr = nullptr; +}; +// --------- common part end --------- + +QDomElementPrivate::QDomElementPrivate() : m_attr{new QMultiHash<>} {} +void QDomElementPrivate::importee() { (void)QMultiHash<>{}; } +struct foo { + QDomElementPrivate m = {}; + static const int value = (QHashPrivate::b, 22); +}; diff --git a/clang/test/Analysis/Inputs/ctu-cxxdefaultinitexpr-import.cpp.externalDefMap.ast-dump.txt b/clang/test/Analysis/Inputs/ctu-cxxdefaultinitexpr-import.cpp.externalDefMap.ast-dump.txt new file mode 100644 index 00000000000000..7c8403c2fe6848 --- /dev/null +++ b/clang/test/Analysis/Inputs/ctu-cxxdefaultinitexpr-import.cpp.externalDefMap.ast-dump.txt @@ -0,0 +1,4 @@ +14:c:@S@foo@value ctu-cxxdefaultinitexpr-import.cpp.ast +35:c:@S@QDomElementPrivate@F@importee# ctu-cxxdefaultinitexpr-import.cpp.ast +45:c:@S@QDomElementPrivate@F@QDomElementPrivate# ctu-cxxdefaultinitexpr-import.cpp.ast +39:c:@S@QDomNodePrivate@F@QDomNodePrivate# ctu-cxxdefaultinitexpr-import.cpp.ast diff --git a/clang/test/Analysis/ctu-cxxdefaultinitexpr.cpp b/clang/test/Analysis/ctu-cxxdefaultinitexpr.cpp new file mode 100644 index 00000000000000..e785a76310bbbe --- /dev/null +++ b/clang/test/Analysis/ctu-cxxdefaultinitexpr.cpp @@ -0,0 +1,35 @@ +// RUN: rm -rf %t && mkdir %t +// RUN: mkdir -p %t/ctudir +// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -std=c++17 \ +// RUN: -emit-pch -o %t/ctudir/ctu-cxxdefaultinitexpr-import.cpp.ast %S/Inputs/ctu-cxxdefaultinitexpr-import.cpp +// RUN: cp %S/Inputs/ctu-cxxdefaultinitexpr-import.cpp.externalDefMap.ast-dump.txt %t/ctudir/externalDefMap.txt +// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -std=c++17 -analyze \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-config experimental-enable-naive-ctu-analysis=true \ +// RUN: -analyzer-config ctu-dir=%t/ctudir \ +// RUN: -verify %s + +// Check that importing this code does not cause crash. +// expected-no-diagnostics + +namespace QHashPrivate { +template int b; +struct Data; +} // namespace QHashPrivate + +struct QDomNodePrivate {}; +template struct QMultiHash { + QHashPrivate::Data *d = nullptr; +}; + +struct QDomNamedNodeMapPrivate { + QMultiHash<> map; +}; +struct QDomElementPrivate : QDomNodePrivate { + QDomElementPrivate(); + void importee(); + QMultiHash<> *m_attr = nullptr; +}; +// --------- common part end --------- + +void importer(QDomElementPrivate x) { x.importee(); } diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index 04da7988fc308f..92b85e8ef7b00b 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -530,6 +530,21 @@ TEST_P(ImportExpr, ImportInitListExpr) { has(floatLiteral(equals(1.0))))))))); } +const internal::VariadicDynCastAllOfMatcher + cxxDefaultInitExpr; + +TEST_P(ImportExpr, ImportCXXDefaultInitExpr) { + MatchVerifier Verifier; + testImport("class declToImport { int DefInit = 5; }; declToImport X;", + Lang_CXX11, "", Lang_CXX11, Verifier, + cxxRecordDecl(hasDescendant(cxxConstructorDecl( + hasAnyConstructorInitializer(cxxCtorInitializer( + withInitializer(cxxDefaultInitExpr()))))))); + testImport( + "struct X { int A = 5; }; X declToImport{};", Lang_CXX17, "", Lang_CXX17, + Verifier, + varDecl(hasInitializer(initListExpr(hasInit(0, cxxDefaultInitExpr()))))); +} const internal::VariadicDynCastAllOfMatcher vaArgExpr; @@ -7475,6 +7490,143 @@ TEST_P(ASTImporterOptionSpecificTestBase, ImportDeductionGuideDifferentOrder) { ToDGOther); } +TEST_P(ASTImporterOptionSpecificTestBase, + ImportRecordWithLayoutRequestingExpr) { + TranslationUnitDecl *FromTU = getTuDecl( + R"( + struct A { + int idx; + static void foo(A x) { + (void)&"text"[x.idx]; + } + }; + )", + Lang_CXX11); + + auto *FromA = FirstDeclMatcher().match( + FromTU, cxxRecordDecl(hasName("A"))); + + // Test that during import of 'foo' the record layout can be obtained without + // crash. + auto *ToA = Import(FromA, Lang_CXX11); + EXPECT_TRUE(ToA); + EXPECT_TRUE(ToA->isCompleteDefinition()); +} + +TEST_P(ASTImporterOptionSpecificTestBase, + ImportRecordWithLayoutRequestingExprDifferentRecord) { + TranslationUnitDecl *FromTU = getTuDecl( + R"( + struct B; + struct A { + int idx; + B *b; + }; + struct B { + static void foo(A x) { + (void)&"text"[x.idx]; + } + }; + )", + Lang_CXX11); + + auto *FromA = FirstDeclMatcher().match( + FromTU, cxxRecordDecl(hasName("A"))); + + // Test that during import of 'foo' the record layout (of 'A') can be obtained + // without crash. It is not possible to have all of the fields of 'A' imported + // at that time (without big code changes). + auto *ToA = Import(FromA, Lang_CXX11); + EXPECT_TRUE(ToA); + EXPECT_TRUE(ToA->isCompleteDefinition()); +} + +TEST_P(ASTImporterOptionSpecificTestBase, ImportInClassInitializerFromField) { + // Encounter import of a field when the field already exists but has the + // in-class initializer expression not yet set. Such case can occur in the AST + // of generated template specializations. + // The first code forces to create a template specialization of + // `A` but without implicit constructors. + // The second ("From") code contains a variable of type `A`, this + // results in a template specialization that has constructors and + // CXXDefaultInitExpr nodes. + Decl *ToTU = getToTuDecl( + R"( + void f(); + template struct A { int X = 1; }; + struct B { A Y; }; + )", + Lang_CXX11); + auto *ToX = FirstDeclMatcher().match( + ToTU, + fieldDecl(hasName("X"), hasParent(classTemplateSpecializationDecl()))); + ASSERT_TRUE(ToX->hasInClassInitializer()); + ASSERT_FALSE(ToX->getInClassInitializer()); + + Decl *FromTU = getTuDecl( + R"( + void f(); + template struct A { int X = 1; }; + struct B { A Y; }; + // + A Z; + )", + Lang_CXX11, "input1.cc"); + auto *FromX = FirstDeclMatcher().match( + FromTU, + fieldDecl(hasName("X"), hasParent(classTemplateSpecializationDecl()))); + + auto *ToXImported = Import(FromX, Lang_CXX11); + EXPECT_EQ(ToXImported, ToX); + EXPECT_TRUE(ToX->getInClassInitializer()); +} + +TEST_P(ASTImporterOptionSpecificTestBase, + ImportInClassInitializerFromCXXDefaultInitExpr) { + // Encounter AST import of a CXXDefaultInitExpr where the "to-field" + // of it exists but has the in-class initializer not set yet. + Decl *ToTU = getToTuDecl( + R"( + namespace N { + template int b; + struct X; + } + template struct A { N::X *X = nullptr; }; + struct B { A Y; }; + )", + Lang_CXX14); + auto *ToX = FirstDeclMatcher().match( + ToTU, + fieldDecl(hasName("X"), hasParent(classTemplateSpecializationDecl()))); + ASSERT_TRUE(ToX->hasInClassInitializer()); + ASSERT_FALSE(ToX->getInClassInitializer()); + + Decl *FromTU = getTuDecl( + R"( + namespace N { + template int b; + struct X; + } + template struct A { N::X *X = nullptr; }; + struct B { A Y; }; + // + void f() { + (void)A{}; + } + struct C { + C(): attr(new A{}){} + A *attr; + const int value = N::b; + }; + )", + Lang_CXX14, "input1.cc"); + auto *FromF = FirstDeclMatcher().match( + FromTU, functionDecl(hasName("f"), isDefinition())); + auto *ToF = Import(FromF, Lang_CXX11); + EXPECT_TRUE(ToF); + EXPECT_TRUE(ToX->getInClassInitializer()); +} + INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest, DefaultTestValuesForRunOptions);