Merge "header-checker: Code cleanup"

vndk/tools/header-checker/header-abi-linker/src/header_abi_linker.cpp

@@ -106,9 +106,13 @@ class HeaderAbiLinker {
                 const abi_util::AbiElementMap<T> &src,
                 const std::function<bool(const std::string &)> &symbol_filter);
 
-  bool ParseVersionScriptFiles();
+  std::unique_ptr<abi_util::TextFormatToIRReader> 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<std::string> &dump_files_;
@@ -142,23 +145,6 @@ class HeaderAbiLinker {
   std::regex globvars_vs_regex_;
 };
 
-template <typename T>
-static bool AddElfSymbols(abi_util::IRDumper *dst,
-                          const std::map<std::string, T> &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<std::string> &dump_files,
     const std::set<std::string> *exported_headers,
@@ -167,6 +153,7 @@ static void DeDuplicateAbiElementsThread(
   std::unique_ptr<abi_util::TextFormatToIRReader> 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<std::mutex> lock(*greader_lock);
   greader->MergeGraphs(*local_reader);
 }
 
-bool HeaderAbiLinker::LinkAndDump() {
+std::unique_ptr<abi_util::TextFormatToIRReader>
-  // If the user specifies that a version script should be used, use that.
+HeaderAbiLinker::ReadInputDumpFiles() {
-  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<abi_util::IRDumper> 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<abi_util::TextFormatToIRReader> 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,
+      std::min(dump_files_.size() / kSourcesPerBatchThread, max_threads) : 0;
-                             max_threads) : 0;
   std::vector<std::thread> threads;
   std::atomic<std::size_t> 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<abi_util::IRDumper> 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 <typename T>
+static bool LinkExportedSymbols(abi_util::IRDumper *dst,
+                                const std::map<std::string, T> &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<std::string> function_regexs =
       version_script_parser.GetFunctionRegexs();
   std::set<std::string> globvar_regexs =
@@ -363,27 +401,13 @@ bool HeaderAbiLinker::ParseVersionScriptFiles() {
   return true;
 }
 
-bool HeaderAbiLinker::ParseSoFile() {
+bool HeaderAbiLinker::ReadExportedSymbolsFromSharedObjectFile() {
-  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<llvm::object::ObjectFile>(&(*Binary.get().getBinary()));
-  if (!objfile) {
-    llvm::errs() << "Not an object file\n";
-    return false;
-  }
-
   std::unique_ptr<abi_util::SoFileParser> so_parser =
-      abi_util::SoFileParser::Create(objfile);
+      abi_util::SoFileParser::Create(so_file_);
-  if (so_parser == nullptr) {
+  if (!so_parser) {
-    llvm::errs() << "Couldn't create soFile Parser\n";
     return false;
   }
-  so_parser->GetSymbols();
+
   function_decl_map_ = so_parser->GetFunctions();
   globvar_decl_map_ = so_parser->GetGlobVars();
   return true;

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 <typename Iterator>
-  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_));

vndk/tools/header-checker/header-abi-util/include/so_file_parser.h

@@ -17,9 +17,6 @@
 
 #include "ir_representation.h"
 
-#include <llvm/Object/ELFObjectFile.h>
-#include <llvm/Object/ELFTypes.h>
-
 #include <memory>
 #include <map>
 #include <string>
@@ -28,38 +25,13 @@ namespace abi_util {
 
 class SoFileParser {
  public:
+  static std::unique_ptr<SoFileParser> Create(const std::string &so_file_path);
+
   virtual ~SoFileParser() {}
 
-  static std::unique_ptr<SoFileParser> Create(
-      const llvm::object::ObjectFile *obj);
-
   virtual const std::map<std::string, ElfFunctionIR> &GetFunctions() const = 0;
+
   virtual const std::map<std::string, ElfObjectIR> &GetGlobVars() const = 0;
-  virtual void GetSymbols() = 0;
-};
-
-template<typename T>
-class ELFSoFileParser : public SoFileParser {
- private:
-  LLVM_ELF_IMPORT_TYPES_ELFT(T)
-  typedef llvm::object::ELFFile<T> ELFO;
-  typedef typename ELFO::Elf_Sym Elf_Sym;
-
- public:
-  ELFSoFileParser(const llvm::object::ELFObjectFile<T> *obj) : obj_(obj) {}
-  ~ELFSoFileParser() override {}
-
-  const std::map<std::string, ElfFunctionIR> &GetFunctions() const override;
-  const std::map<std::string, ElfObjectIR> &GetGlobVars() const override;
-  void GetSymbols() override;
-
- private:
-  const llvm::object::ELFObjectFile<T> *obj_;
-  std::map<std::string, abi_util::ElfFunctionIR> functions_;
-  std::map<std::string, abi_util::ElfObjectIR> globvars_;
-
- private:
-  bool IsSymbolExported(const Elf_Sym *elf_sym) const;
 };
 
 }  // namespace abi_util

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<llvm::sys::fs::basic_file_status> 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;
     }
 

vndk/tools/header-checker/header-abi-util/src/so_file_parser.cpp

@@ -21,65 +21,71 @@
 #include <llvm/Object/ELFTypes.h>
 #include <llvm/Object/SymbolSize.h>
 
-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 <typename T>
-static inline T UnWrap(llvm::Expected<T> ValueOrError) {
+static inline T UnWrap(llvm::Expected<T> value_or_error) {
-  if (!ValueOrError) {
+  if (!value_or_error) {
-    llvm::errs() << "\nError: " << llvm::toString(ValueOrError.takeError())
+    llvm::errs() << "\nerror: " << llvm::toString(value_or_error.takeError())
                  << ".\n";
     llvm::errs().flush();
     exit(1);
   }
-  return std::move(ValueOrError.get());
+  return std::move(value_or_error.get());
-}
-
-template<typename T>
-const std::map<std::string, abi_util::ElfFunctionIR> &
-ELFSoFileParser<T>::GetFunctions() const {
-  return functions_;
-}
-
-template<typename T>
-const std::map<std::string, abi_util::ElfObjectIR> &
-ELFSoFileParser<T>::GetGlobVars() const {
-  return globvars_;
-}
-
-template<typename T>
-bool ELFSoFileParser<T>::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);
 }
 
 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<typename T>
-void ELFSoFileParser<T>::GetSymbols() {
+class ELFSoFileParser : public SoFileParser {
-  assert(obj_ != nullptr);
+ private:
-  for (auto symbol_it : obj_->getDynamicSymbolIterators()) {
+  LLVM_ELF_IMPORT_TYPES_ELFT(T)
-    const Elf_Sym *elf_sym = obj_->getSymbol(symbol_it.getRawDataRefImpl());
+  typedef llvm::object::ELFFile<T> ELFO;
+  typedef typename ELFO::Elf_Sym Elf_Sym;
+
+ public:
+  ELFSoFileParser(const llvm::object::ELFObjectFile<T> *obj);
+
+  ~ELFSoFileParser() override {}
+
+  const std::map<std::string, ElfFunctionIR> &GetFunctions() const override {
+    return functions_;
+  }
+
+  const std::map<std::string, ElfObjectIR> &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<T> *obj_;
+  std::map<std::string, abi_util::ElfFunctionIR> functions_;
+  std::map<std::string, abi_util::ElfObjectIR> globvars_;
+};
+
+template<typename T>
+ELFSoFileParser<T>::ELFSoFileParser(const llvm::object::ELFObjectFile<T> *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<SoFileParser> CreateELFSoFileParser(
 }
 
 std::unique_ptr<SoFileParser> 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<llvm::object::ObjectFile>(binary.get().getBinary());
+  if (!obj_file) {
+    return nullptr;
+  }
+
   // Little-endian 32-bit
-  if (const ELF32LEObjectFile *ELFObj = dyn_cast<ELF32LEObjectFile>(objfile)) {
+  if (auto elf_obj_file =
-    return CreateELFSoFileParser(ELFObj);
+          llvm::dyn_cast<llvm::object::ELF32LEObjectFile>(obj_file)) {
+    return CreateELFSoFileParser(elf_obj_file);
   }
 
   // Big-endian 32-bit
-  if (const ELF32BEObjectFile *ELFObj = dyn_cast<ELF32BEObjectFile>(objfile)) {
+  if (auto elf_obj_file =
-    return CreateELFSoFileParser(ELFObj);
+          llvm::dyn_cast<llvm::object::ELF32BEObjectFile>(obj_file)) {
+    return CreateELFSoFileParser(elf_obj_file);
   }
 
   // Little-endian 64-bit
-  if (const ELF64LEObjectFile *ELFObj = dyn_cast<ELF64LEObjectFile>(objfile)) {
+  if (auto elf_obj_file =
-    return CreateELFSoFileParser(ELFObj);
+          llvm::dyn_cast<llvm::object::ELF64LEObjectFile>(obj_file)) {
+    return CreateELFSoFileParser(elf_obj_file);
   }
 
   // Big-endian 64-bit
-  if (const ELF64BEObjectFile *ELFObj = dyn_cast<ELF64BEObjectFile>(objfile)) {
+  if (auto elf_obj_file =
-    return CreateELFSoFileParser(ELFObj);
+          llvm::dyn_cast<llvm::object::ELF64BEObjectFile>(obj_file)) {
+    return CreateELFSoFileParser(elf_obj_file);
   }
+
   return nullptr;
 }