Author: dingfei Date: 2023-08-02T11:21:10+08:00 New Revision: 91b7952afc82f90e7b810701d9d5c776a8e21688
URL: https://github.com/llvm/llvm-project/commit/91b7952afc82f90e7b810701d9d5c776a8e21688 DIFF: https://github.com/llvm/llvm-project/commit/91b7952afc82f90e7b810701d9d5c776a8e21688.diff LOG: [ASTImporter] Fix corrupted RecordLayout caused by circular referenced fields UnaryOperator(&)'s creation might need layout of some records whose fields importation are still on fly, the layout is incorrectly computed and cached. Clients relying on this will not work properly or crash direclty (e.g StaticAnalyzer's MemRegion.cpp (calculateOffset)). Use UnaryOperator::CreateEmpty() instead of UnaryOperator::Create() to avoid this computation. Fixes https://github.com/llvm/llvm-project/issues/64170 Reviewed By: aaron.ballman, balazske, shafik Differential Revision: https://reviews.llvm.org/D156201 Added: Modified: clang/docs/ReleaseNotes.rst clang/include/clang/AST/Expr.h clang/lib/AST/ASTImporter.cpp clang/unittests/AST/ASTImporterTest.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index a536a9fe2363cf..381346f9605b4a 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -133,6 +133,10 @@ Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ - Fixed an import failure of recursive friend class template. `Issue 64169 <https://github.com/llvm/llvm-project/issues/64169>`_ +- Remove unnecessary RecordLayout computation when importing UnaryOperator. The + computed RecordLayout is incorrect if fields are not completely imported and + should not be cached. + `Issue 64170 <https://github.com/llvm/llvm-project/issues/64170>`_ Miscellaneous Bug Fixes ^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index f9795b6386c46f..6737721e1ed1b2 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -2341,7 +2341,7 @@ class UnaryOperator final } protected: - /// Set FPFeatures in trailing storage, used only by Serialization + /// Set FPFeatures in trailing storage, used by Serialization & ASTImporter. void setStoredFPFeatures(FPOptionsOverride F) { getTrailingFPFeatures() = F; } public: @@ -2359,6 +2359,7 @@ class UnaryOperator final } friend TrailingObjects; + friend class ASTNodeImporter; friend class ASTReader; friend class ASTStmtReader; friend class ASTStmtWriter; diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 1ad7067f8ca9c1..f122c09629e1c2 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -7414,10 +7414,17 @@ ExpectedStmt ASTNodeImporter::VisitUnaryOperator(UnaryOperator *E) { if (Err) return std::move(Err); - return UnaryOperator::Create( - Importer.getToContext(), ToSubExpr, E->getOpcode(), ToType, - E->getValueKind(), E->getObjectKind(), ToOperatorLoc, E->canOverflow(), - E->getFPOptionsOverride()); + auto *UO = UnaryOperator::CreateEmpty(Importer.getToContext(), + E->hasStoredFPFeatures()); + UO->setType(ToType); + UO->setSubExpr(ToSubExpr); + UO->setOpcode(E->getOpcode()); + UO->setOperatorLoc(ToOperatorLoc); + UO->setCanOverflow(E->canOverflow()); + if (E->hasStoredFPFeatures()) + UO->setStoredFPFeatures(E->getStoredFPFeatures()); + + return UO; } ExpectedStmt diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index 544550f05fd747..4c53d4b7eee9d8 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -10,6 +10,7 @@ // //===----------------------------------------------------------------------===// +#include "clang/AST/RecordLayout.h" #include "clang/ASTMatchers/ASTMatchers.h" #include "llvm/ADT/StringMap.h" #include "llvm/Support/SmallVectorMemoryBuffer.h" @@ -2333,7 +2334,7 @@ TEST_P(ImportFunctions, } TEST_P(ASTImporterOptionSpecificTestBase, - ImportVirtualOverriddenMethodOnALoopTest) { + ImportVirtualOverriddenMethodOnALoop) { // B::f() calls => f1() ==> C ==> C::f() // \ // \---- A::f() @@ -8057,7 +8058,7 @@ TEST_P(ASTImporterOptionSpecificTestBase, ImportDeductionGuideDifferentOrder) { } TEST_P(ASTImporterOptionSpecificTestBase, - ImportFieldsFirstForCorrectRecordLayoutTest) { + ImportFieldsFirstForCorrectRecordLayout) { // UnaryOperator(&) triggers RecordLayout computation, which relies on // correctly imported fields. auto Code = @@ -8077,6 +8078,45 @@ TEST_P(ASTImporterOptionSpecificTestBase, Import(FromF, Lang_CXX11); } +TEST_P(ASTImporterOptionSpecificTestBase, + ImportCirularRefFieldsWithoutCorruptedRecordLayoutCache) { + // Import sequence: A => A.b => B => B.f() => ... => UnaryOperator(&) => ... + // + // UnaryOperator(&) should not introduce invalid RecordLayout since 'A' is + // still not completely imported. + auto Code = + R"( + class B; + class A { + B* b; + int c; + }; + class B { + A *f() { return &((B *)0)->a; } + A a; + }; + )"; + + auto *FromR = FirstDeclMatcher<CXXRecordDecl>().match( + getTuDecl(Code, Lang_CXX11), cxxRecordDecl(hasName("A"))); + FromR = FromR->getDefinition(); + auto &FromAST = FromR->getASTContext(); + auto *ToR = Import(FromR, Lang_CXX11); + auto &ToAST = ToR->getASTContext(); + + uint64_t SecondFieldOffset = FromAST.getTypeSize(FromAST.VoidPtrTy); + + EXPECT_TRUE(FromR->isCompleteDefinition()); + const auto &FromLayout = FromAST.getASTRecordLayout(FromR); + EXPECT_TRUE(FromLayout.getFieldOffset(0) == 0); + EXPECT_TRUE(FromLayout.getFieldOffset(1) == SecondFieldOffset); + + EXPECT_TRUE(ToR->isCompleteDefinition()); + const auto &ToLayout = ToAST.getASTRecordLayout(ToR); + EXPECT_TRUE(ToLayout.getFieldOffset(0) == 0); + EXPECT_TRUE(ToLayout.getFieldOffset(1) == SecondFieldOffset); +} + TEST_P(ASTImporterOptionSpecificTestBase, ImportRecordWithLayoutRequestingExpr) { TranslationUnitDecl *FromTU = getTuDecl( _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits