Introduce an RAII type for managing bulk symbol modifications

Fixes https://github.com/Vector35/binaryninja-api/issues/7666.

Correctly managing the state of bulk symbol modifications via
`BeginBulkModifySymbols` / `EndBulkModifySymbols` is error-prone in the
face of exceptions and early returns. Leaking a bulk symbol modification
can leave the view in a state where no further changes to symbols will
be applied.

All users of the C++ API are encouraged to move from
`BeginBulkModifySymbols` / `EndBulkModifySymbols` to the new
`BulkSymbolModification` class.

5 files changed, 10 insertions, 13 deletions

diff --git a/view/elf/elfview.cpp b/view/elf/elfview.cpp
index 0e206032..e473795f 100644
--- a/view/elf/elfview.cpp
+++ b/view/elf/elfview.cpp
@@ -1053,7 +1053,7 @@ bool ElfView::Init()
 		m_logger->LogError("ELF relocation table invalid");
 	}
 
-	BeginBulkModifySymbols();
+	BulkSymbolModification bulkSymbolModification(this);
 
 	vector<ElfSymbolTableEntry> auxSymbolTable;
 	try
@@ -1408,7 +1408,7 @@ bool ElfView::Init()
 	delete m_symbolQueue;
 	m_symbolQueue = nullptr;
 
-	EndBulkModifySymbols();
+	bulkSymbolModification.End();
 
 	auto relocHandler = m_arch->GetRelocationHandler("ELF");
 	if (relocHandler)
diff --git a/view/kernelcache/core/MachOProcessor.cpp b/view/kernelcache/core/MachOProcessor.cpp
index fb3d2b27..5731d80e 100644
--- a/view/kernelcache/core/MachOProcessor.cpp
+++ b/view/kernelcache/core/MachOProcessor.cpp
@@ -50,7 +50,7 @@ void KernelCacheMachOProcessor::ApplyHeader(const KernelCache& cache, KernelCach
 			m_view->AddFunctionForAnalysis(targetPlatform, func, false);
 	}
 
-	m_view->BeginBulkModifySymbols();
+	BulkSymbolModification bulkSymbolModification(m_view);
 
 	// Apply symbols from symbol table.
 	if (header.symtab.symoff != 0)
@@ -78,7 +78,6 @@ void KernelCacheMachOProcessor::ApplyHeader(const KernelCache& cache, KernelCach
 			ApplySymbol(m_view, typeLib, symbol, symbolType);
 		}
 	}
-	m_view->EndBulkModifySymbols();
 }
 
 uint64_t KernelCacheMachOProcessor::ApplyHeaderSections(KernelCacheMachOHeader& header)
diff --git a/view/macho/machoview.cpp b/view/macho/machoview.cpp
index 7353e995..1c8d8512 100644
--- a/view/macho/machoview.cpp
+++ b/view/macho/machoview.cpp
@@ -2088,7 +2088,7 @@ bool MachoView::InitializeHeader(MachOHeader& header, bool isMainHeader, uint64_
 			ParseFunctionStarts(GetDefaultPlatform(), header.textBase, header.functionStarts);
 	}
 
-	BeginBulkModifySymbols();
+	BulkSymbolModification bulkSymbolModification(this);
 	m_symbolQueue = new SymbolQueue();
 
 	try
@@ -2105,8 +2105,7 @@ bool MachoView::InitializeHeader(MachOHeader& header, bool isMainHeader, uint64_
 	m_symbolQueue->Process();
 	delete m_symbolQueue;
 	m_symbolQueue = nullptr;
-
-	EndBulkModifySymbols();
+	bulkSymbolModification.End();
 
 	for (auto& relocation : header.rebaseRelocations)
 	{
diff --git a/view/pe/peview.cpp b/view/pe/peview.cpp
index 8537a036..ffe9c91a 100644
--- a/view/pe/peview.cpp
+++ b/view/pe/peview.cpp
@@ -1307,7 +1307,8 @@ bool PEView::Init()
 	}
 
 	vector<pair<BNRelocationInfo, string>> relocs;
-	BeginBulkModifySymbols();
+	
+	BulkSymbolModification bulkSymbolModification(this);
 	m_symbolQueue = new SymbolQueue();
 	m_symExternMappingMetadata = new Metadata(KeyValueDataType);
 
@@ -2582,7 +2583,7 @@ bool PEView::Init()
 	delete m_symbolQueue;
 	m_symbolQueue = nullptr;
 
-	EndBulkModifySymbols();
+	bulkSymbolModification.End();
 
 	StoreMetadata("SymbolExternalLibraryMapping", m_symExternMappingMetadata, true);
 
diff --git a/view/sharedcache/core/MachOProcessor.cpp b/view/sharedcache/core/MachOProcessor.cpp
index 281f40c2..31fdbe6c 100644
--- a/view/sharedcache/core/MachOProcessor.cpp
+++ b/view/sharedcache/core/MachOProcessor.cpp
@@ -52,7 +52,7 @@ void SharedCacheMachOProcessor::ApplyHeader(const SharedCache& cache, SharedCach
 				m_view->AddFunctionForAnalysis(targetPlatform, func, false);
 		}
 
-		m_view->BeginBulkModifySymbols();
+		BulkSymbolModification bulkSymbolModification(m_view);
 
 		// Apply symbols from symbol table.
 		if (header.symtab.symoff != 0)
@@ -80,7 +80,6 @@ void SharedCacheMachOProcessor::ApplyHeader(const SharedCache& cache, SharedCach
 				ApplySymbol(m_view, typeLib, symbol, symbolType);
 			}
 		}
-		m_view->EndBulkModifySymbols();
 	}
 
 	// Apply symbols from the .symbols cache files.
@@ -127,14 +126,13 @@ void SharedCacheMachOProcessor::ApplyUnmappedLocalSymbols(const SharedCache& cac
 		uint64_t symbolTableStart = localSymbolsAddr + (localSymbolsEntry.nlistStartIndex * sizeof(nlist_64));
 		TableInfo symbolInfo = {symbolTableStart, localSymbolsEntry.nlistCount};
 		TableInfo stringInfo = {localStringsAddr, localSymbolsInfo.stringsSize};
-		m_view->BeginBulkModifySymbols();
+		BulkSymbolModification bulkSymbolModification(m_view);
 		const auto symbols = header.ReadSymbolTable(*localSymbolsVM, symbolInfo, stringInfo);
 		for (const auto &sym: symbols)
 		{
 			auto [symbol, symbolType] = sym.GetBNSymbolAndType(*m_view);
 			ApplySymbol(m_view, typeLib, std::move(symbol), std::move(symbolType));
 		}
-		m_view->EndBulkModifySymbols();
 		return;
 	}
 }