[PATCH] D89318: [ASTImporter] Fix crash caused by unset AttributeSpellingListIndex
Gabor Marton via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 13 07:12:29 PDT 2020
martong created this revision.
martong added reviewers: teemperor, shafik.
Herald added subscribers: cfe-commits, pengfei, gamesh411, Szelethus, arphaman, dkrupp, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a project: clang.
martong requested review of this revision.
During the import of attributes we forgot to set the spelling list
index. This caused a segfault when we wanted to traverse the AST
(e.g. by the dump() method).
Here is the relevant valgrind trace of the ASTMerge test when accessing
an already freed memory region:
==25717== Invalid read of size 8
==25717== at 0x86E96FA: clang::AttributeCommonInfo::calculateAttributeSpellingListIndex() const (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangBasic.so.12git)
==25717== by 0xAE73598: clang::RestrictAttr::getSpelling() const (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
==25717== by 0xB11D93B: clang::attrvisitor::Base<llvm::make_const_ptr, clang::TextNodeDumper, void>::Visit(clang::Attr const*) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
==25717== by 0xADC0880: std::_Function_handler<void (bool), void clang::TextTreeStructure::AddChild<clang::ASTNodeTraverser<clang::ASTDumper, clang::TextNodeDumper>::Visit(clang::Attr const*)::{lambda()#1}>(llvm::StringRef, clang::ASTNodeTraverser<clang::ASTDumper, clang::TextNodeDumper>::Visit(clang::Attr const*)::{lambda()#1})::{lambda(bool)#1}>::_M_invoke(std::_Any_data const&, bool&&) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
==25717== by 0xADC9DAD: std::_Function_handler<void (bool), void clang::TextTreeStructure::AddChild<clang::ASTNodeTraverser<clang::ASTDumper, clang::TextNodeDumper>::Visit(clang::Decl const*)::{lambda()#1}>(llvm::StringRef, clang::ASTNodeTraverser<clang::ASTDumper, clang::TextNodeDumper>::Visit(clang::Decl const*)::{lambda()#1})::{lambda(bool)#1}>::_M_invoke(std::_Any_data const&, bool&&) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
==25717== by 0xADBE58A: void clang::TextTreeStructure::AddChild<clang::ASTNodeTraverser<clang::ASTDumper, clang::TextNodeDumper>::Visit(clang::Decl const*)::{lambda()#1}>(llvm::StringRef, clang::ASTNodeTraverser<clang::ASTDumper, clang::TextNodeDumper>::Visit(clang::Decl const*)::{lambda()#1}) [clone .constprop.2050] (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
==25717== by 0xADBE2BA: clang::ASTNodeTraverser<clang::ASTDumper, clang::TextNodeDumper>::Visit(clang::Decl const*)::{lambda()#1}::operator()() const (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
==25717== by 0xADBE706: void clang::TextTreeStructure::AddChild<clang::ASTNodeTraverser<clang::ASTDumper, clang::TextNodeDumper>::Visit(clang::Decl const*)::{lambda()#1}>(llvm::StringRef, clang::ASTNodeTraverser<clang::ASTDumper, clang::TextNodeDumper>::Visit(clang::Decl const*)::{lambda()#1}) [clone .constprop.2050] (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
==25717== by 0xADD4226: clang::Decl::dump(llvm::raw_ostream&, bool, clang::ASTDumpOutputFormat) const (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
==25717== by 0x73CA1B5: (anonymous namespace)::ASTPrinter::HandleTranslationUnit(clang::ASTContext&) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
==25717== by 0xB84A568: clang::ParseAST(clang::Sema&, bool, bool) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangParse.so.12git)
==25717== by 0x7401AA0: clang::ASTMergeAction::ExecuteAction() (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
==25717== Address 0xe42fa88 is 3,528 bytes inside a block of size 4,096 free'd
==25717== at 0x4C3123B: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==25717== by 0xB4A4D3F: clang::Preprocessor::~Preprocessor() (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangLex.so.12git)
==25717== by 0x7402D6C: std::_Sp_counted_deleter<clang::Preprocessor*, std::__shared_ptr<clang::Preprocessor, (__gnu_cxx::_Lock_policy)2>::_Deleter<std::allocator<clang::Preprocessor> >, std::allocator<clang::Preprocessor>, (__gnu_cxx::_Lock_policy)2>::_M_dispose() (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
==25717== by 0x57548D5: std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangCodeGen.so.12git)
==25717== by 0x741DB08: clang::ASTUnit::~ASTUnit() (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
==25717== by 0x7401CFF: clang::ASTMergeAction::ExecuteAction() (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
==25717== by 0x747C298: clang::FrontendAction::Execute() (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
==25717== by 0x742EA69: clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
==25717== by 0x40DC565: clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontendTool.so.12git)
==25717== by 0x11BE7B: cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (in /home/egbomrt/WORK/llvm5/build/release_assert/bin/clang-11)
==25717== by 0x116E38: ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&) (in /home/egbomrt/WORK/llvm5/build/release_assert/bin/clang-11)
==25717== by 0x114DC3: main (in /home/egbomrt/WORK/llvm5/build/release_assert/bin/clang-11)
==25717== Block was alloc'd at
==25717== at 0x4C3017F: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==25717== by 0x8709BE7: clang::IdentifierTable::get(llvm::StringRef) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangBasic.so.12git)
==25717== by 0x8729BFC: AddKeyword(llvm::StringRef, clang::tok::TokenKind, unsigned int, clang::LangOptions const&, clang::IdentifierTable&) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangBasic.so.12git)
==25717== by 0x872B71C: clang::IdentifierTable::AddKeywords(clang::LangOptions const&) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangBasic.so.12git)
==25717== by 0x7418AF9: (anonymous namespace)::ASTInfoCollector::ReadTargetOptions(clang::TargetOptions const&, bool, bool) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
==25717== by 0x786502C: clang::ASTReader::ParseTargetOptions(llvm::SmallVector<unsigned long, 64u> const&, bool, clang::ASTReaderListener&, bool) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangSerialization.so.12git)
==25717== by 0x7871E84: clang::ASTReader::ReadOptionsBlock(llvm::BitstreamCursor&, unsigned int, bool, clang::ASTReaderListener&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangSerialization.so.12git)
==25717== by 0x787608D: clang::ASTReader::ReadControlBlock(clang::serialization::ModuleFile&, llvm::SmallVectorImpl<clang::ASTReader::ImportedModule>&, clang::serialization::ModuleFile const*, unsigned int) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangSerialization.so.12git)
==25717== by 0x787700A: clang::ASTReader::ReadASTCore(llvm::StringRef, clang::serialization::ModuleKind, clang::SourceLocation, clang::serialization::ModuleFile*, llvm::SmallVectorImpl<clang::ASTReader::ImportedModule>&, long, long, clang::ASTFileSignature, unsigned int) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangSerialization.so.12git)
==25717== by 0x78BDB8E: clang::ASTReader::ReadAST(llvm::StringRef, clang::serialization::ModuleKind, clang::SourceLocation, unsigned int, llvm::SmallVectorImpl<clang::ASTReader::ImportedSubmodule>*) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangSerialization.so.12git)
==25717== by 0x741EDC6: clang::ASTUnit::LoadFromASTFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, clang::PCHContainerReader const&, clang::ASTUnit::WhatToLoad, llvm::IntrusiveRefCntPtr<clang::DiagnosticsEngine>, clang::FileSystemOptions const&, bool, bool, llvm::ArrayRef<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, llvm::MemoryBuffer*> >, clang::CaptureDiagsKind, bool, bool) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
==25717== by 0x7401A10: clang::ASTMergeAction::ExecuteAction() (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
==25717==
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D89318
Files:
clang/lib/AST/ASTImporter.cpp
clang/test/ASTMerge/attr/Inputs/RestrictAttr.cpp
clang/test/ASTMerge/attr/testRestrictAttr.cpp
clang/unittests/AST/ASTImporterTest.cpp
Index: clang/unittests/AST/ASTImporterTest.cpp
===================================================================
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -5767,6 +5767,31 @@
EXPECT_TRUE(ToA);
}
+TEST_P(ASTImporterOptionSpecificTestBase, ImportRestrictAttr) {
+ Decl *FromTU = getTuDecl(
+ R"(
+ void *foo(unsigned, unsigned)
+ __attribute__((__malloc__));
+ )",
+ Lang_CXX03, "input.cc");
+ auto *FromD = FirstDeclMatcher<FunctionDecl>().match(
+ FromTU, functionDecl(hasName("foo")));
+ ASSERT_TRUE(FromD);
+
+ auto *ToD = Import(FromD, Lang_CXX03);
+ ASSERT_TRUE(ToD);
+ ToD->dump(); // Should not crash!
+
+ auto *FromAttr = FromD->getAttr<RestrictAttr>();
+ auto *ToAttr = ToD->getAttr<RestrictAttr>();
+ EXPECT_EQ(FromAttr->isInherited(), ToAttr->isInherited());
+ EXPECT_EQ(FromAttr->isPackExpansion(), ToAttr->isPackExpansion());
+ EXPECT_EQ(FromAttr->isImplicit(), ToAttr->isImplicit());
+ EXPECT_EQ(FromAttr->getSyntax(), ToAttr->getSyntax());
+ EXPECT_EQ(FromAttr->getAttributeSpellingListIndex(),
+ ToAttr->getAttributeSpellingListIndex());
+}
+
template <typename T>
auto ExtendWithOptions(const T &Values, const std::vector<std::string> &Args) {
auto Copy = Values;
Index: clang/test/ASTMerge/attr/testRestrictAttr.cpp
===================================================================
--- /dev/null
+++ clang/test/ASTMerge/attr/testRestrictAttr.cpp
@@ -0,0 +1,2 @@
+// RUN: %clang -x c++-header -o %t.a.ast %S/Inputs/RestrictAttr.cpp
+// RUN: %clang_cc1 -x c++ -ast-merge %t.a.ast /dev/null -ast-dump
Index: clang/test/ASTMerge/attr/Inputs/RestrictAttr.cpp
===================================================================
--- /dev/null
+++ clang/test/ASTMerge/attr/Inputs/RestrictAttr.cpp
@@ -0,0 +1,2 @@
+void *foo(unsigned, unsigned)
+__attribute__((__malloc__));
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -8094,8 +8094,6 @@
return ToTOrErr.takeError();
}
To->setInherited(From->isInherited());
- To->setPackExpansion(From->isPackExpansion());
- To->setImplicit(From->isImplicit());
ToAttr = To;
break;
}
@@ -8106,8 +8104,11 @@
ToAttr->setRange(ToRange);
break;
}
+ ToAttr->setAttributeSpellingListIndex(
+ FromAttr->getAttributeSpellingListIndex());
+ ToAttr->setPackExpansion(FromAttr->isPackExpansion());
+ ToAttr->setImplicit(FromAttr->isImplicit());
assert(ToAttr && "Attribute should be created.");
-
return ToAttr;
}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D89318.297853.patch
Type: text/x-patch
Size: 2664 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201013/8f807a7a/attachment-0001.bin>
More information about the cfe-commits
mailing list