Move instruction bytes from BasicBlock into a transient object

Instruction bytes were previously stored on each basic block during
`AnalyzeBasicBlock` and consumed during `LiftFunction` before being
discarded. The `DataBuffer` member increased the size of every basic
block object, even after the instruction data was discarded.

The new approach uses a `LifterInstructionData` class that holds a map
from basic block start address to the corresponding instruction data. An
instance is created prior to `AnalyzeBasicBlock` and stored on the
function object. After the function is lifted, the instance is
discarded. `AnalyzeBasicBlock` and `LiftFunction` can access the
`LifterInstructionData` via their context objects in order to populate
and access its data respectively.

Author: bdash

architecture.cpp

@@ -204,9 +204,42 @@ vector<InstructionTextToken> InstructionTextToken::ConvertInstructionTextTokenLi
 	return result;
 }
 
+LifterInstructionData::LifterInstructionData(BNLifterInstructionData* instrData)
+{
+	m_object = instrData;
+}
+
+
+void LifterInstructionData::Append(BasicBlock* block, std::span<const uint8_t> data)
+{
+	BNLifterInstructionDataAppend(m_object, block->GetObject(), data.data(), data.size());
+}
+
+
+std::span<const uint8_t> LifterInstructionData::Get(BasicBlock* block, uint64_t addr)
+{
+	size_t len = 0;
+	const uint8_t* opcode = BNLifterInstructionDataGet(m_object, block->GetObject(), addr, &len);
+	if (!opcode)
+		return {};
+	return std::span<const uint8_t>(opcode, len);
+}
+
+
 BasicBlockAnalysisContext::BasicBlockAnalysisContext(BNBasicBlockAnalysisContext* context)
 {
 	m_context = context;
+	if (context->lifterInstructionData)
+	{
+		m_lifterInstructionData =
+			new LifterInstructionData(BNNewLifterInstructionDataReference(context->lifterInstructionData));
+	}
+}
+
+
+Ref<LifterInstructionData> BasicBlockAnalysisContext::GetLifterInstructionData()
+{
+	return m_lifterInstructionData;
 }
 
 const std::map<ArchAndAddr, std::set<ArchAndAddr>> BasicBlockAnalysisContext::GetIndirectBranches()
@@ -526,6 +559,11 @@ FunctionLifterContext::FunctionLifterContext(LowLevelILFunction* func, BNFunctio
 	}
 
 	m_functionArchContext = context->functionArchContext;
+	if (context->lifterInstructionData)
+	{
+		m_lifterInstructionData =
+			new LifterInstructionData(BNNewLifterInstructionDataReference(context->lifterInstructionData));
+	}
 	m_containsInlinedFunctions = context->containsInlinedFunctions;
 }
 

basicblock.cpp

@@ -454,24 +454,6 @@ void BasicBlock::SetUndeterminedOutgoingEdges(bool value)
 }
 
 
-bool BasicBlock::HasInstructionData() const
-{
-	return BNBasicBlockHasInstructionData(m_object);
-}
-
-
-const uint8_t* BasicBlock::GetInstructionData(uint64_t addr, size_t* len) const
-{
-	return BNBasicBlockGetInstructionData(m_object, addr, len);
-}
-
-
-void BasicBlock::AddInstructionData(const void* data, size_t len)
-{
-	BNBasicBlockAddInstructionData(m_object, data, len);
-}
-
-
 void BasicBlock::SetFallThroughToFunction(bool value)
 {
 	BNBasicBlockSetFallThroughToFunction(m_object, value);

binaryninjaapi.h

@@ -52,6 +52,7 @@
 #include <type_traits>
 #include <optional>
 #include <memory>
+#include <span>
 #include <any>
 #include <fmt/format.h>
 #include <fmt/ranges.h>
@@ -9647,6 +9648,19 @@ namespace BinaryNinja {
 
 	typedef size_t ExprId;
 
+	/*! Per-function store of basic block instruction bytes, populated during basic block analysis
+	    and read during lifting. Reached through BasicBlockAnalysisContext and FunctionLifterContext.
+	*/
+	class LifterInstructionData : public CoreRefCountObject<BNLifterInstructionData,
+		BNNewLifterInstructionDataReference, BNFreeLifterInstructionData>
+	{
+	public:
+		LifterInstructionData(BNLifterInstructionData* instrData);
+
+		void Append(BasicBlock* block, std::span<const uint8_t> data);
+		std::span<const uint8_t> Get(BasicBlock* block, uint64_t addr);
+	};
+
 	class BasicBlockAnalysisContext
 	{
 	private:
@@ -9663,6 +9677,8 @@ namespace BinaryNinja {
 		std::optional<std::set<ArchAndAddr>> m_haltedDisassemblyAddresses;
 		std::optional<std::map<ArchAndAddr, ArchAndAddr>> m_inlinedUnresolvedIndirectBranches;
 
+		Ref<LifterInstructionData> m_lifterInstructionData;
+
 	public:
 		BNBasicBlockAnalysisContext* m_context;
 
@@ -9691,6 +9707,8 @@ namespace BinaryNinja {
 		bool SetFunctionArchContextRaw(void* p);
 		void* GetFunctionArchContextRaw() const { return m_context->functionArchContext; }
 
+		Ref<LifterInstructionData> GetLifterInstructionData();
+
 		template <class ArchT>
 		bool SetFunctionArchContext(const ArchT* arch, typename ArchT::FunctionArchContext* context)
 		{
@@ -9726,6 +9744,7 @@ namespace BinaryNinja {
 		std::set<uint64_t> m_inlinedCalls;
 		bool* m_containsInlinedFunctions;
 		void* m_functionArchContext;
+		Ref<LifterInstructionData> m_lifterInstructionData;
 
 	public:
 		BNFunctionLifterContext* m_context;
@@ -9742,6 +9761,7 @@ namespace BinaryNinja {
 		std::set<uint64_t>& GetInlinedCalls();
 		void SetContainsInlinedFunctions(bool value);
 		void* GetFunctionArchContextRaw() const { return m_functionArchContext; }
+		Ref<LifterInstructionData>& GetLifterInstructionData() { return m_lifterInstructionData; }
 		template <class ArchT>
 		typename ArchT::FunctionArchContext* GetFunctionArchContext(const ArchT* arch)
 		{
@@ -12734,21 +12754,6 @@ namespace BinaryNinja {
 		*/
 		void SetUndeterminedOutgoingEdges(bool value);
 
-		/*! Get the instruction data for a specific address in this basic block
-
-			\param addr Address of the instruction
-			\param len Pointer to a size_t variable to store the length of the instruction data
-			\return Pointer to the instruction data
-		*/
-		const uint8_t* GetInstructionData(uint64_t addr, size_t* len) const;
-
-		/*! Add instruction data to the basic block
-
-			\param data Pointer to the instruction data
-			\param len Length of the instruction data
-		*/
-		void AddInstructionData(const void* data, size_t len);
-
 		/*! Set whether the basic blocks falls through to a function
 
 			\param value Whether the basic block falls through to a function
@@ -12773,12 +12778,6 @@ namespace BinaryNinja {
 		*/
 		void SetCanExit(bool value);
 
-		/*! Determine whether this basic block has instruction data
-
-			\return Whether this basic block has instruction data
-		*/
-		bool HasInstructionData() const;
-
 		/*! List of dominators for this basic block
 
 			\param post Whether to get post dominators (default: false)

binaryninjacore.h

@@ -37,14 +37,14 @@
 // Current ABI version for linking to the core. This is incremented any time
 // there are changes to the API that affect linking, including new functions,
 // new types, or modifications to existing functions or types.
-#define BN_CURRENT_CORE_ABI_VERSION 178
+#define BN_CURRENT_CORE_ABI_VERSION 179
 
 // Minimum ABI version that is supported for loading of plugins. Plugins that
 // are linked to an ABI version less than this will not be able to load and
 // will require rebuilding. The minimum version is increased when there are
 // incompatible changes that break binary compatibility, such as changes to
 // existing types or functions.
-#define BN_MINIMUM_CORE_ABI_VERSION 178
+#define BN_MINIMUM_CORE_ABI_VERSION 179
 
 #ifdef __GNUC__
 	#ifdef BINARYNINJACORE_LIBRARY
@@ -250,6 +250,7 @@ extern "C"
 	typedef struct BNArchitecture BNArchitecture;
 	typedef struct BNFunction BNFunction;
 	typedef struct BNBasicBlock BNBasicBlock;
+	typedef struct BNLifterInstructionData BNLifterInstructionData;
 	typedef struct BNDownloadProvider BNDownloadProvider;
 	typedef struct BNDownloadInstance BNDownloadInstance;
 	typedef struct BNWebsocketProvider BNWebsocketProvider;
@@ -2168,6 +2169,7 @@ extern "C"
 		BNArchitectureAndAddress* inlinedUnresolvedIndirectBranches;
 
 		void* functionArchContext;
+		BNLifterInstructionData* lifterInstructionData;
 	} BNBasicBlockAnalysisContext;
 
 	typedef struct BNFunctionLifterContext {
@@ -2195,6 +2197,7 @@ extern "C"
 		uint64_t* inlinedCalls;
 
 		void* functionArchContext;
+		BNLifterInstructionData* lifterInstructionData;
 
 		// OUT
 		bool* containsInlinedFunctions;
@@ -5414,6 +5417,14 @@ extern "C"
 
 	BINARYNINJACOREAPI BNBasicBlock* BNNewBasicBlockReference(BNBasicBlock* block);
 	BINARYNINJACOREAPI void BNFreeBasicBlock(BNBasicBlock* block);
+
+	BINARYNINJACOREAPI BNLifterInstructionData* BNNewLifterInstructionDataReference(
+		BNLifterInstructionData* instrData);
+	BINARYNINJACOREAPI void BNFreeLifterInstructionData(BNLifterInstructionData* instrData);
+	BINARYNINJACOREAPI void BNLifterInstructionDataAppend(
+		BNLifterInstructionData* instrData, BNBasicBlock* block, const void* data, size_t len);
+	BINARYNINJACOREAPI const uint8_t* BNLifterInstructionDataGet(
+		BNLifterInstructionData* instrData, BNBasicBlock* block, uint64_t addr, size_t* len);
 	BINARYNINJACOREAPI BNBasicBlock** BNGetFunctionBasicBlockList(BNFunction* func, size_t* count);
 	BINARYNINJACOREAPI void BNFreeBasicBlockList(BNBasicBlock** blocks, size_t count);
 	BINARYNINJACOREAPI BNBasicBlock* BNGetFunctionBasicBlockAtAddress(
@@ -5573,9 +5584,6 @@ extern "C"
 	BINARYNINJACOREAPI void BNFreePendingBasicBlockEdgeList(BNPendingBasicBlockEdge* edges);
 	BINARYNINJACOREAPI void BNClearBasicBlockPendingOutgoingEdges(BNBasicBlock* block);
 	BINARYNINJACOREAPI void BNBasicBlockSetUndeterminedOutgoingEdges(BNBasicBlock* block, bool value);
-	BINARYNINJACOREAPI const bool BNBasicBlockHasInstructionData(BNBasicBlock* block);
-	BINARYNINJACOREAPI const uint8_t* BNBasicBlockGetInstructionData(BNBasicBlock* block, uint64_t addr, size_t* len);
-	BINARYNINJACOREAPI void BNBasicBlockAddInstructionData(BNBasicBlock* block, const void* data, size_t len);
 	BINARYNINJACOREAPI void BNBasicBlockSetFallThroughToFunction(BNBasicBlock* block, bool value);
 	BINARYNINJACOREAPI bool BNBasicBlockIsFallThroughToFunction(BNBasicBlock* block);
 	BINARYNINJACOREAPI bool BNBasicBlockCanExit(BNBasicBlock* block);

defaultarch.cpp

@@ -101,6 +101,8 @@ void Architecture::DefaultAnalyzeBasicBlocks(Function* function, BasicBlockAnaly
 	auto& haltedDisassemblyAddresses = context.GetHaltedDisassemblyAddresses();
 	auto& inlinedUnresolvedIndirectBranches = context.GetInlinedUnresolvedIndirectBranches();
 
+	Ref<LifterInstructionData> instrData = context.GetLifterInstructionData();
+
 	bool hasInvalidInstructions = false;
 	set<ArchAndAddr> guidedSourceBlockTargets;
 	auto guidedSourceBlocks = function->GetGuidedSourceBlocks();
@@ -211,9 +213,13 @@ void Architecture::DefaultAnalyzeBasicBlocks(Function* function, BasicBlockAnaly
 					{
 						// Instruction is in the middle of a block, need to split the basic block into two
 						Ref<BasicBlock> splitBlock = context.CreateBasicBlock(location.arch, location.address);
-						size_t instrDataLen;
-						const uint8_t* instrData = targetBlock->GetInstructionData(location.address, &instrDataLen);
-						splitBlock->AddInstructionData(instrData, instrDataLen);
+						if (instrData)
+						{
+							// Copy before appending, as Append can invalidate the span returned by Get
+							std::span<const uint8_t> tail = instrData->Get(targetBlock, location.address);
+							std::vector<uint8_t> splitData(tail.begin(), tail.end());
+							instrData->Append(splitBlock, splitData);
+						}
 						splitBlock->SetFallThroughToFunction(targetBlock->IsFallThroughToFunction());
 						splitBlock->SetUndeterminedOutgoingEdges(targetBlock->HasUndeterminedOutgoingEdges());
 						splitBlock->SetCanExit(targetBlock->CanExit());
@@ -594,7 +600,8 @@ void Architecture::DefaultAnalyzeBasicBlocks(Function* function, BasicBlockAnaly
 			}
 
 			location.address += info.length;
-			block->AddInstructionData(opcode, info.length);
+			if (instrData)
+				instrData->Append(block, std::span<const uint8_t>(opcode, info.length));
 
 			if (endsBlock && !info.delaySlots)
 				break;
@@ -780,12 +787,13 @@ static void ApplyExternPointerForRelocation(
 
 bool Architecture::DefaultLiftFunction(LowLevelILFunction* function, FunctionLifterContext& context)
 {
-	std::unique_ptr<FastBasicBlockMap<DataBuffer>> instrData;
 	Ref<BinaryView> data = context.GetView();
 	Ref<Logger> logger = context.GetLogger();
 	Ref<Platform> platform = context.GetPlatform();
 	std::set<ArchAndAddr> noReturnCalls = context.GetNoReturnCalls();
 	std::vector<Ref<BasicBlock>> blocks = context.GetBasicBlocks();
+	Ref<LifterInstructionData> lifterInstructionData = context.GetLifterInstructionData();
+	FastBasicBlockMap<DataBuffer> instrData(blocks);
 	std::map<ArchAndAddr, bool> contextualReturns = context.GetContextualReturns();
 	std::map<ArchAndAddr, ArchAndAddr> inlinedRemapping = context.GetInlinedRemapping();
 	std::optional<pair<ArchAndAddr, ArchAndAddr>> indirectSource;
@@ -835,26 +843,20 @@ bool Architecture::DefaultLiftFunction(LowLevelILFunction* function, FunctionLif
 			}
 
 			size_t len = 0;
-			const uint8_t* opcode;
-
-			if (i->HasInstructionData())
+			const uint8_t* opcode = nullptr;
+			if (lifterInstructionData)
 			{
-				opcode = i->GetInstructionData(addr, &len);
-
-				if (len == 0)
-				{
-					// Instruction data not found, emit undefined IL instruction
-					function->AddInstruction(function->AddExpr(LLIL_UNDEF, 0, 0));
-					logger->LogDebug("Instruction data not found, inserted LLIL_UNDEF at %#" PRIx64, addr);
-					break;
-				}
+				std::span<const uint8_t> bytes = lifterInstructionData->Get(i, addr);
+				opcode = bytes.data();
+				len = bytes.size();
 			}
-			else
-			{
-				if (!instrData)
-					instrData = std::make_unique<FastBasicBlockMap<DataBuffer>>(blocks);
 
-				DataBuffer& buffer = (*instrData)[i];
+			if (!opcode)
+			{
+				// The instruction data has no bytes for this block (a function loaded from the
+				// database, a block split after analysis, or an architecture that does not populate
+				// it). Read the block from the view instead.
+				DataBuffer& buffer = instrData[i];
 				if (buffer.GetLength() == 0)
 					buffer = data->ReadBuffer(i->GetStart(), i->GetEnd() - i->GetStart());
 

python/architecture.py

@@ -68,6 +68,34 @@
 IntrinsicType = Union[IntrinsicName, 'lowlevelil.ILIntrinsic', IntrinsicIndex]
 
 
+class LifterInstructionData:
+	"""Per-function store of basic block instruction bytes, populated during basic block analysis and
+	read during lifting.
+
+    .. note:: This class is meant to be used by Architecture plugins only
+    """
+
+	def __init__(self, handle: core.BNLifterInstructionDataHandle):
+		self.handle = handle
+
+	def __del__(self):
+		if core is not None:
+			core.BNFreeLifterInstructionData(self.handle)
+
+	def append(self, block: "basicblock.BasicBlock", data: bytes) -> None:
+		"""Append decoded bytes for a block. Call during basic block analysis only."""
+		core.BNLifterInstructionDataAppend(self.handle, block.handle, data, len(data))
+
+	def get(self, block: "basicblock.BasicBlock", addr: int) -> bytes:
+		"""Returns the bytes from ``addr`` to the end of its block, or ``b''`` when the block has no
+		stored data. Read-only, call during lifting."""
+		size = ctypes.c_ulonglong(0)
+		ptr = core.BNLifterInstructionDataGet(self.handle, block.handle, addr, ctypes.byref(size))
+		if not ptr:
+			return b''
+		return ctypes.string_at(ptr, size.value)
+
+
 @dataclass
 class BasicBlockAnalysisContext:
 	"""Used by ``analyze_basic_blocks`` and contains analysis settings and other contextual information.
@@ -217,6 +245,16 @@ def max_function_size(self) -> int:
 
 		return self._max_function_size
 
+	@property
+	def lifter_instruction_data(self) -> Optional["LifterInstructionData"]:
+		"""The per-function instruction byte store. Populate it during basic block analysis so that
+		lifting can read instruction bytes without touching the view from the multi-threaded stage."""
+
+		handle = self._handle.lifterInstructionData
+		if not handle:
+			return None
+		return LifterInstructionData(core.BNNewLifterInstructionDataReference(handle))
+
 	@property
 	def halt_on_invalid_instruction(self) -> bool:
 		"""Get the setting from context that determines if analysis should halt on invalid instructions."""
@@ -533,6 +571,15 @@ def function_arch_context(self) -> Any:
 
 		return self._function.arch.function_arch_contexts.get(self._function_arch_context_token, None)
 
+	@property
+	def lifter_instruction_data(self) -> Optional["LifterInstructionData"]:
+		"""The per-function instruction byte store populated during basic block analysis."""
+
+		handle = self._handle.lifterInstructionData
+		if not handle:
+			return None
+		return LifterInstructionData(core.BNNewLifterInstructionDataReference(handle))
+
 @dataclass(frozen=True)
 class RegisterInfo:
 	full_width_reg: RegisterName