From a7304199365a61862364ef788330ebe8165eeb11 Mon Sep 17 00:00:00 2001 From: Logan Chien Date: Fri, 19 Oct 2018 17:28:51 +0800 Subject: [PATCH] header-checker: Code cleanup This commit breaks `HeaderAbiLinker::LinkAndDump()` into several functions, moves `llvm::object::Binary`-related code to `SoFileParser`, removes unused code in `IRReader`, and remove some space characters before semicolon. Test: ./tests/test.py Change-Id: I9f90f07985c84ff95bf093c65ede091d44849e33 --- .../src/header_abi_linker.cpp | 150 ++++++++++-------- .../include/ir_representation.h | 12 -- .../header-abi-util/include/so_file_parser.h | 34 +--- .../src/collect_exported_headers.cpp | 5 +- .../header-abi-util/src/so_file_parser.cpp | 120 ++++++++------ 5 files changed, 164 insertions(+), 157 deletions(-) diff --git a/vndk/tools/header-checker/header-abi-linker/src/header_abi_linker.cpp b/vndk/tools/header-checker/header-abi-linker/src/header_abi_linker.cpp index cb3fcb09e..3b81b9ac7 100644 --- a/vndk/tools/header-checker/header-abi-linker/src/header_abi_linker.cpp +++ b/vndk/tools/header-checker/header-abi-linker/src/header_abi_linker.cpp @@ -106,9 +106,13 @@ class HeaderAbiLinker { const abi_util::AbiElementMap &src, const std::function &symbol_filter); - bool ParseVersionScriptFiles(); + std::unique_ptr ReadInputDumpFiles(); - bool ParseSoFile(); + bool ReadExportedSymbols(); + + bool ReadExportedSymbolsFromVersionScript(); + + bool ReadExportedSymbolsFromSharedObjectFile(); bool LinkTypes(const abi_util::TextFormatToIRReader *ir_reader, abi_util::IRDumper *ir_dumper); @@ -119,8 +123,7 @@ class HeaderAbiLinker { bool LinkGlobalVars(const abi_util::TextFormatToIRReader *ir_reader, abi_util::IRDumper *ir_dumper); - bool AddElfSymbols(abi_util::IRDumper *ir_dumper); - + bool LinkExportedSymbols(abi_util::IRDumper *ir_dumper); private: const std::vector &dump_files_; @@ -142,23 +145,6 @@ class HeaderAbiLinker { std::regex globvars_vs_regex_; }; -template -static bool AddElfSymbols(abi_util::IRDumper *dst, - const std::map &symbols) { - for (auto &&symbol : symbols) { - if (!dst->AddElfSymbolMessageIR(&(symbol.second))) { - return false; - } - } - return true; -} - -// To be called right after parsing the .so file / version script. -bool HeaderAbiLinker::AddElfSymbols(abi_util::IRDumper *ir_dumper) { - return ::AddElfSymbols(ir_dumper, function_decl_map_) && - ::AddElfSymbols(ir_dumper, globvar_decl_map_); -} - static void DeDuplicateAbiElementsThread( const std::vector &dump_files, const std::set *exported_headers, @@ -167,6 +153,7 @@ static void DeDuplicateAbiElementsThread( std::unique_ptr local_reader = abi_util::TextFormatToIRReader::CreateTextFormatToIRReader( input_format, exported_headers); + auto begin_it = dump_files.begin(); std::size_t num_sources = dump_files.size(); while (1) { @@ -188,36 +175,20 @@ static void DeDuplicateAbiElementsThread( local_reader->MergeGraphs(*reader); } } + std::lock_guard lock(*greader_lock); greader->MergeGraphs(*local_reader); } -bool HeaderAbiLinker::LinkAndDump() { - // If the user specifies that a version script should be used, use that. - if (!so_file_.empty()) { - exported_headers_ = - abi_util::CollectAllExportedHeaders(exported_header_dirs_); - if (!ParseSoFile()) { - llvm::errs() << "Couldn't parse so file\n"; - return false; - } - } else if (!ParseVersionScriptFiles()) { - llvm::errs() << "Failed to parse stub files for exported symbols\n"; - return false; - } - std::unique_ptr ir_dumper = - abi_util::IRDumper::CreateIRDumper(output_format, out_dump_name_); - assert(ir_dumper != nullptr); - AddElfSymbols(ir_dumper.get()); - // Create a reader, on which we never actually call ReadDump(), since multiple - // dump files are associated with it. +std::unique_ptr +HeaderAbiLinker::ReadInputDumpFiles() { std::unique_ptr greader = abi_util::TextFormatToIRReader::CreateTextFormatToIRReader( input_format, &exported_headers_); + std::size_t max_threads = std::thread::hardware_concurrency(); std::size_t num_threads = kSourcesPerBatchThread < dump_files_.size() ? - std::min(dump_files_.size() / kSourcesPerBatchThread, - max_threads) : 0; + std::min(dump_files_.size() / kSourcesPerBatchThread, max_threads) : 0; std::vector threads; std::atomic cnt(0); std::mutex greader_lock; @@ -232,16 +203,42 @@ bool HeaderAbiLinker::LinkAndDump() { thread.join(); } + return greader; +} + +bool HeaderAbiLinker::LinkAndDump() { + // Extract exported functions and variables from a shared lib or a version + // script. + if (!ReadExportedSymbols()) { + return false; + } + + // Construct the list of exported headers for source location filtering. + exported_headers_ = + abi_util::CollectAllExportedHeaders(exported_header_dirs_); + + // Read all input ABI dumps. + auto greader = ReadInputDumpFiles(); + + // Link input ABI dumps. + std::unique_ptr ir_dumper = + abi_util::IRDumper::CreateIRDumper(output_format, out_dump_name_); + assert(ir_dumper != nullptr); + + LinkExportedSymbols(ir_dumper.get()); + if (!LinkTypes(greader.get(), ir_dumper.get()) || !LinkFunctions(greader.get(), ir_dumper.get()) || !LinkGlobalVars(greader.get(), ir_dumper.get())) { llvm::errs() << "Failed to link elements\n"; return false; } + if (!ir_dumper->Dump()) { - llvm::errs() << "Serialization to ostream failed\n"; + llvm::errs() << "Failed to serialize the linked output to ostream\n"; return false; } + return true; } @@ -345,15 +342,56 @@ bool HeaderAbiLinker::LinkGlobalVars( return LinkDecl(ir_dumper, reader->GetGlobalVariables(), symbol_filter); } -bool HeaderAbiLinker::ParseVersionScriptFiles() { +template +static bool LinkExportedSymbols(abi_util::IRDumper *dst, + const std::map &symbols) { + for (auto &&symbol : symbols) { + if (!dst->AddElfSymbolMessageIR(&(symbol.second))) { + return false; + } + } + return true; +} + +// To be called right after parsing the .so file / version script. +bool HeaderAbiLinker::LinkExportedSymbols(abi_util::IRDumper *ir_dumper) { + return ::LinkExportedSymbols(ir_dumper, function_decl_map_) && + ::LinkExportedSymbols(ir_dumper, globvar_decl_map_); +} + +bool HeaderAbiLinker::ReadExportedSymbols() { + if (!so_file_.empty()) { + if (!ReadExportedSymbolsFromSharedObjectFile()) { + llvm::errs() << "Failed to parse the shared library (.so file): " + << so_file_ << "\n"; + return false; + } + return true; + } + + if (!version_script_.empty()) { + if (!ReadExportedSymbolsFromVersionScript()) { + llvm::errs() << "Failed to parse the version script: " << version_script_ + << "\n"; + return false; + } + return true; + } + + llvm::errs() << "Either shared lib or version script must be specified.\n"; + return false; +} + +bool HeaderAbiLinker::ReadExportedSymbolsFromVersionScript() { abi_util::VersionScriptParser version_script_parser( version_script_, arch_, api_); if (!version_script_parser.Parse()) { - llvm::errs() << "Failed to parse version script\n"; return false; } + function_decl_map_ = version_script_parser.GetFunctions(); globvar_decl_map_ = version_script_parser.GetGlobVars(); + std::set function_regexs = version_script_parser.GetFunctionRegexs(); std::set globvar_regexs = @@ -363,27 +401,13 @@ bool HeaderAbiLinker::ParseVersionScriptFiles() { return true; } -bool HeaderAbiLinker::ParseSoFile() { - auto Binary = llvm::object::createBinary(so_file_); - - if (!Binary) { - llvm::errs() << "Couldn't really create object File \n"; - return false; - } - llvm::object::ObjectFile *objfile = - llvm::dyn_cast(&(*Binary.get().getBinary())); - if (!objfile) { - llvm::errs() << "Not an object file\n"; - return false; - } - +bool HeaderAbiLinker::ReadExportedSymbolsFromSharedObjectFile() { std::unique_ptr so_parser = - abi_util::SoFileParser::Create(objfile); - if (so_parser == nullptr) { - llvm::errs() << "Couldn't create soFile Parser\n"; + abi_util::SoFileParser::Create(so_file_); + if (!so_parser) { return false; } - so_parser->GetSymbols(); + function_decl_map_ = so_parser->GetFunctions(); globvar_decl_map_ = so_parser->GetGlobVars(); return true; diff --git a/vndk/tools/header-checker/header-abi-util/include/ir_representation.h b/vndk/tools/header-checker/header-abi-util/include/ir_representation.h index 7235248b8..886c39e85 100644 --- a/vndk/tools/header-checker/header-abi-util/include/ir_representation.h +++ b/vndk/tools/header-checker/header-abi-util/include/ir_representation.h @@ -933,18 +933,6 @@ class TextFormatToIRReader { virtual bool ReadDump(const std::string &dump_file) = 0; - template - bool ReadDumps(Iterator begin, Iterator end) { - Iterator it = begin; - while (it != end) { - if (!ReadDump(*it)) { - return false; - } - ++it; - } - return true; - } - void Merge(TextFormatToIRReader &&addend) { MergeElements(&functions_, std::move(addend.functions_)); MergeElements(&global_variables_, std::move(addend.global_variables_)); diff --git a/vndk/tools/header-checker/header-abi-util/include/so_file_parser.h b/vndk/tools/header-checker/header-abi-util/include/so_file_parser.h index 9c035bdfe..b19b8f337 100644 --- a/vndk/tools/header-checker/header-abi-util/include/so_file_parser.h +++ b/vndk/tools/header-checker/header-abi-util/include/so_file_parser.h @@ -17,9 +17,6 @@ #include "ir_representation.h" -#include -#include - #include #include #include @@ -28,38 +25,13 @@ namespace abi_util { class SoFileParser { public: + static std::unique_ptr Create(const std::string &so_file_path); + virtual ~SoFileParser() {} - static std::unique_ptr Create( - const llvm::object::ObjectFile *obj); - virtual const std::map &GetFunctions() const = 0; + virtual const std::map &GetGlobVars() const = 0; - virtual void GetSymbols() = 0; -}; - -template -class ELFSoFileParser : public SoFileParser { - private: - LLVM_ELF_IMPORT_TYPES_ELFT(T) - typedef llvm::object::ELFFile ELFO; - typedef typename ELFO::Elf_Sym Elf_Sym; - - public: - ELFSoFileParser(const llvm::object::ELFObjectFile *obj) : obj_(obj) {} - ~ELFSoFileParser() override {} - - const std::map &GetFunctions() const override; - const std::map &GetGlobVars() const override; - void GetSymbols() override; - - private: - const llvm::object::ELFObjectFile *obj_; - std::map functions_; - std::map globvars_; - - private: - bool IsSymbolExported(const Elf_Sym *elf_sym) const; }; } // namespace abi_util diff --git a/vndk/tools/header-checker/header-abi-util/src/collect_exported_headers.cpp b/vndk/tools/header-checker/header-abi-util/src/collect_exported_headers.cpp index 39b3f6465..0b6b07978 100644 --- a/vndk/tools/header-checker/header-abi-util/src/collect_exported_headers.cpp +++ b/vndk/tools/header-checker/header-abi-util/src/collect_exported_headers.cpp @@ -50,7 +50,8 @@ bool CollectExportedHeaderSet(const std::string &dir_name, llvm::sys::fs::recursive_directory_iterator end; for ( ; walker != end; walker.increment(ec)) { if (ec) { - llvm::errs() << "Failed to walk dir : " << dir_name << "\n"; + llvm::errs() << "Failed to walk directory: " << dir_name << ": " + << ec.message() << "\n"; return false; } @@ -67,7 +68,7 @@ bool CollectExportedHeaderSet(const std::string &dir_name, llvm::ErrorOr status = walker->status(); if (!status) { - llvm::errs() << "Failed to stat file : " << file_path << "\n"; + llvm::errs() << "Failed to stat file: " << file_path << "\n"; return false; } diff --git a/vndk/tools/header-checker/header-abi-util/src/so_file_parser.cpp b/vndk/tools/header-checker/header-abi-util/src/so_file_parser.cpp index 5750c321e..fa4d975e7 100644 --- a/vndk/tools/header-checker/header-abi-util/src/so_file_parser.cpp +++ b/vndk/tools/header-checker/header-abi-util/src/so_file_parser.cpp @@ -21,65 +21,71 @@ #include #include -using llvm::ELF::STB_GLOBAL; -using llvm::ELF::STB_WEAK; -using llvm::ELF::STV_DEFAULT; -using llvm::ELF::STV_PROTECTED; -using llvm::dyn_cast; -using llvm::object::ELF32BEObjectFile; -using llvm::object::ELF32LEObjectFile; -using llvm::object::ELF64BEObjectFile; -using llvm::object::ELF64LEObjectFile; - namespace abi_util { template -static inline T UnWrap(llvm::Expected ValueOrError) { - if (!ValueOrError) { - llvm::errs() << "\nError: " << llvm::toString(ValueOrError.takeError()) +static inline T UnWrap(llvm::Expected value_or_error) { + if (!value_or_error) { + llvm::errs() << "\nerror: " << llvm::toString(value_or_error.takeError()) << ".\n"; llvm::errs().flush(); exit(1); } - return std::move(ValueOrError.get()); -} - -template -const std::map & -ELFSoFileParser::GetFunctions() const { - return functions_; -} - -template -const std::map & -ELFSoFileParser::GetGlobVars() const { - return globvars_; -} - -template -bool ELFSoFileParser::IsSymbolExported(const Elf_Sym *elf_sym) const { - unsigned char visibility = elf_sym->getVisibility(); - unsigned char binding = elf_sym->getBinding(); - return (binding == STB_GLOBAL || binding == STB_WEAK) && - (visibility == STV_DEFAULT || visibility == STV_PROTECTED); + return std::move(value_or_error.get()); } static abi_util::ElfSymbolIR::ElfSymbolBinding LLVMToIRSymbolBinding(unsigned char binding) { switch (binding) { - case STB_GLOBAL: + case llvm::ELF::STB_GLOBAL: return abi_util::ElfSymbolIR::ElfSymbolBinding::Global; - case STB_WEAK: + case llvm::ELF::STB_WEAK: return abi_util::ElfSymbolIR::ElfSymbolBinding::Weak; } assert(0); } template -void ELFSoFileParser::GetSymbols() { - assert(obj_ != nullptr); - for (auto symbol_it : obj_->getDynamicSymbolIterators()) { - const Elf_Sym *elf_sym = obj_->getSymbol(symbol_it.getRawDataRefImpl()); +class ELFSoFileParser : public SoFileParser { + private: + LLVM_ELF_IMPORT_TYPES_ELFT(T) + typedef llvm::object::ELFFile ELFO; + typedef typename ELFO::Elf_Sym Elf_Sym; + + public: + ELFSoFileParser(const llvm::object::ELFObjectFile *obj); + + ~ELFSoFileParser() override {} + + const std::map &GetFunctions() const override { + return functions_; + } + + const std::map &GetGlobVars() const override { + return globvars_; + } + + private: + bool IsSymbolExported(const Elf_Sym *elf_sym) const { + unsigned char visibility = elf_sym->getVisibility(); + unsigned char binding = elf_sym->getBinding(); + return ((binding == llvm::ELF::STB_GLOBAL || + binding == llvm::ELF::STB_WEAK) && + (visibility == llvm::ELF::STV_DEFAULT || + visibility == llvm::ELF::STV_PROTECTED)); + } + + private: + const llvm::object::ELFObjectFile *obj_; + std::map functions_; + std::map globvars_; +}; + +template +ELFSoFileParser::ELFSoFileParser(const llvm::object::ELFObjectFile *obj) { + assert(obj != nullptr); + for (auto symbol_it : obj->getDynamicSymbolIterators()) { + const Elf_Sym *elf_sym = obj->getSymbol(symbol_it.getRawDataRefImpl()); assert (elf_sym != nullptr); if (!IsSymbolExported(elf_sym) || elf_sym->isUndefined()) { continue; @@ -104,26 +110,42 @@ static std::unique_ptr CreateELFSoFileParser( } std::unique_ptr SoFileParser::Create( - const llvm::object::ObjectFile *objfile) { + const std::string &so_file_path) { + auto binary = llvm::object::createBinary(so_file_path); + if (!binary) { + return nullptr; + } + + llvm::object::ObjectFile *obj_file = + llvm::dyn_cast(binary.get().getBinary()); + if (!obj_file) { + return nullptr; + } + // Little-endian 32-bit - if (const ELF32LEObjectFile *ELFObj = dyn_cast(objfile)) { - return CreateELFSoFileParser(ELFObj); + if (auto elf_obj_file = + llvm::dyn_cast(obj_file)) { + return CreateELFSoFileParser(elf_obj_file); } // Big-endian 32-bit - if (const ELF32BEObjectFile *ELFObj = dyn_cast(objfile)) { - return CreateELFSoFileParser(ELFObj); + if (auto elf_obj_file = + llvm::dyn_cast(obj_file)) { + return CreateELFSoFileParser(elf_obj_file); } // Little-endian 64-bit - if (const ELF64LEObjectFile *ELFObj = dyn_cast(objfile)) { - return CreateELFSoFileParser(ELFObj); + if (auto elf_obj_file = + llvm::dyn_cast(obj_file)) { + return CreateELFSoFileParser(elf_obj_file); } // Big-endian 64-bit - if (const ELF64BEObjectFile *ELFObj = dyn_cast(objfile)) { - return CreateELFSoFileParser(ELFObj); + if (auto elf_obj_file = + llvm::dyn_cast(obj_file)) { + return CreateELFSoFileParser(elf_obj_file); } + return nullptr; }