[PATCH] D89319: [ASTImporter] Fix crash caused by unimported type of FromatAttr
Gabor Marton via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 13 07:13:47 PDT 2020
martong created this revision.
martong added reviewers: teemperor, shafik.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a project: clang.
martong requested review of this revision.
During the import of FormatAttrs we forgot to import the type (e.g
`__scanf__`) of the attribute. 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:
==1941== Invalid read of size 8
==1941== at 0xB112EE6: clang::TextNodeDumper::VisitFormatAttr(clang::FormatAttr const*) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
==1941== by 0xB11CB6C: 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)
==1941== 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)
==1941== 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)
==1941== 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)
==1941== 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)
==1941== 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)
==1941== by 0xADD4226: clang::Decl::dump(llvm::raw_ostream&, bool, clang::ASTDumpOutputFormat) const (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
==1941== by 0x73CA1B5: (anonymous namespace)::ASTPrinter::HandleTranslationUnit(clang::ASTContext&) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
==1941== by 0xB84A568: clang::ParseAST(clang::Sema&, bool, bool) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangParse.so.12git)
==1941== by 0x7401AA0: clang::ASTMergeAction::ExecuteAction() (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
==1941== by 0x747C298: clang::FrontendAction::Execute() (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
==1941== Address 0xe42fab8 is 3,576 bytes inside a block of size 4,096 free'd
==1941== at 0x4C3123B: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1941== by 0xB4A4D3F: clang::Preprocessor::~Preprocessor() (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangLex.so.12git)
==1941== 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)
==1941== 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)
==1941== by 0x741DB08: clang::ASTUnit::~ASTUnit() (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
==1941== by 0x7401CFF: clang::ASTMergeAction::ExecuteAction() (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
==1941== by 0x747C298: clang::FrontendAction::Execute() (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
==1941== by 0x742EA69: clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
==1941== by 0x40DC565: clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontendTool.so.12git)
==1941== by 0x11BE7B: cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (in /home/egbomrt/WORK/llvm5/build/release_assert/bin/clang-11)
==1941== by 0x116E38: ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&) (in /home/egbomrt/WORK/llvm5/build/release_assert/bin/clang-11)
==1941== by 0x114DC3: main (in /home/egbomrt/WORK/llvm5/build/release_assert/bin/clang-11)
==1941== Block was alloc'd at
==1941== at 0x4C3017F: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1941== by 0x8709BE7: clang::IdentifierTable::get(llvm::StringRef) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangBasic.so.12git)
==1941== 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)
==1941== by 0x872B71C: clang::IdentifierTable::AddKeywords(clang::LangOptions const&) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangBasic.so.12git)
==1941== by 0x7418AF9: (anonymous namespace)::ASTInfoCollector::ReadTargetOptions(clang::TargetOptions const&, bool, bool) (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
==1941== 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)
==1941== 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)
==1941== 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)
==1941== 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)
==1941== 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)
==1941== 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)
==1941== by 0x7401A10: clang::ASTMergeAction::ExecuteAction() (in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D89319
Files:
clang/lib/AST/ASTImporter.cpp
clang/test/ASTMerge/attr/Inputs/FormatAttr.cpp
clang/test/ASTMerge/attr/testFormatAttr.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, ImportFormatAttr) {
+ Decl *FromTU = getTuDecl(
+ R"(
+ int foo(const char * fmt, ...)
+ __attribute__ ((__format__ (__scanf__, 1, 2)));
+ )",
+ 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<FormatAttr>();
+ auto *ToAttr = ToD->getAttr<FormatAttr>();
+ 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());
+ EXPECT_EQ(FromAttr->getType()->getName(), ToAttr->getType()->getName());
+}
template <typename T>
auto ExtendWithOptions(const T &Values, const std::vector<std::string> &Args) {
auto Copy = Values;
Index: clang/test/ASTMerge/attr/testFormatAttr.cpp
===================================================================
--- /dev/null
+++ clang/test/ASTMerge/attr/testFormatAttr.cpp
@@ -0,0 +1,2 @@
+// RUN: %clang -x c++-header -o %t.a.ast %S/Inputs/FormatAttr.cpp
+// RUN: %clang_cc1 -x c++ -ast-merge %t.a.ast /dev/null -ast-dump
Index: clang/test/ASTMerge/attr/Inputs/FormatAttr.cpp
===================================================================
--- /dev/null
+++ clang/test/ASTMerge/attr/Inputs/FormatAttr.cpp
@@ -0,0 +1,2 @@
+int foo(const char * fmt, ...)
+__attribute__ ((__format__ (__scanf__, 1, 2)));
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -8099,6 +8099,16 @@
ToAttr = To;
break;
}
+ case attr::Format: {
+ const auto *From = cast<FormatAttr>(FromAttr);
+ FormatAttr *To;
+ IdentifierInfo *ToAttrType = Import(From->getType());
+ To = FormatAttr::Create(ToContext, ToAttrType, From->getFormatIdx(),
+ From->getFirstArg(), ToRange, From->getSyntax());
+ To->setInherited(From->isInherited());
+ ToAttr = To;
+ break;
+ }
default:
// FIXME: 'clone' copies every member but some of them should be imported.
// Handle other Attrs that have parameters that should be imported.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D89319.297854.patch
Type: text/x-patch
Size: 2783 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201013/3d2f1c5e/attachment.bin>
More information about the cfe-commits
mailing list