fileformat/elf: Prevent creation of duplicate imports while parsing (#…

…1210)

* fileformat/elf: Prevent creation of duplicate imports while parsing
symbols

* fileformat/elf: use ptrs in import cache instead of refs

include/retdec/fileformat/types/import_table/import.h

@@ -39,7 +39,7 @@ class Import
 
 		/// @name Getters
 		/// @{
-		std::string getName() const;
+		const std::string& getName() const;
 		std::uint64_t getLibraryIndex() const;
 		std::uint64_t getAddress() const;
 		bool getOrdinalNumber(std::uint64_t &importOrdinalNumber) const;

include/retdec/fileformat/types/import_table/import_table.h

@@ -60,7 +60,7 @@ class ImportTable
 		virtual void computeHashes();
 		void clear();
 		void addLibrary(std::string name, bool missingDependency = false);
-		void addImport(std::unique_ptr<Import>&& import);
+		const Import* addImport(std::unique_ptr<Import>&& import);
 		bool hasLibraries() const;
 		bool hasLibrary(const std::string &name) const;
 		bool hasLibraryCaseInsensitive(const std::string &name) const;

src/fileformat/file_format/elf/elf_format.cpp

@@ -5,6 +5,7 @@
  */
 
 #include <elfio/elf_types.hpp>
+#include <functional>
 #include <map>
 #include <regex>
 
@@ -1744,10 +1745,30 @@ void ElfFormat::loadSymbols(const ELFIO::elfio *file, const ELFIO::symbol_sectio
 		telfhashSymbols = {};
 	}
 
+	// Cache keeping track of created imports (i.e. (name, address) pairs)
+	// to prevent repeated creation of the same imports.
+	// To save space, this uses string references and relies on imports not
+	// being moved -- they should not be, as ImportTable stores vector of unique
+	// pointers, but if that ever changes, this will end badly.
+    auto createdImportsComparator = [](
+        const std::pair<const std::string*, unsigned long long>& lhs,
+        const std::pair<const std::string*, unsigned long long>& rhs
+    ) {
+        if (*(lhs.first) != *(rhs.first)) {
+            return *(lhs.first) < *(rhs.first);
+        }
+        return lhs.second < rhs.second;
+    };
+    std::set<
+        std::pair<const std::string*, unsigned long long>,
+        decltype(createdImportsComparator)
+    > createdImports(createdImportsComparator);
+
 	for(std::size_t i = 0, e = elfSymbolTable->get_loaded_symbols_num(); i < e; ++i)
 	{
 		auto symbol = std::make_shared<ElfSymbol>();
 		elfSymbolTable->get_symbol(i, name, value, size, bind, type, link, other);
+
 		size ? symbol->setSize(size) : symbol->invalidateSize();
 		symbol->setType(getSymbolType(bind, type, link));
 		symbol->setUsageType(getSymbolUsageType(type));
@@ -1781,23 +1802,39 @@ void ElfFormat::loadSymbols(const ELFIO::elfio *file, const ELFIO::symbol_sectio
 					importTable = new ElfImportTable();
 				}
 				auto keyIter = importNameAddressMap.equal_range(name);
+
 				// we create std::set from std::multimap values in order to ensure determinism
-				std::set<std::pair<std::string, unsigned long long>> addresses(keyIter.first, keyIter.second);
+				std::set<std::pair<std::string, unsigned long long>> addresses;
+				for (auto it = keyIter.first; it != keyIter.second; ++it)
+				{
+					if (!createdImports.count({&it->first, it->second})) {
+						addresses.insert(*it);
+					}
+				}
+
 				for(const auto &address : addresses)
 				{
 					auto import = std::make_unique<Import>();
 					import->setName(name);
 					import->setAddress(address.second);
 					import->setUsageType(symbolToImportUsage(symbol->getUsageType()));
-					importTable->addImport(std::move(import));
+					auto* inserted = importTable->addImport(std::move(import));
+
+					if (inserted) {
+						createdImports.emplace(&inserted->getName(), inserted->getAddress());
+					}
 				}
 				if(keyIter.first == keyIter.second && getSectionFromAddress(value))
 				{
 					auto import = std::make_unique<Import>();
 					import->setName(name);
 					import->setAddress(value);
 					import->setUsageType(symbolToImportUsage(symbol->getUsageType()));
-					importTable->addImport(std::move(import));
+					auto* inserted = importTable->addImport(std::move(import));
+
+					if (inserted) {
+						createdImports.emplace(&inserted->getName(), inserted->getAddress());
+					}
 				}
 			}
 		}

src/fileformat/types/import_table/import.cpp

@@ -13,7 +13,7 @@ namespace fileformat {
  * Get import name
  * @return Import name
  */
-std::string Import::getName() const
+const std::string& Import::getName() const
 {
 	return name;
 }

src/fileformat/types/import_table/import_table.cpp

@@ -322,9 +322,10 @@ void ImportTable::addLibrary(std::string name, bool isMissingDependency)
  * Add import
  * @param import Import which will be added
  */
-void ImportTable::addImport(std::unique_ptr<Import>&& import)
+const Import* ImportTable::addImport(std::unique_ptr<Import>&& import)
 {
 	imports.push_back(std::move(import));
+	return imports.back().get();
 }
 
 /**