Crashlog: Re-use BFD instances for the same object

src/crashlog.cpp

@@ -1236,10 +1236,10 @@ void CrashLog::MakeCrashSavegameAndScreenshot()
 sym_info_bfd::sym_info_bfd(bfd_vma addr_) : addr(addr_), abfd(nullptr), syms(nullptr), sym_count(0),
 		file_name(nullptr), function_name(nullptr), function_addr(0), line(0), found(false) {}
 
-sym_info_bfd::~sym_info_bfd()
+sym_bfd_obj::~sym_bfd_obj()
 {
-	free(syms);
-	if (abfd != nullptr) bfd_close(abfd);
+	free(this->syms);
+	if (this->abfd != nullptr) bfd_close(this->abfd);
 }
 
 static void find_address_in_section(bfd *abfd, asection *section, void *data)
@@ -1282,20 +1282,33 @@ static void find_address_in_section(bfd *abfd, asection *section, void *data)
 	}
 }
 
-void lookup_addr_bfd(const char *obj_file_name, sym_info_bfd &info)
+void lookup_addr_bfd(const char *obj_file_name, sym_bfd_obj_cache &bfdc, sym_info_bfd &info)
 {
-	info.abfd = bfd_openr(obj_file_name, nullptr);
+	auto res = bfdc.cache.try_emplace(obj_file_name);
+	sym_bfd_obj &obj = res.first->second;
+	if (res.second) {
+		/* New sym_bfd_obj */
+		obj.abfd = bfd_openr(obj_file_name, nullptr);
 
-	if (info.abfd == nullptr) return;
+		if (obj.abfd == nullptr) return;
 
-	if (!bfd_check_format(info.abfd, bfd_object) || (bfd_get_file_flags(info.abfd) & HAS_SYMS) == 0) return;
+		if (!bfd_check_format(obj.abfd, bfd_object) || (bfd_get_file_flags(obj.abfd) & HAS_SYMS) == 0) return;
 
-	unsigned int size;
-	info.sym_count = bfd_read_minisymbols(info.abfd, false, (void**) &(info.syms), &size);
-	if (info.sym_count <= 0) {
-		info.sym_count = bfd_read_minisymbols(info.abfd, true, (void**) &(info.syms), &size);
+		unsigned int size;
+		obj.sym_count = bfd_read_minisymbols(obj.abfd, false, (void**) &(obj.syms), &size);
+		if (obj.sym_count <= 0) {
+			obj.sym_count = bfd_read_minisymbols(obj.abfd, true, (void**) &(obj.syms), &size);
+		}
+		if (obj.sym_count <= 0) return;
+
+		obj.usable = true;
 	}
-	if (info.sym_count <= 0) return;
+
+	if (!obj.usable) return;
+
+	info.abfd = obj.abfd;
+	info.syms = obj.syms;
+	info.sym_count = obj.sym_count;
 
 	bfd_map_over_sections(info.abfd, find_address_in_section, &info);
 }

src/crashlog_bfd.h

@@ -29,9 +29,23 @@
 #undef PACKAGE_VERSION
 #endif
 
+#include <map>
+
 #if defined(WITH_BFD)
-struct sym_info_bfd;
-void lookup_addr_bfd(const char *obj_file_name, sym_info_bfd &info);
+
+struct sym_bfd_obj {
+	bfd *abfd = nullptr;
+	asymbol **syms = nullptr;
+	const char *file_name = nullptr;
+	long sym_count = 0;
+	bool usable = false;
+
+	~sym_bfd_obj();
+};
+
+struct sym_bfd_obj_cache {
+	std::map<std::string, sym_bfd_obj> cache;
+};
 
 struct sym_info_bfd {
 	bfd_vma addr;
@@ -45,9 +59,10 @@ struct sym_info_bfd {
 	bool found;
 
 	sym_info_bfd(bfd_vma addr_);
-	~sym_info_bfd();
 };
 
+void lookup_addr_bfd(const char *obj_file_name, sym_bfd_obj_cache &bfdc, sym_info_bfd &info);
+
 #endif
 
 #endif /* CRASHLOG_BFD_H */

src/os/unix/crashlog_unix.cpp

@@ -521,6 +521,7 @@ class CrashLogUnix : public CrashLog {
 		char **messages = backtrace_symbols(trace, trace_size);
 
 #if defined(WITH_BFD)
+		sym_bfd_obj_cache bfd_cache;
 		bfd_init();
 #endif /* WITH_BFD */
 
@@ -574,7 +575,7 @@ class CrashLogUnix : public CrashLog {
 			/* subtract one to get the line before the return address, i.e. the function call line */
 			sym_info_bfd bfd_info(reinterpret_cast<bfd_vma>(trace[i]) - reinterpret_cast<bfd_vma>(info.dli_fbase) - 1);
 			if (dladdr_result && info.dli_fname) {
-				lookup_addr_bfd(info.dli_fname, bfd_info);
+				lookup_addr_bfd(info.dli_fname, bfd_cache, bfd_info);
 				if (bfd_info.file_name != nullptr) file_name = bfd_info.file_name;
 				if (bfd_info.function_name != nullptr) func_name = bfd_info.function_name;
 				if (bfd_info.function_addr != 0) func_addr = reinterpret_cast<void *>(bfd_info.function_addr + reinterpret_cast<bfd_vma>(info.dli_fbase));

src/os/windows/crashlog_win.cpp

@@ -399,6 +399,11 @@ static const uint MAX_FRAMES     = 64;
 
 		std::array<DWORD64, 8> last_offsets = {};
 
+#if defined(WITH_BFD)
+		sym_bfd_obj_cache bfd_cache;
+		bfd_init();
+#endif /* WITH_BFD */
+
 		/* Walk stack at most MAX_FRAMES deep in case the stack is corrupt. */
 		for (uint num = 0; num < MAX_FRAMES; num++) {
 			auto guard = scope_guard([&]() {
@@ -448,7 +453,7 @@ static const uint MAX_FRAMES     = 64;
 #if defined (WITH_BFD)
 				/* subtract one to get the line before the return address, i.e. the function call line */
 				sym_info_bfd bfd_info(static_cast<bfd_vma>(frame.AddrPC.Offset) - 1);
-				lookup_addr_bfd(image_name, bfd_info);
+				lookup_addr_bfd(image_name, bfd_cache, bfd_info);
 				if (bfd_info.function_name != nullptr) {
 					const char *func_name = bfd_info.function_name;
 #if defined(WITH_DEMANGLE)