Author: Balazs Benics Date: 2021-05-18T09:43:20+02:00 New Revision: 88ee91cd87794813f4394f82d2c693c8d766e1d2
URL: https://github.com/llvm/llvm-project/commit/88ee91cd87794813f4394f82d2c693c8d766e1d2 DIFF: https://github.com/llvm/llvm-project/commit/88ee91cd87794813f4394f82d2c693c8d766e1d2.diff LOG: [ASTimporter] Remove decl from lookup only if it has decl context In the case of TypedefDecls we set the DeclContext after we imported it. It turns out, it could lead to null pointer dereferences during the cleanup part of a failed import. This patch demonstrates this issue and fixes it by checking if the DeclContext is available or not. Reviewed By: shafik Differential Revision: https://reviews.llvm.org/D102640 Added: Modified: clang/lib/AST/ASTImporter.cpp clang/unittests/AST/ASTImporterTest.cpp Removed: ################################################################################ diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index dd11e3662148e..9caa1178c0b7b 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -8383,7 +8383,11 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) { // traverse of the 'to' context). auto PosF = ImportedFromDecls.find(ToD); if (PosF != ImportedFromDecls.end()) { - SharedState->removeDeclFromLookup(ToD); + // In the case of TypedefNameDecl we create the Decl first and only + // then we import and set its DeclContext. So, the DC might not be set + // when we reach here. + if (ToD->getDeclContext()) + SharedState->removeDeclFromLookup(ToD); ImportedFromDecls.erase(PosF); } diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index e62203f286572..db87b2ab5e9c0 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -5225,6 +5225,40 @@ TEST_P(ErrorHandlingTest, ErrorIsNotPropagatedFromMemberToNamespace) { EXPECT_TRUE(ImportedOK); } +TEST_P(ErrorHandlingTest, ODRViolationWithinTypedefDecls) { + // Importing `z` should fail - instead of crashing - due to an ODR violation. + // The `bar::e` typedef sets it's DeclContext after the import is done. + // However, if the importation fails, it will be left as a nullptr. + // During the cleanup of the failed import, we should check whether the + // DeclContext is null or not - instead of dereferencing that unconditionally. + constexpr auto ToTUCode = R"( + namespace X { + struct bar { + int odr_violation; + }; + })"; + constexpr auto FromTUCode = R"( + namespace X { + enum b {}; + struct bar { + typedef b e; + static e d; + }; + } + int z = X::bar::d; + )"; + Decl *ToTU = getToTuDecl(ToTUCode, Lang_CXX11); + static_cast<void>(ToTU); + Decl *FromTU = getTuDecl(FromTUCode, Lang_CXX11); + auto *FromZ = + FirstDeclMatcher<VarDecl>().match(FromTU, varDecl(hasName("z"))); + ASSERT_TRUE(FromZ); + ASSERT_TRUE(FromZ->hasInit()); + + auto *ImportedZ = Import(FromZ, Lang_CXX11); + EXPECT_FALSE(ImportedZ); +} + // An error should be set for a class if it had a previous import with an error // from another TU. TEST_P(ErrorHandlingTest, _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
