From 32cd2f45ed6d8d5319f10437762d65294988dfb5 Mon Sep 17 00:00:00 2001 From: Brandon Miller Date: Tue, 13 Oct 2026 09:25:05 -0400 Subject: [PATCH 1/8] Add support for Rust architecture hook plugins Introduce ArchitectureHook and register_architecture_hook with default forwarding to the previous implementation. Preserve address normalization and architecture-specific flag IL and intrinsic classes when delegating. --- rust/src/architecture.rs | 117 +++++++++++++---- rust/src/architecture/hook.rs | 219 ++++++++++++++++++++++++++++++++ rust/tests/architecture_hook.rs | 73 +++++++++++ 3 files changed, 385 insertions(+), 24 deletions(-) create mode 100644 rust/src/architecture/hook.rs create mode 100644 rust/tests/architecture_hook.rs diff --git a/rust/src/architecture.rs b/rust/src/architecture.rs index c8e6b7f103..5fb70e3748 100644 --- a/rust/src/architecture.rs +++ b/rust/src/architecture.rs @@ -16,7 +16,8 @@ //! analysis and decompilation. //! //! For more information see the [`Architecture`] trait and the [`CoreArchitecture`] structure for -//! querying already registered architectures. +//! querying already registered architectures. To override selected behavior of an existing +//! architecture, implement [`ArchitectureHook`] and call [`register_architecture_hook`]. // RegisterInfo purge use binaryninjacore_sys::*; @@ -52,7 +53,7 @@ use crate::relocation::{CustomRelocationHandlerHandle, RelocationHandler}; use crate::basic_block::BasicBlock; use crate::confidence::Conf; use crate::logger::Logger; -use crate::low_level_il::expression::ValueExpr; +use crate::low_level_il::expression::{LowLevelExpressionIndex, ValueExpr}; use crate::low_level_il::lifting::{ get_default_flag_cond_llil, get_default_flag_write_llil, LowLevelILFlagWriteOp, }; @@ -61,6 +62,7 @@ use crate::low_level_il::{LowLevelILMutableExpression, LowLevelILMutableFunction pub mod basic_block; pub mod branches; pub mod flag; +pub mod hook; pub mod instruction; pub mod intrinsic; pub mod register; @@ -71,6 +73,7 @@ pub use basic_block::*; pub use binaryninjacore_sys::BNLinearSweepAnalysisCapability as LinearSweepAnalysisCapability; pub use branches::*; pub use flag::*; +pub use hook::{register_architecture_hook, ArchitectureHook}; pub use instruction::*; pub use intrinsic::*; pub use register::*; @@ -811,6 +814,13 @@ impl CoreArchitecture { CoreArchitecture { handle } } + /// Selects the architecture for `addr` and applies its address normalization in place. + /// For example, ARM uses the low address bit to select Thumb mode, then clears that bit. + pub fn associated_arch_by_addr_mut(&self, addr: &mut u64) -> CoreArchitecture { + let handle = unsafe { BNGetAssociatedArchitectureByAddress(self.handle, addr) }; + CoreArchitecture { handle } + } + pub fn list_all() -> CoreArchitectureList { let mut count: usize = 0; let archs = unsafe { BNGetArchitectureList(&mut count) }; @@ -893,9 +903,8 @@ impl Architecture for CoreArchitecture { unsafe { BNGetArchitectureOpcodeDisplayLength(self.handle) } } - fn associated_arch_by_addr(&self, addr: u64) -> CoreArchitecture { - let handle = unsafe { BNGetAssociatedArchitectureByAddress(self.handle, addr as *mut _) }; - CoreArchitecture { handle } + fn associated_arch_by_addr(&self, mut addr: u64) -> CoreArchitecture { + self.associated_arch_by_addr_mut(&mut addr) } fn instruction_info(&self, data: &[u8], addr: u64) -> Option { @@ -1020,12 +1029,29 @@ impl Architecture for CoreArchitecture { fn flag_write_llil<'a>( &self, - _flag: Self::Flag, - _flag_write: Self::FlagWrite, - _op: LowLevelILFlagWriteOp, - _il: &'a LowLevelILMutableFunction, + flag: Self::Flag, + flag_write: Self::FlagWrite, + op: LowLevelILFlagWriteOp, + il: &'a LowLevelILMutableFunction, ) -> Option> { - None + let (size, operation) = op.size_and_op(); + let (count, mut operands) = op.raw_operands(); + let index = unsafe { + BNGetArchitectureFlagWriteLowLevelIL( + self.handle, + operation, + size, + flag_write.id().0, + flag.id().0, + operands.as_mut_ptr(), + count, + il.handle, + ) + }; + Some(LowLevelILMutableExpression::new( + il, + LowLevelExpressionIndex(index), + )) } fn flags_required_for_flag_condition( @@ -1058,19 +1084,36 @@ impl Architecture for CoreArchitecture { fn flag_cond_llil<'a>( &self, - _cond: FlagCondition, - _class: Option, - _il: &'a LowLevelILMutableFunction, + cond: FlagCondition, + class: Option, + il: &'a LowLevelILMutableFunction, ) -> Option> { - None + let index = unsafe { + BNGetArchitectureFlagConditionLowLevelIL( + self.handle, + cond, + class.map_or(0, |class| class.id().0), + il.handle, + ) + }; + Some(LowLevelILMutableExpression::new( + il, + LowLevelExpressionIndex(index), + )) } fn flag_group_llil<'a>( &self, - _group: Self::FlagGroup, - _il: &'a LowLevelILMutableFunction, + group: Self::FlagGroup, + il: &'a LowLevelILMutableFunction, ) -> Option> { - None + let index = unsafe { + BNGetArchitectureSemanticFlagGroupLowLevelIL(self.handle, group.id().0, il.handle) + }; + Some(LowLevelILMutableExpression::new( + il, + LowLevelExpressionIndex(index), + )) } fn registers_all(&self) -> Vec { @@ -1281,6 +1324,10 @@ impl Architecture for CoreArchitecture { } } + fn intrinsic_class(&self, id: IntrinsicId) -> BNIntrinsicClass { + unsafe { BNGetArchitectureIntrinsicClass(self.handle, id.0) } + } + fn intrinsic_from_id(&self, id: IntrinsicId) -> Option { CoreIntrinsic::new(*self, id) } @@ -1553,10 +1600,19 @@ where A: 'static + Architecture> + Send + Sync + Sized, F: FnOnce(CustomArchitectureHandle, CoreArchitecture) -> A, { - register_architecture_impl(name, func, |_| {}) + register_architecture_impl(ArchitectureRegistration::New(name), func, |_| {}) +} + +enum ArchitectureRegistration<'a> { + New(&'a str), + Hook(CoreArchitecture), } -fn register_architecture_impl(name: &str, func: F, customize: C) -> &'static A +fn register_architecture_impl( + registration: ArchitectureRegistration<'_>, + func: F, + customize: C, +) -> &'static A where A: 'static + Architecture> + Send + Sync + Sized, F: FnOnce(CustomArchitectureHandle, CoreArchitecture) -> A, @@ -2726,8 +2782,6 @@ where custom_arch.skip_and_return_value(data, addr, val) } - let name = name.to_cstr(); - let uninit_arch = ArchitectureBuilder { arch: MaybeUninit::zeroed(), func: Some(func), @@ -2825,8 +2879,23 @@ where customize(&mut custom_arch); unsafe { - let res = BNRegisterArchitecture(name.as_ptr(), &mut custom_arch as *mut _); - assert!(!res.is_null()); + match registration { + ArchitectureRegistration::Hook(base) => { + // Hook registration swaps the callbacks and returns the previous implementation. + // Unlike BNRegisterArchitecture, it does not invoke the init callback. + let previous = BNRegisterArchitectureHook(base.handle, &mut custom_arch); + assert!(!previous.is_null()); + cb_init::(raw.cast(), previous); + // Finalization queries our callbacks to rebuild the register/flag caches, so the + // Rust implementation must be fully initialized before this call. + BNFinalizeArchitectureHook(base.handle); + } + ArchitectureRegistration::New(name) => { + let name = name.to_cstr(); + let res = BNRegisterArchitecture(name.as_ptr(), &mut custom_arch); + assert!(!res.is_null()); + } + } (*raw).arch.assume_init_mut() } @@ -2901,7 +2970,7 @@ where true } - register_architecture_impl(name, func, |custom_arch| { + register_architecture_impl(ArchitectureRegistration::New(name), func, |custom_arch| { custom_arch.freeFunctionArchContext = Some(cb_free_function_arch_context_typed::); custom_arch.getInstructionTextWithContext = Some(cb_get_instruction_text_with_context_typed::); diff --git a/rust/src/architecture/hook.rs b/rust/src/architecture/hook.rs new file mode 100644 index 0000000000..cd38cefd56 --- /dev/null +++ b/rust/src/architecture/hook.rs @@ -0,0 +1,219 @@ +// Copyright 2021-2026 Vector 35 Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Hooks that override selected operations of an existing architecture. + +use super::*; + +// Keep the hook defaults and the Architecture adapter in sync using one signature list. +macro_rules! architecture_hook { + ($(fn $name:ident $(<$lt:lifetime>)? ($($arg:ident: $ty:ty),* $(,)?) -> $ret:ty;)*) => { + /// Overrides selected operations of an already registered architecture. + /// + /// Every method defaults to forwarding to `base`, the previous implementation in the + /// hook chain. Override only the methods you need. Use the supplied `base` to delegate: + /// looking up the hooked architecture by name inside a callback would call the hook again. + /// + /// The methods have the same semantics as their counterparts in [`Architecture`]. + /// Contextual disassembly forwards to the previous contextual implementation; override + /// [`ArchitectureHook::instruction_text_with_context`] to change contextual text. + /// + /// Register implementations with [`register_architecture_hook`]. + pub trait ArchitectureHook: Send + Sync + 'static { + /// Selects an architecture and updates the address, preserving normalization such + /// as clearing the ARM Thumb bit. Unlike [`Architecture::associated_arch_by_addr`], + /// this callback can update the caller's address. + fn associated_arch_by_addr(&self, base: &CoreArchitecture, addr: &mut u64) -> CoreArchitecture { + base.associated_arch_by_addr_mut(addr) + } + + $( + #[doc = concat!("See [`Architecture::", stringify!($name), "`].")] + fn $name $(<$lt>)? (&self, base: &CoreArchitecture, $($arg: $ty),*) -> $ret { + base.$name($($arg),*) + } + )* + } + + impl Architecture for HookArchitecture { + type Handle = CustomArchitectureHandle; + type RegisterInfo = CoreRegisterInfo; + type Register = CoreRegister; + type RegisterStackInfo = CoreRegisterStackInfo; + type RegisterStack = CoreRegisterStack; + type Flag = CoreFlag; + type FlagWrite = CoreFlagWrite; + type FlagClass = CoreFlagClass; + type FlagGroup = CoreFlagGroup; + type Intrinsic = CoreIntrinsic; + + $( + fn $name $(<$lt>)? (&self, $($arg: $ty),*) -> $ret { + self.hook.$name(&self.base, $($arg),*) + } + )* + + fn associated_arch_by_addr(&self, mut addr: u64) -> CoreArchitecture { + self.hook.associated_arch_by_addr(&self.base, &mut addr) + } + + fn handle(&self) -> Self::Handle { + self.handle + } + } + }; +} + +architecture_hook! { + fn endianness() -> Endianness; + fn address_size() -> usize; + fn default_integer_size() -> usize; + fn instruction_alignment() -> usize; + fn linear_sweep_initial_alignment() -> usize; + fn linear_sweep_analysis_capabilities() -> u32; + fn max_instr_len() -> usize; + fn opcode_display_len() -> usize; + fn instruction_info(data: &[u8], addr: u64) -> Option; + fn instruction_text(data: &[u8], addr: u64) -> Option<(usize, Vec)>; + fn instruction_text_with_context( + data: &[u8], + addr: u64, + context: Option>, + ) -> Option<(usize, Vec)>; + fn instruction_llil( + data: &[u8], + addr: u64, + il: &LowLevelILMutableFunction, + ) -> Option<(usize, bool)>; + fn analyze_basic_blocks(function: &mut Function, context: &mut BasicBlockAnalysisContext) -> (); + fn lift_function( + function: LowLevelILMutableFunction, + context: &mut FunctionLifterContext, + ) -> bool; + fn flag_write_llil<'a>( + flag: CoreFlag, + flag_write_type: CoreFlagWrite, + op: LowLevelILFlagWriteOp, + il: &'a LowLevelILMutableFunction, + ) -> Option>; + fn flags_required_for_flag_condition( + condition: FlagCondition, + class: Option, + ) -> Vec; + fn flag_cond_llil<'a>( + cond: FlagCondition, + class: Option, + il: &'a LowLevelILMutableFunction, + ) -> Option>; + fn flag_group_llil<'a>( + group: CoreFlagGroup, + il: &'a LowLevelILMutableFunction, + ) -> Option>; + fn registers_all() -> Vec; + fn register_from_id(id: RegisterId) -> Option; + fn registers_full_width() -> Vec; + fn registers_global() -> Vec; + fn registers_system() -> Vec; + fn stack_pointer_reg() -> Option; + fn link_reg() -> Option; + fn register_stacks() -> Vec; + fn register_stack_from_id(id: RegisterStackId) -> Option; + fn flags() -> Vec; + fn flag_from_id(id: FlagId) -> Option; + fn flag_write_types() -> Vec; + fn flag_write_from_id(id: FlagWriteId) -> Option; + fn flag_classes() -> Vec; + fn flag_class_from_id(id: FlagClassId) -> Option; + fn flag_groups() -> Vec; + fn flag_group_from_id(id: FlagGroupId) -> Option; + fn intrinsics() -> Vec; + fn intrinsic_class(id: IntrinsicId) -> BNIntrinsicClass; + fn intrinsic_from_id(id: IntrinsicId) -> Option; + fn can_assemble() -> bool; + fn assemble(code: &str, addr: u64) -> Result, String>; + fn is_never_branch_patch_available(data: &[u8], addr: u64) -> bool; + fn is_always_branch_patch_available(data: &[u8], addr: u64) -> bool; + fn is_invert_branch_patch_available(data: &[u8], addr: u64) -> bool; + fn is_skip_and_return_zero_patch_available(data: &[u8], addr: u64) -> bool; + fn is_skip_and_return_value_patch_available(data: &[u8], addr: u64) -> bool; + fn convert_to_nop(data: &mut [u8], addr: u64) -> bool; + fn always_branch(data: &mut [u8], addr: u64) -> bool; + fn invert_branch(data: &mut [u8], addr: u64) -> bool; + fn skip_and_return_value(data: &mut [u8], addr: u64, value: u64) -> bool; +} + +struct HookArchitecture { + hook: H, + base: CoreArchitecture, + handle: CustomArchitectureHandle, +} + +impl AsRef for HookArchitecture { + fn as_ref(&self) -> &CoreArchitecture { + &self.base + } +} + +/// Installs a hook on `base`, affecting all existing users of that architecture. +/// +/// Hooks run in reverse registration order. Each callback receives the previous implementation, +/// which can be called to delegate without recursion. The hook is retained for the lifetime of +/// the core and cannot be unregistered. The returned reference can be used to access its state. +/// +/// Like [`register_architecture`], call this during `CorePluginInit`, after the base architecture +/// is registered and before analysis starts. +/// +/// ```no_run +/// use binaryninja::architecture::{ +/// register_architecture_hook, Architecture, ArchitectureHook, CoreArchitecture, +/// }; +/// use binaryninja::disassembly::{InstructionTextToken, InstructionTextTokenKind}; +/// +/// struct Annotate; +/// impl ArchitectureHook for Annotate { +/// fn instruction_text( +/// &self, base: &CoreArchitecture, data: &[u8], addr: u64, +/// ) -> Option<(usize, Vec)> { +/// let (length, mut tokens) = base.instruction_text(data, addr)?; +/// tokens.push(InstructionTextToken::new(" ; hooked", InstructionTextTokenKind::Annotation)); +/// Some((length, tokens)) +/// } +/// } +/// register_architecture_hook(CoreArchitecture::by_name("x86_64").unwrap(), Annotate); +/// ``` +pub fn register_architecture_hook( + base: CoreArchitecture, + hook: H, +) -> &'static H { + unsafe extern "C" fn associated_arch( + context: *mut c_void, + addr: *mut u64, + ) -> *mut BNArchitecture { + let arch = unsafe { &*context.cast::>() }; + arch.hook + .associated_arch_by_addr(&arch.base, unsafe { &mut *addr }) + .handle + } + + let registered = super::register_architecture_impl( + ArchitectureRegistration::Hook(base), + |handle, base| HookArchitecture { hook, base, handle }, + |callbacks| { + // The Architecture trait takes an address by value; hooks must also preserve any + // address normalization performed by the previous architecture in the chain. + callbacks.getAssociatedArchitectureByAddress = Some(associated_arch::); + }, + ); + ®istered.hook +} diff --git a/rust/tests/architecture_hook.rs b/rust/tests/architecture_hook.rs new file mode 100644 index 0000000000..305fe76688 --- /dev/null +++ b/rust/tests/architecture_hook.rs @@ -0,0 +1,73 @@ +//! Hook registration is permanent, so these checks run in a separate integration-test process. +use binaryninja::architecture::{ + register_architecture_hook, Architecture, ArchitectureHook, CoreArchitecture, +}; +use binaryninja::disassembly::{InstructionTextToken, InstructionTextTokenKind}; +use binaryninja::headless::Session; +use std::ffi::c_void; +use std::ptr::NonNull; + +struct TextHook; + +impl ArchitectureHook for TextHook { + fn instruction_text( + &self, + _base: &CoreArchitecture, + _data: &[u8], + _addr: u64, + ) -> Option<(usize, Vec)> { + Some(( + 1, + vec![InstructionTextToken::new( + "hooked", + InstructionTextTokenKind::Text, + )], + )) + } + + fn instruction_text_with_context( + &self, + base: &CoreArchitecture, + data: &[u8], + addr: u64, + _context: Option>, + ) -> Option<(usize, Vec)> { + self.instruction_text(base, data, addr) + } +} + +struct PassThroughHook; +impl ArchitectureHook for PassThroughHook {} + +#[test] +fn test_architecture_hook() { + let _session = Session::new().expect("Failed to initialize session"); + let arch = CoreArchitecture::by_name("x86_64").expect("Failed to get architecture"); + let data = [0x90]; // nop + let address = 0x1000; + let original = arch.instruction_text(&data, address).unwrap(); + let expected = Some(( + 1, + vec![InstructionTextToken::new( + "hooked", + InstructionTextTokenKind::Text, + )], + )); + assert_ne!(Some(original), expected); + + // Match the Python hook regressions: replace text, then stack empty hooks. + register_architecture_hook(arch, TextHook); + assert_eq!(arch.instruction_text(&data, address), expected); + assert_eq!( + arch.instruction_text_with_context(&data, address, None), + expected + ); + + register_architecture_hook(arch, PassThroughHook); + register_architecture_hook(arch, PassThroughHook); + assert_eq!(arch.instruction_text(&data, address), expected); + assert_eq!( + arch.instruction_text_with_context(&data, address, None), + expected + ); +} From 18b78f77126030152bd6c187767c49a3ee4f5a8d Mon Sep 17 00:00:00 2001 From: Brandon Miller Date: Tue, 13 Oct 2026 11:00:07 -0400 Subject: [PATCH 2/8] Fix flag-write forwarding and intrinsic-class overrides in Rust hooks Forward flag-write operations unsupported by LowLevelILFlagWriteOp to the previous architecture with their raw arguments intact, preserving native floating-point flag semantics. Route intrinsic-class queries through ArchitectureHook::intrinsic_class while retaining existing custom-architecture behavior. --- rust/src/architecture.rs | 35 ++++-- rust/src/architecture/hook.rs | 15 +++ rust/tests/architecture_hook.rs | 192 +++++++++++++++++++++++++++++++- 3 files changed, 229 insertions(+), 13 deletions(-) diff --git a/rust/src/architecture.rs b/rust/src/architecture.rs index 5fb70e3748..17bd1ac86f 100644 --- a/rust/src/architecture.rs +++ b/rust/src/architecture.rs @@ -2246,7 +2246,7 @@ where .unwrap_or(0) } - extern "C" fn cb_flag_write_llil( + extern "C" fn cb_flag_write_llil( ctxt: *mut c_void, op: BNLowLevelILOperation, size: usize, @@ -2260,19 +2260,37 @@ where A: 'static + Architecture> + Send + Sync, { let custom_arch = unsafe { &*(ctxt as *mut A) }; - let flag_write = custom_arch.flag_write_from_id(FlagWriteId(flag_write)); - let flag = custom_arch.flag_from_id(FlagId(flag)); + let flag_write_type = custom_arch.flag_write_from_id(FlagWriteId(flag_write)); + let flag_value = custom_arch.flag_from_id(FlagId(flag)); let operands = unsafe { std::slice::from_raw_parts(operands_raw, operand_count) }; let lifter = unsafe { LowLevelILMutableFunction::from_raw_with_arch(il, Some(*custom_arch.as_ref())) }; - if let (Some(flag_write), Some(flag)) = (flag_write, flag) { + if let (Some(flag_write_type), Some(flag_value)) = (flag_write_type, flag_value) { if let Some(op) = LowLevelILFlagWriteOp::from_op(custom_arch, size, op, operands) { - if let Some(expr) = custom_arch.flag_write_llil(flag, flag_write, op, &lifter) { + if let Some(expr) = + custom_arch.flag_write_llil(flag_value, flag_write_type, op, &lifter) + { // TODO verify that returned expr is a bool value return expr.index.0; } + } else if IS_HOOK { + // Hook AsRef points to the previous implementation. Preserve native semantics + // for operations absent from LowLevelILFlagWriteOp (e.g. LLIL_FSUB), passing + // the original operation, IDs and operands through without conversion. + return unsafe { + BNGetArchitectureFlagWriteLowLevelIL( + custom_arch.as_ref().handle, + op, + size, + flag_write, + flag, + operands_raw, + operand_count, + il, + ) + }; } else { tracing::warn!( "unable to unpack flag write op: {:?} with {} operands", @@ -2281,7 +2299,7 @@ where ); } - let role = flag.role(flag_write.class()); + let role = flag_value.role(flag_write_type.class()); unsafe { BNGetDefaultArchitectureFlagWriteLowLevelIL( @@ -2836,7 +2854,10 @@ where getFlagsWrittenByFlagWriteType: Some(cb_flags_written_by_write_type::), getSemanticClassForFlagWriteType: Some(cb_semantic_class_for_flag_write_type::), - getFlagWriteLowLevelIL: Some(cb_flag_write_llil::), + getFlagWriteLowLevelIL: match registration { + ArchitectureRegistration::New(_) => Some(cb_flag_write_llil::), + ArchitectureRegistration::Hook(_) => Some(cb_flag_write_llil::), + }, getFlagConditionLowLevelIL: Some(cb_flag_cond_llil::), getSemanticFlagGroupLowLevelIL: Some(cb_flag_group_llil::), diff --git a/rust/src/architecture/hook.rs b/rust/src/architecture/hook.rs index cd38cefd56..2212c7c033 100644 --- a/rust/src/architecture/hook.rs +++ b/rust/src/architecture/hook.rs @@ -25,6 +25,9 @@ macro_rules! architecture_hook { /// hook chain. Override only the methods you need. Use the supplied `base` to delegate: /// looking up the hooked architecture by name inside a callback would call the hook again. /// + /// Flag-write operations that cannot be represented by [`LowLevelILFlagWriteOp`] bypass + /// the typed callback and forward directly to the previous implementation. + /// /// The methods have the same semantics as their counterparts in [`Architecture`]. /// Contextual disassembly forwards to the previous contextual implementation; override /// [`ArchitectureHook::instruction_text_with_context`] to change contextual text. @@ -206,6 +209,15 @@ pub fn register_architecture_hook( .handle } + unsafe extern "C" fn intrinsic_class( + context: *mut c_void, + intrinsic: u32, + ) -> BNIntrinsicClass { + let arch = unsafe { &*context.cast::>() }; + arch.hook + .intrinsic_class(&arch.base, IntrinsicId(intrinsic)) + } + let registered = super::register_architecture_impl( ArchitectureRegistration::Hook(base), |handle, base| HookArchitecture { hook, base, handle }, @@ -213,6 +225,9 @@ pub fn register_architecture_hook( // The Architecture trait takes an address by value; hooks must also preserve any // address normalization performed by the previous architecture in the chain. callbacks.getAssociatedArchitectureByAddress = Some(associated_arch::); + // Hooks expose a class override independent of intrinsic lookup. Ordinary custom + // architectures continue to obtain the class from their Intrinsic implementation. + callbacks.getIntrinsicClass = Some(intrinsic_class::); }, ); ®istered.hook diff --git a/rust/tests/architecture_hook.rs b/rust/tests/architecture_hook.rs index 305fe76688..93ee97bdc8 100644 --- a/rust/tests/architecture_hook.rs +++ b/rust/tests/architecture_hook.rs @@ -1,11 +1,25 @@ -//! Hook registration is permanent, so these checks run in a separate integration-test process. +//! Hooks permanently modify global architectures. Run all regressions sequentially in one session. use binaryninja::architecture::{ - register_architecture_hook, Architecture, ArchitectureHook, CoreArchitecture, + register_architecture_hook, Architecture, ArchitectureHook, CoreArchitecture, CoreFlag, + CoreFlagWrite, CoreRegister, Flag, FlagWrite, Intrinsic, IntrinsicId, }; use binaryninja::disassembly::{InstructionTextToken, InstructionTextTokenKind}; use binaryninja::headless::Session; -use std::ffi::c_void; +use binaryninja::low_level_il::expression::ValueExpr; +use binaryninja::low_level_il::lifting::LowLevelILFlagWriteOp; +use binaryninja::low_level_il::{LowLevelILMutableExpression, LowLevelILMutableFunction}; +use binaryninjacore_sys::*; +use std::ffi::{c_void, CString}; use std::ptr::NonNull; +use std::sync::atomic::{AtomicUsize, Ordering}; + +#[test] +fn test_architecture_hook() { + let _session = Session::new().expect("Failed to initialize session"); + check_instruction_text(); + check_floating_point_flags(); + check_intrinsic_class_override(); +} struct TextHook; @@ -39,9 +53,7 @@ impl ArchitectureHook for TextHook { struct PassThroughHook; impl ArchitectureHook for PassThroughHook {} -#[test] -fn test_architecture_hook() { - let _session = Session::new().expect("Failed to initialize session"); +fn check_instruction_text() { let arch = CoreArchitecture::by_name("x86_64").expect("Failed to get architecture"); let data = [0x90]; // nop let address = 0x1000; @@ -71,3 +83,171 @@ fn test_architecture_hook() { expected ); } + +struct TypedFlagHook; +impl ArchitectureHook for TypedFlagHook { + fn flag_write_llil<'a>( + &self, + _base: &CoreArchitecture, + _flag: CoreFlag, + _write: CoreFlagWrite, + _op: LowLevelILFlagWriteOp, + il: &'a LowLevelILMutableFunction, + ) -> Option> { + Some(il.const_int(0, 1)) + } +} + +// Compare the full expression, including operand values and sizes, rather than just its root op. +type Expression = (BNLowLevelILOperation, usize, u32, [u64; 4]); + +fn flag_write_il( + arch: CoreArchitecture, + write: u32, + flag: u32, + operation: BNLowLevelILOperation, +) -> (usize, Vec) { + let name = CString::new(arch.name()).unwrap(); + let mut operands = [1.25f64.to_bits(), 2.5f64.to_bits()].map(|value| BNRegisterOrConstant { + constant: true, + reg: 0, + value, + }); + // Use the core API because LLIL_FSUB has no LowLevelILFlagWriteOp variant. + unsafe { + let arch = BNGetArchitectureByName(name.as_ptr()); + let il = BNCreateLowLevelILFunction(arch, std::ptr::null_mut()); + assert!(!il.is_null()); + let root = BNGetArchitectureFlagWriteLowLevelIL( + arch, + operation, + 8, + write, + flag, + operands.as_mut_ptr(), + operands.len(), + il, + ); + let expressions = (0..BNGetLowLevelILExprCount(il)) + .map(|index| { + let expr = BNGetLowLevelILByIndex(il, index); + (expr.operation, expr.size, expr.flags, expr.operands) + }) + .collect(); + BNFreeLowLevelILFunction(il); + (root, expressions) + } +} + +fn check_floating_point_flags() { + use BNLowLevelILOperation::*; + for (name, write_name, flags) in [ + ( + "armv7", + "fcmp", + vec![("n", LLIL_FCMP_LT), ("v", LLIL_FCMP_UO)], + ), + ( + "x86", + "vcomi", + vec![("s", LLIL_CONST), ("o", LLIL_CONST), ("a", LLIL_CONST)], + ), + ( + "x86_64", + "vcomi", + vec![("s", LLIL_CONST), ("o", LLIL_CONST), ("a", LLIL_CONST)], + ), + ] { + let arch = CoreArchitecture::by_name(name).unwrap(); + let write = arch + .flag_write_types() + .into_iter() + .find(|write| write.name() == write_name) + .unwrap() + .id() + .0; + let originals: Vec<_> = flags + .into_iter() + .map(|(flag_name, operation)| { + let flag = arch + .flags() + .into_iter() + .find(|flag| flag.name() == flag_name) + .unwrap() + .id() + .0; + let original = flag_write_il(arch, write, flag, LLIL_FSUB); + assert_eq!(original.1[original.0].0, operation, "{name}: {flag_name}"); + (flag, original) + }) + .collect(); + + register_architecture_hook(arch, PassThroughHook); + for (flag, expected) in &originals { + assert_eq!( + flag_write_il(arch, write, *flag, LLIL_FSUB), + *expected, + "{name}" + ); + } + + register_architecture_hook(arch, TypedFlagHook); + // An unsupported operation delegates through both hooks with its raw arguments intact. + for (flag, expected) in &originals { + assert_eq!( + flag_write_il(arch, write, *flag, LLIL_FSUB), + *expected, + "{name}" + ); + } + // Supported operations must still call the typed hook override. + let (root, expressions) = flag_write_il(arch, write, originals[0].0, LLIL_ADD); + assert_eq!(expressions[root].0, LLIL_CONST); + assert_eq!(expressions[root].3[0], 1); + } +} + +fn opposite_class(class: BNIntrinsicClass) -> BNIntrinsicClass { + match class { + BNIntrinsicClass::GeneralIntrinsicClass => BNIntrinsicClass::MemoryIntrinsicClass, + BNIntrinsicClass::MemoryIntrinsicClass => BNIntrinsicClass::GeneralIntrinsicClass, + } +} + +#[derive(Default)] +struct IntrinsicClassHook { + calls: AtomicUsize, +} + +impl ArchitectureHook for IntrinsicClassHook { + fn intrinsic_class(&self, base: &CoreArchitecture, id: IntrinsicId) -> BNIntrinsicClass { + self.calls.fetch_add(1, Ordering::Relaxed); + opposite_class(base.intrinsic_class(id)) + } +} + +fn check_intrinsic_class_override() { + for name in ["x86_64", "armv7"] { + let arch = CoreArchitecture::by_name(name).unwrap(); + let intrinsic = arch.intrinsics().into_iter().next().unwrap(); + let id = intrinsic.id(); + let original = arch.intrinsic_class(id); + + register_architecture_hook(arch, PassThroughHook); + assert_eq!(arch.intrinsic_class(id), original, "{name}"); + + let hook = register_architecture_hook(arch, IntrinsicClassHook::default()); + let calls = hook.calls.load(Ordering::Relaxed); + assert_eq!(arch.intrinsic_class(id), opposite_class(original), "{name}"); + assert_eq!(hook.calls.load(Ordering::Relaxed), calls + 1, "{name}"); + assert_eq!(intrinsic.class(), opposite_class(original), "{name}"); + + // Default forwarding through another hook must retain the override. + register_architecture_hook(arch, PassThroughHook); + assert_eq!(arch.intrinsic_class(id), opposite_class(original), "{name}"); + + // Delegation should reach the earlier override, so flipping twice restores the class. + register_architecture_hook(arch, IntrinsicClassHook::default()); + assert_eq!(arch.intrinsic_class(id), original, "{name}"); + } +} From 079ec5c222dae85b513dfbc511134d3bcb71e2b1 Mon Sep 17 00:00:00 2001 From: Brandon Miller Date: Tue, 20 Oct 2026 11:22:08 -0400 Subject: [PATCH 3/8] Preserve address mutation throughout the Rust architecture API Change associated_arch_by_addr to accept &mut u64 and preserve updates through the shared C callback. Update RISC-V and MSP430 implementations. --- arch/msp430/src/architecture.rs | 2 +- arch/riscv/src/lib.rs | 2 +- rust/src/architecture.rs | 18 +++++++----------- rust/src/architecture/hook.rs | 25 +------------------------ rust/tests/architecture.rs | 11 +++++++++++ rust/tests/architecture_hook.rs | 29 +++++++++++++++++++++++++++++ 6 files changed, 50 insertions(+), 37 deletions(-) diff --git a/arch/msp430/src/architecture.rs b/arch/msp430/src/architecture.rs index 76f93ac422..6575fdfbf9 100644 --- a/arch/msp430/src/architecture.rs +++ b/arch/msp430/src/architecture.rs @@ -75,7 +75,7 @@ impl Architecture for Msp430 { self.max_instr_len() } - fn associated_arch_by_addr(&self, _addr: u64) -> CoreArchitecture { + fn associated_arch_by_addr(&self, _addr: &mut u64) -> CoreArchitecture { self.handle } diff --git a/arch/riscv/src/lib.rs b/arch/riscv/src/lib.rs index d796bef1b4..7909045c28 100644 --- a/arch/riscv/src/lib.rs +++ b/arch/riscv/src/lib.rs @@ -770,7 +770,7 @@ impl Architecture for RiscVArch { self.max_instr_len() } - fn associated_arch_by_addr(&self, _addr: u64) -> CoreArchitecture { + fn associated_arch_by_addr(&self, _addr: &mut u64) -> CoreArchitecture { self.handle } diff --git a/rust/src/architecture.rs b/rust/src/architecture.rs index 17bd1ac86f..7e1b55786b 100644 --- a/rust/src/architecture.rs +++ b/rust/src/architecture.rs @@ -183,7 +183,9 @@ pub trait Architecture: 'static + Sized + AsRef { /// In binaries with multiple architectures, you may wish to associate a specific architecture /// with a given virtual address. This can be seen in armv7 where odd addresses are associated /// with the thumb architecture. - fn associated_arch_by_addr(&self, _addr: u64) -> CoreArchitecture { + /// + /// The address can be updated in place to normalize it, such as clearing the ARM Thumb bit. + fn associated_arch_by_addr(&self, _addr: &mut u64) -> CoreArchitecture { *self.as_ref() } @@ -814,13 +816,6 @@ impl CoreArchitecture { CoreArchitecture { handle } } - /// Selects the architecture for `addr` and applies its address normalization in place. - /// For example, ARM uses the low address bit to select Thumb mode, then clears that bit. - pub fn associated_arch_by_addr_mut(&self, addr: &mut u64) -> CoreArchitecture { - let handle = unsafe { BNGetAssociatedArchitectureByAddress(self.handle, addr) }; - CoreArchitecture { handle } - } - pub fn list_all() -> CoreArchitectureList { let mut count: usize = 0; let archs = unsafe { BNGetArchitectureList(&mut count) }; @@ -903,8 +898,9 @@ impl Architecture for CoreArchitecture { unsafe { BNGetArchitectureOpcodeDisplayLength(self.handle) } } - fn associated_arch_by_addr(&self, mut addr: u64) -> CoreArchitecture { - self.associated_arch_by_addr_mut(&mut addr) + fn associated_arch_by_addr(&self, addr: &mut u64) -> CoreArchitecture { + let handle = unsafe { BNGetAssociatedArchitectureByAddress(self.handle, addr) }; + CoreArchitecture { handle } } fn instruction_info(&self, data: &[u8], addr: u64) -> Option { @@ -1718,7 +1714,7 @@ where A: 'static + Architecture> + Send + Sync, { let custom_arch = unsafe { &*(ctxt as *mut A) }; - let addr = unsafe { *(addr) }; + let addr = unsafe { &mut *addr }; custom_arch.associated_arch_by_addr(addr).handle } diff --git a/rust/src/architecture/hook.rs b/rust/src/architecture/hook.rs index 2212c7c033..ac20166b74 100644 --- a/rust/src/architecture/hook.rs +++ b/rust/src/architecture/hook.rs @@ -34,13 +34,6 @@ macro_rules! architecture_hook { /// /// Register implementations with [`register_architecture_hook`]. pub trait ArchitectureHook: Send + Sync + 'static { - /// Selects an architecture and updates the address, preserving normalization such - /// as clearing the ARM Thumb bit. Unlike [`Architecture::associated_arch_by_addr`], - /// this callback can update the caller's address. - fn associated_arch_by_addr(&self, base: &CoreArchitecture, addr: &mut u64) -> CoreArchitecture { - base.associated_arch_by_addr_mut(addr) - } - $( #[doc = concat!("See [`Architecture::", stringify!($name), "`].")] fn $name $(<$lt>)? (&self, base: &CoreArchitecture, $($arg: $ty),*) -> $ret { @@ -67,10 +60,6 @@ macro_rules! architecture_hook { } )* - fn associated_arch_by_addr(&self, mut addr: u64) -> CoreArchitecture { - self.hook.associated_arch_by_addr(&self.base, &mut addr) - } - fn handle(&self) -> Self::Handle { self.handle } @@ -87,6 +76,7 @@ architecture_hook! { fn linear_sweep_analysis_capabilities() -> u32; fn max_instr_len() -> usize; fn opcode_display_len() -> usize; + fn associated_arch_by_addr(addr: &mut u64) -> CoreArchitecture; fn instruction_info(data: &[u8], addr: u64) -> Option; fn instruction_text(data: &[u8], addr: u64) -> Option<(usize, Vec)>; fn instruction_text_with_context( @@ -199,16 +189,6 @@ pub fn register_architecture_hook( base: CoreArchitecture, hook: H, ) -> &'static H { - unsafe extern "C" fn associated_arch( - context: *mut c_void, - addr: *mut u64, - ) -> *mut BNArchitecture { - let arch = unsafe { &*context.cast::>() }; - arch.hook - .associated_arch_by_addr(&arch.base, unsafe { &mut *addr }) - .handle - } - unsafe extern "C" fn intrinsic_class( context: *mut c_void, intrinsic: u32, @@ -222,9 +202,6 @@ pub fn register_architecture_hook( ArchitectureRegistration::Hook(base), |handle, base| HookArchitecture { hook, base, handle }, |callbacks| { - // The Architecture trait takes an address by value; hooks must also preserve any - // address normalization performed by the previous architecture in the chain. - callbacks.getAssociatedArchitectureByAddress = Some(associated_arch::); // Hooks expose a class override independent of intrinsic lookup. Ordinary custom // architectures continue to obtain the class from their Intrinsic implementation. callbacks.getIntrinsicClass = Some(intrinsic_class::); diff --git a/rust/tests/architecture.rs b/rust/tests/architecture.rs index eef1d29ef2..4d195b69b1 100644 --- a/rust/tests/architecture.rs +++ b/rust/tests/architecture.rs @@ -13,6 +13,17 @@ fn test_architecture_info() { assert_eq!(arch.address_size(), 8); } +#[test] +fn test_associated_arch_by_addr() { + let _session = Session::new().expect("Failed to initialize session"); + let arch = CoreArchitecture::by_name("armv7").expect("Failed to get architecture"); + let mut address = 0x1001; + assert_eq!(arch.associated_arch_by_addr(&mut address).name(), "thumb2"); + assert_eq!(address, 0x1000); + assert_eq!(arch.associated_arch_by_addr(&mut address), arch); + assert_eq!(address, 0x1000); +} + #[test] fn test_architecture_disassembly() { let _session = Session::new().expect("Failed to initialize session"); diff --git a/rust/tests/architecture_hook.rs b/rust/tests/architecture_hook.rs index 93ee97bdc8..1f9d856abe 100644 --- a/rust/tests/architecture_hook.rs +++ b/rust/tests/architecture_hook.rs @@ -19,6 +19,35 @@ fn test_architecture_hook() { check_instruction_text(); check_floating_point_flags(); check_intrinsic_class_override(); + check_associated_arch_by_addr(); +} + +struct AddressHook; + +impl ArchitectureHook for AddressHook { + fn associated_arch_by_addr(&self, base: &CoreArchitecture, addr: &mut u64) -> CoreArchitecture { + *addr += 0x1000; + let arch = base.associated_arch_by_addr(addr); + *addr += 0x2000; + arch + } +} + +fn check_associated_arch_by_addr() { + let arch = CoreArchitecture::by_name("armv7").expect("Failed to get architecture"); + let mut address = 0x1001; + assert_eq!(arch.associated_arch_by_addr(&mut address).name(), "thumb2"); + assert_eq!(address, 0x1000); + + register_architecture_hook(arch, AddressHook); + let mut address = 0x1001; + assert_eq!(arch.associated_arch_by_addr(&mut address).name(), "thumb2"); + assert_eq!(address, 0x4000); + + register_architecture_hook(arch, PassThroughHook); + let mut address = 0x1001; + assert_eq!(arch.associated_arch_by_addr(&mut address).name(), "thumb2"); + assert_eq!(address, 0x4000); } struct TextHook; From bc30dba1b0425e2bae1368e78d3efd48b0bb0cb2 Mon Sep 17 00:00:00 2001 From: Brandon Miller Date: Tue, 20 Oct 2026 12:05:20 -0400 Subject: [PATCH 4/8] Honor architecture-level intrinsic class overrides in Rust Dispatch the shared intrinsic-class callback through Architecture::intrinsic_class. Default to Intrinsic::class() for known IDs and GeneralIntrinsicClass otherwise, preserving existing intrinsic implementations. --- rust/src/architecture.rs | 14 +++++++------- rust/src/architecture/hook.rs | 15 +-------------- 2 files changed, 8 insertions(+), 21 deletions(-) diff --git a/rust/src/architecture.rs b/rust/src/architecture.rs index 7e1b55786b..bb52eecb7e 100644 --- a/rust/src/architecture.rs +++ b/rust/src/architecture.rs @@ -507,8 +507,12 @@ pub trait Architecture: 'static + Sized + AsRef { Vec::new() } - fn intrinsic_class(&self, _id: IntrinsicId) -> BNIntrinsicClass { - BNIntrinsicClass::GeneralIntrinsicClass + /// Returns the class from [`Intrinsic::class`], or the general class for an unknown ID. + fn intrinsic_class(&self, id: IntrinsicId) -> BNIntrinsicClass { + match self.intrinsic_from_id(id) { + Some(intrinsic) => intrinsic.class(), + None => BNIntrinsicClass::GeneralIntrinsicClass, + } } /// Get the [`Self::Intrinsic`] associated with the given [`IntrinsicId`]. @@ -2483,11 +2487,7 @@ where A: 'static + Architecture> + Send + Sync, { let custom_arch = unsafe { &*(ctxt as *mut A) }; - match custom_arch.intrinsic_from_id(IntrinsicId(intrinsic)) { - Some(intrinsic) => intrinsic.class(), - // TODO: Make this unreachable? - None => BNIntrinsicClass::GeneralIntrinsicClass, - } + custom_arch.intrinsic_class(IntrinsicId(intrinsic)) } extern "C" fn cb_intrinsic_name(ctxt: *mut c_void, intrinsic: u32) -> *mut c_char diff --git a/rust/src/architecture/hook.rs b/rust/src/architecture/hook.rs index ac20166b74..221a3012af 100644 --- a/rust/src/architecture/hook.rs +++ b/rust/src/architecture/hook.rs @@ -189,23 +189,10 @@ pub fn register_architecture_hook( base: CoreArchitecture, hook: H, ) -> &'static H { - unsafe extern "C" fn intrinsic_class( - context: *mut c_void, - intrinsic: u32, - ) -> BNIntrinsicClass { - let arch = unsafe { &*context.cast::>() }; - arch.hook - .intrinsic_class(&arch.base, IntrinsicId(intrinsic)) - } - let registered = super::register_architecture_impl( ArchitectureRegistration::Hook(base), |handle, base| HookArchitecture { hook, base, handle }, - |callbacks| { - // Hooks expose a class override independent of intrinsic lookup. Ordinary custom - // architectures continue to obtain the class from their Intrinsic implementation. - callbacks.getIntrinsicClass = Some(intrinsic_class::); - }, + |_| {}, ); ®istered.hook } From 55933ee0e78e4f5b6cef5eefa7cf1b4a3d707b7f Mon Sep 17 00:00:00 2001 From: Brandon Miller Date: Tue, 20 Oct 2026 12:17:15 -0400 Subject: [PATCH 5/8] Expand Rust flag-write operations and unify callback dispatch Add floating-point and comparison variants to LowLevelILFlagWriteOp, updating decoding, operation/size mapping, and operand conversion together. Add a shared Raw variant to preserve unrepresented operations and complete operand lists. --- rust/src/architecture.rs | 52 +--- rust/src/architecture/hook.rs | 4 +- rust/src/low_level_il/lifting.rs | 426 ++++++++++++++++++++++++-- rust/tests/architecture_flag_write.rs | 304 ++++++++++++++++++ rust/tests/architecture_hook.rs | 16 +- 5 files changed, 726 insertions(+), 76 deletions(-) create mode 100644 rust/tests/architecture_flag_write.rs diff --git a/rust/src/architecture.rs b/rust/src/architecture.rs index bb52eecb7e..56daea3bca 100644 --- a/rust/src/architecture.rs +++ b/rust/src/architecture.rs @@ -1035,7 +1035,7 @@ impl Architecture for CoreArchitecture { il: &'a LowLevelILMutableFunction, ) -> Option> { let (size, operation) = op.size_and_op(); - let (count, mut operands) = op.raw_operands(); + let mut operands = op.raw_operands(); let index = unsafe { BNGetArchitectureFlagWriteLowLevelIL( self.handle, @@ -1044,7 +1044,7 @@ impl Architecture for CoreArchitecture { flag_write.id().0, flag.id().0, operands.as_mut_ptr(), - count, + operands.len(), il.handle, ) }; @@ -2246,7 +2246,7 @@ where .unwrap_or(0) } - extern "C" fn cb_flag_write_llil( + extern "C" fn cb_flag_write_llil( ctxt: *mut c_void, op: BNLowLevelILOperation, size: usize, @@ -2262,41 +2262,22 @@ where let custom_arch = unsafe { &*(ctxt as *mut A) }; let flag_write_type = custom_arch.flag_write_from_id(FlagWriteId(flag_write)); let flag_value = custom_arch.flag_from_id(FlagId(flag)); - let operands = unsafe { std::slice::from_raw_parts(operands_raw, operand_count) }; + let operands = if operand_count == 0 { + &[] + } else { + unsafe { std::slice::from_raw_parts(operands_raw, operand_count) } + }; let lifter = unsafe { LowLevelILMutableFunction::from_raw_with_arch(il, Some(*custom_arch.as_ref())) }; if let (Some(flag_write_type), Some(flag_value)) = (flag_write_type, flag_value) { - if let Some(op) = LowLevelILFlagWriteOp::from_op(custom_arch, size, op, operands) { - if let Some(expr) = - custom_arch.flag_write_llil(flag_value, flag_write_type, op, &lifter) - { - // TODO verify that returned expr is a bool value - return expr.index.0; - } - } else if IS_HOOK { - // Hook AsRef points to the previous implementation. Preserve native semantics - // for operations absent from LowLevelILFlagWriteOp (e.g. LLIL_FSUB), passing - // the original operation, IDs and operands through without conversion. - return unsafe { - BNGetArchitectureFlagWriteLowLevelIL( - custom_arch.as_ref().handle, - op, - size, - flag_write, - flag, - operands_raw, - operand_count, - il, - ) - }; - } else { - tracing::warn!( - "unable to unpack flag write op: {:?} with {} operands", - op, - operands.len() - ); + let decoded_op = LowLevelILFlagWriteOp::from_op(custom_arch, size, op, operands); + if let Some(expr) = + custom_arch.flag_write_llil(flag_value, flag_write_type, decoded_op, &lifter) + { + // TODO verify that returned expr is a bool value + return expr.index.0; } let role = flag_value.role(flag_write_type.class()); @@ -2850,10 +2831,7 @@ where getFlagsWrittenByFlagWriteType: Some(cb_flags_written_by_write_type::), getSemanticClassForFlagWriteType: Some(cb_semantic_class_for_flag_write_type::), - getFlagWriteLowLevelIL: match registration { - ArchitectureRegistration::New(_) => Some(cb_flag_write_llil::), - ArchitectureRegistration::Hook(_) => Some(cb_flag_write_llil::), - }, + getFlagWriteLowLevelIL: Some(cb_flag_write_llil::), getFlagConditionLowLevelIL: Some(cb_flag_cond_llil::), getSemanticFlagGroupLowLevelIL: Some(cb_flag_group_llil::), diff --git a/rust/src/architecture/hook.rs b/rust/src/architecture/hook.rs index 221a3012af..ab8ad55262 100644 --- a/rust/src/architecture/hook.rs +++ b/rust/src/architecture/hook.rs @@ -25,8 +25,8 @@ macro_rules! architecture_hook { /// hook chain. Override only the methods you need. Use the supplied `base` to delegate: /// looking up the hooked architecture by name inside a callback would call the hook again. /// - /// Flag-write operations that cannot be represented by [`LowLevelILFlagWriteOp`] bypass - /// the typed callback and forward directly to the previous implementation. + /// Flag-write operations without a typed representation are delivered as + /// [`LowLevelILFlagWriteOp::Raw`], including their complete operand list. /// /// The methods have the same semantics as their counterparts in [`Architecture`]. /// Contextual disassembly forwards to the previous contextual implementation; override diff --git a/rust/src/low_level_il/lifting.rs b/rust/src/low_level_il/lifting.rs index e2fb84e425..d64ef0cbb9 100644 --- a/rust/src/low_level_il/lifting.rs +++ b/rust/src/low_level_il/lifting.rs @@ -68,8 +68,11 @@ impl From> for BNRegisterOrConstant } } -// TODO flesh way out -#[derive(Copy, Clone)] +/// An operation whose result is used to calculate a flag value. +/// +/// Floating-point constants retain their original bit patterns. Operations that cannot be +/// represented by a typed variant are passed to the same callback as [`Self::Raw`]. +#[derive(Clone)] pub enum LowLevelILFlagWriteOp { SetReg(usize, LowLevelILRegisterOrConstant), SetRegSplit( @@ -266,7 +269,135 @@ pub enum LowLevelILFlagWriteOp { ), Pop(usize), - // TODO: floating point stuff, llil comparison ops that set flags, intrinsics + Fsqrt(usize, LowLevelILRegisterOrConstant), + Fneg(usize, LowLevelILRegisterOrConstant), + Fabs(usize, LowLevelILRegisterOrConstant), + IntToFloat(usize, LowLevelILRegisterOrConstant), + FloatConv(usize, LowLevelILRegisterOrConstant), + RoundToInt(usize, LowLevelILRegisterOrConstant), + Floor(usize, LowLevelILRegisterOrConstant), + Ceil(usize, LowLevelILRegisterOrConstant), + Ftrunc(usize, LowLevelILRegisterOrConstant), + + Fadd( + usize, + LowLevelILRegisterOrConstant, + LowLevelILRegisterOrConstant, + ), + Fsub( + usize, + LowLevelILRegisterOrConstant, + LowLevelILRegisterOrConstant, + ), + Fmul( + usize, + LowLevelILRegisterOrConstant, + LowLevelILRegisterOrConstant, + ), + Fdiv( + usize, + LowLevelILRegisterOrConstant, + LowLevelILRegisterOrConstant, + ), + CmpE( + usize, + LowLevelILRegisterOrConstant, + LowLevelILRegisterOrConstant, + ), + CmpNe( + usize, + LowLevelILRegisterOrConstant, + LowLevelILRegisterOrConstant, + ), + CmpSlt( + usize, + LowLevelILRegisterOrConstant, + LowLevelILRegisterOrConstant, + ), + CmpUlt( + usize, + LowLevelILRegisterOrConstant, + LowLevelILRegisterOrConstant, + ), + CmpSle( + usize, + LowLevelILRegisterOrConstant, + LowLevelILRegisterOrConstant, + ), + CmpUle( + usize, + LowLevelILRegisterOrConstant, + LowLevelILRegisterOrConstant, + ), + CmpSge( + usize, + LowLevelILRegisterOrConstant, + LowLevelILRegisterOrConstant, + ), + CmpUge( + usize, + LowLevelILRegisterOrConstant, + LowLevelILRegisterOrConstant, + ), + CmpSgt( + usize, + LowLevelILRegisterOrConstant, + LowLevelILRegisterOrConstant, + ), + CmpUgt( + usize, + LowLevelILRegisterOrConstant, + LowLevelILRegisterOrConstant, + ), + FcmpE( + usize, + LowLevelILRegisterOrConstant, + LowLevelILRegisterOrConstant, + ), + FcmpNe( + usize, + LowLevelILRegisterOrConstant, + LowLevelILRegisterOrConstant, + ), + FcmpLt( + usize, + LowLevelILRegisterOrConstant, + LowLevelILRegisterOrConstant, + ), + FcmpLe( + usize, + LowLevelILRegisterOrConstant, + LowLevelILRegisterOrConstant, + ), + FcmpGe( + usize, + LowLevelILRegisterOrConstant, + LowLevelILRegisterOrConstant, + ), + FcmpGt( + usize, + LowLevelILRegisterOrConstant, + LowLevelILRegisterOrConstant, + ), + FcmpO( + usize, + LowLevelILRegisterOrConstant, + LowLevelILRegisterOrConstant, + ), + FcmpUo( + usize, + LowLevelILRegisterOrConstant, + LowLevelILRegisterOrConstant, + ), + + /// Preserves the operation, size, and complete operand list when typed decoding is unavailable. + /// This includes intrinsic flag writes (whose first operand is the intrinsic ID), unexpected + /// operand counts, and operands that cannot be resolved as architecture registers. + Raw { + operation: BNLowLevelILOperation, + size: usize, + operands: Vec, + }, } impl LowLevelILFlagWriteOp { @@ -275,6 +406,23 @@ impl LowLevelILFlagWriteOp { size: usize, op: BNLowLevelILOperation, operands: &[BNRegisterOrConstant], + ) -> Self + where + A: Architecture, + R: ArchReg, + { + Self::try_from_op(arch, size, op, operands).unwrap_or_else(|| Self::Raw { + operation: op, + size, + operands: operands.to_vec(), + }) + } + + fn try_from_op( + arch: &A, + size: usize, + op: BNLowLevelILOperation, + operands: &[BNRegisterOrConstant], ) -> Option where A: Architecture, @@ -287,24 +435,23 @@ impl LowLevelILFlagWriteOp { arch: &A, size: usize, operand: &BNRegisterOrConstant, - ) -> LowLevelILRegisterOrConstant + ) -> Option> where A: Architecture, R: ArchReg, { if operand.constant { - LowLevelILRegisterOrConstant::Constant(size, operand.value) + Some(LowLevelILRegisterOrConstant::Constant(size, operand.value)) } else { let raw_id = RegisterId(operand.reg); - let il_reg = - LowLevelILRegisterKind::from_raw(arch, raw_id).expect("Bad register ID"); - LowLevelILRegisterOrConstant::Register(size, il_reg) + let il_reg = LowLevelILRegisterKind::from_raw(arch, raw_id)?; + Some(LowLevelILRegisterOrConstant::Register(size, il_reg)) } } macro_rules! op { ($x:ident, $($ops:expr),*) => { - ( $x(size, $( build_op(arch, size, &operands[$ops]), )* ) ) + ( $x(size, $( build_op(arch, size, &operands[$ops])?, )* ) ) }; } @@ -369,6 +516,38 @@ impl LowLevelILFlagWriteOp { (0, LLIL_POP) => op!(Pop,), + (1, LLIL_FSQRT) => op!(Fsqrt, 0), + (1, LLIL_FNEG) => op!(Fneg, 0), + (1, LLIL_FABS) => op!(Fabs, 0), + (1, LLIL_INT_TO_FLOAT) => op!(IntToFloat, 0), + (1, LLIL_FLOAT_CONV) => op!(FloatConv, 0), + (1, LLIL_ROUND_TO_INT) => op!(RoundToInt, 0), + (1, LLIL_FLOOR) => op!(Floor, 0), + (1, LLIL_CEIL) => op!(Ceil, 0), + (1, LLIL_FTRUNC) => op!(Ftrunc, 0), + (2, LLIL_FADD) => op!(Fadd, 0, 1), + (2, LLIL_FSUB) => op!(Fsub, 0, 1), + (2, LLIL_FMUL) => op!(Fmul, 0, 1), + (2, LLIL_FDIV) => op!(Fdiv, 0, 1), + (2, LLIL_CMP_E) => op!(CmpE, 0, 1), + (2, LLIL_CMP_NE) => op!(CmpNe, 0, 1), + (2, LLIL_CMP_SLT) => op!(CmpSlt, 0, 1), + (2, LLIL_CMP_ULT) => op!(CmpUlt, 0, 1), + (2, LLIL_CMP_SLE) => op!(CmpSle, 0, 1), + (2, LLIL_CMP_ULE) => op!(CmpUle, 0, 1), + (2, LLIL_CMP_SGE) => op!(CmpSge, 0, 1), + (2, LLIL_CMP_UGE) => op!(CmpUge, 0, 1), + (2, LLIL_CMP_SGT) => op!(CmpSgt, 0, 1), + (2, LLIL_CMP_UGT) => op!(CmpUgt, 0, 1), + (2, LLIL_FCMP_E) => op!(FcmpE, 0, 1), + (2, LLIL_FCMP_NE) => op!(FcmpNe, 0, 1), + (2, LLIL_FCMP_LT) => op!(FcmpLt, 0, 1), + (2, LLIL_FCMP_LE) => op!(FcmpLe, 0, 1), + (2, LLIL_FCMP_GE) => op!(FcmpGe, 0, 1), + (2, LLIL_FCMP_GT) => op!(FcmpGt, 0, 1), + (2, LLIL_FCMP_O) => op!(FcmpO, 0, 1), + (2, LLIL_FCMP_UO) => op!(FcmpUo, 0, 1), + _ => return None, }) } @@ -437,16 +616,52 @@ impl LowLevelILFlagWriteOp { Rrc(size, ..) => (size, LLIL_RRC), Pop(size) => (size, LLIL_POP), + + Fsqrt(size, ..) => (size, LLIL_FSQRT), + Fneg(size, ..) => (size, LLIL_FNEG), + Fabs(size, ..) => (size, LLIL_FABS), + IntToFloat(size, ..) => (size, LLIL_INT_TO_FLOAT), + FloatConv(size, ..) => (size, LLIL_FLOAT_CONV), + RoundToInt(size, ..) => (size, LLIL_ROUND_TO_INT), + Floor(size, ..) => (size, LLIL_FLOOR), + Ceil(size, ..) => (size, LLIL_CEIL), + Ftrunc(size, ..) => (size, LLIL_FTRUNC), + Fadd(size, ..) => (size, LLIL_FADD), + Fsub(size, ..) => (size, LLIL_FSUB), + Fmul(size, ..) => (size, LLIL_FMUL), + Fdiv(size, ..) => (size, LLIL_FDIV), + CmpE(size, ..) => (size, LLIL_CMP_E), + CmpNe(size, ..) => (size, LLIL_CMP_NE), + CmpSlt(size, ..) => (size, LLIL_CMP_SLT), + CmpUlt(size, ..) => (size, LLIL_CMP_ULT), + CmpSle(size, ..) => (size, LLIL_CMP_SLE), + CmpUle(size, ..) => (size, LLIL_CMP_ULE), + CmpSge(size, ..) => (size, LLIL_CMP_SGE), + CmpUge(size, ..) => (size, LLIL_CMP_UGE), + CmpSgt(size, ..) => (size, LLIL_CMP_SGT), + CmpUgt(size, ..) => (size, LLIL_CMP_UGT), + FcmpE(size, ..) => (size, LLIL_FCMP_E), + FcmpNe(size, ..) => (size, LLIL_FCMP_NE), + FcmpLt(size, ..) => (size, LLIL_FCMP_LT), + FcmpLe(size, ..) => (size, LLIL_FCMP_LE), + FcmpGe(size, ..) => (size, LLIL_FCMP_GE), + FcmpGt(size, ..) => (size, LLIL_FCMP_GT), + FcmpO(size, ..) => (size, LLIL_FCMP_O), + FcmpUo(size, ..) => (size, LLIL_FCMP_UO), + Raw { + operation, size, .. + } => (size, operation), } } - pub(crate) fn raw_operands(&self) -> (usize, [BNRegisterOrConstant; 5]) { + pub(crate) fn raw_operands(&self) -> Vec { use self::LowLevelILFlagWriteOp::*; - let mut operands: [BNRegisterOrConstant; 5] = [BNRegisterOrConstant::default(); 5]; + let mut operands = Vec::new(); - let count = match *self { - Pop(_) => 0, + match *self { + Raw { ref operands, .. } => return operands.clone(), + Pop(_) => {} SetReg(_, op0) | Load(_, op0) @@ -464,9 +679,17 @@ impl LowLevelILFlagWriteOp { | Zx(_, op0) | LowPart(_, op0) | BoolToInt(_, op0) - | FloatToInt(_, op0) => { - operands[0] = op0.into(); - 1 + | FloatToInt(_, op0) + | Fsqrt(_, op0) + | Fneg(_, op0) + | Fabs(_, op0) + | IntToFloat(_, op0) + | FloatConv(_, op0) + | RoundToInt(_, op0) + | Floor(_, op0) + | Ceil(_, op0) + | Ftrunc(_, op0) => { + operands.push(op0.into()); } SetRegSplit(_, op0, op1) @@ -497,24 +720,41 @@ impl LowLevelILFlagWriteOp { | ModuDp(_, op0, op1) | ModsDp(_, op0, op1) | TestBit(_, op0, op1) - | AddOverflow(_, op0, op1) => { - operands[0] = op0.into(); - operands[1] = op1.into(); - 2 + | AddOverflow(_, op0, op1) + | Fadd(_, op0, op1) + | Fsub(_, op0, op1) + | Fmul(_, op0, op1) + | Fdiv(_, op0, op1) + | CmpE(_, op0, op1) + | CmpNe(_, op0, op1) + | CmpSlt(_, op0, op1) + | CmpUlt(_, op0, op1) + | CmpSle(_, op0, op1) + | CmpUle(_, op0, op1) + | CmpSge(_, op0, op1) + | CmpUge(_, op0, op1) + | CmpSgt(_, op0, op1) + | CmpUgt(_, op0, op1) + | FcmpE(_, op0, op1) + | FcmpNe(_, op0, op1) + | FcmpLt(_, op0, op1) + | FcmpLe(_, op0, op1) + | FcmpGe(_, op0, op1) + | FcmpGt(_, op0, op1) + | FcmpO(_, op0, op1) + | FcmpUo(_, op0, op1) => { + operands.extend([op0, op1].map(BNRegisterOrConstant::from)); } Adc(_, op0, op1, op2) | Sbb(_, op0, op1, op2) | Rlc(_, op0, op1, op2) | Rrc(_, op0, op1, op2) => { - operands[0] = op0.into(); - operands[1] = op1.into(); - operands[2] = op2.into(); - 3 + operands.extend([op0, op1, op2].map(BNRegisterOrConstant::from)); } }; - (count, operands) + operands } } @@ -528,7 +768,7 @@ where A: 'func + Architecture, { let (size, operation) = op.size_and_op(); - let (count, operands) = op.raw_operands(); + let mut operands = op.raw_operands(); let expr_idx = unsafe { use binaryninjacore_sys::BNGetDefaultArchitectureFlagWriteLowLevelIL; @@ -537,8 +777,8 @@ where operation, size, role, - operands.as_ptr() as *mut _, - count, + operands.as_mut_ptr(), + operands.len(), il.handle, ) }; @@ -546,6 +786,136 @@ where LowLevelILExpression::new(il, LowLevelExpressionIndex(expr_idx)) } +#[cfg(test)] +mod flag_write_tests { + use super::*; + use crate::architecture::CoreArchitecture; + use crate::headless::Session; + use binaryninjacore_sys::BNLowLevelILOperation::*; + + fn operand_values(operands: &[BNRegisterOrConstant]) -> Vec<(bool, u32, u64)> { + operands + .iter() + .map(|op| (op.constant, op.reg, op.value)) + .collect() + } + + #[test] + fn test_flag_write_op_round_trip() { + let _session = Session::new().expect("Failed to initialize session"); + let arch = CoreArchitecture::by_name("x86_64").unwrap(); + let reg = arch + .registers_all() + .into_iter() + .find(|r| r.name() == "rax") + .unwrap(); + let operands = [ + BNRegisterOrConstant { + constant: true, + reg: 0, + value: 0x7ff8_0000_0000_1234, + }, + BNRegisterOrConstant { + constant: false, + reg: reg.id().0, + value: 0, + }, + BNRegisterOrConstant { + constant: false, + reg: 0x8000_0007, + value: 0, + }, + ]; + let unary = [ + LLIL_FSQRT, + LLIL_FNEG, + LLIL_FABS, + LLIL_FLOAT_TO_INT, + LLIL_INT_TO_FLOAT, + LLIL_FLOAT_CONV, + LLIL_ROUND_TO_INT, + LLIL_FLOOR, + LLIL_CEIL, + LLIL_FTRUNC, + ]; + let binary = [ + LLIL_FADD, + LLIL_FSUB, + LLIL_FMUL, + LLIL_FDIV, + LLIL_CMP_E, + LLIL_CMP_NE, + LLIL_CMP_SLT, + LLIL_CMP_ULT, + LLIL_CMP_SLE, + LLIL_CMP_ULE, + LLIL_CMP_SGE, + LLIL_CMP_UGE, + LLIL_CMP_SGT, + LLIL_CMP_UGT, + LLIL_FCMP_E, + LLIL_FCMP_NE, + LLIL_FCMP_LT, + LLIL_FCMP_LE, + LLIL_FCMP_GE, + LLIL_FCMP_GT, + LLIL_FCMP_O, + LLIL_FCMP_UO, + LLIL_ADD, + LLIL_SUB, + LLIL_MULU_DP, + LLIL_DIVS_DP, + ]; + let ternary = [LLIL_ADC, LLIL_SBB, LLIL_RLC, LLIL_RRC]; + + for (count, operations) in [ + (0, &[LLIL_POP][..]), + (1, &unary), + (2, &binary), + (3, &ternary), + ] { + for &operation in operations { + for size in [1, 4, 8, 16] { + // Rotate through constants, architecture registers, and temporary registers. + for offset in 0..operands.len() { + let input: Vec<_> = operands + .iter() + .copied() + .cycle() + .skip(offset) + .take(count) + .collect(); + let decoded = + LowLevelILFlagWriteOp::from_op(&arch, size, operation, &input); + assert!( + !matches!(decoded, LowLevelILFlagWriteOp::Raw { .. }), + "{operation:?}" + ); + assert_eq!(decoded.size_and_op(), (size, operation)); + assert_eq!( + operand_values(&decoded.raw_operands()), + operand_values(&input), + "{operation:?}" + ); + } + } + } + } + + // Intrinsics have a variable number of arguments; unexpected shapes must also survive. + let input: Vec<_> = operands.iter().copied().cycle().take(9).collect(); + for (operation, input) in [(LLIL_INTRINSIC, input.as_slice()), (LLIL_FSUB, &input[..1])] { + let decoded = LowLevelILFlagWriteOp::from_op(&arch, 8, operation, input); + assert!(matches!(decoded, LowLevelILFlagWriteOp::Raw { .. })); + assert_eq!(decoded.size_and_op(), (8, operation)); + assert_eq!( + operand_values(&decoded.raw_operands()), + operand_values(input) + ); + } + } +} + pub fn get_default_flag_cond_llil<'func, A>( arch: &A, cond: FlagCondition, diff --git a/rust/tests/architecture_flag_write.rs b/rust/tests/architecture_flag_write.rs new file mode 100644 index 0000000000..6c8f96e333 --- /dev/null +++ b/rust/tests/architecture_flag_write.rs @@ -0,0 +1,304 @@ +use binaryninja::architecture::*; +use binaryninja::disassembly::InstructionTextToken; +use binaryninja::headless::Session; +use binaryninja::low_level_il::expression::ValueExpr; +use binaryninja::low_level_il::lifting::LowLevelILFlagWriteOp; +use binaryninja::low_level_il::{LowLevelILMutableExpression, LowLevelILMutableFunction}; +use binaryninja::Endianness; +use binaryninjacore_sys::*; +use std::ffi::CString; +use std::sync::Mutex; + +#[derive(Debug, PartialEq)] +struct CapturedOperation { + operation: BNLowLevelILOperation, + size: usize, + operands: Vec<(bool, u32, u64)>, +} + +impl CapturedOperation { + fn new( + operation: BNLowLevelILOperation, + size: usize, + operands: &[BNRegisterOrConstant], + ) -> Self { + Self { + operation, + size, + operands: operands + .iter() + .map(|op| (op.constant, op.reg, op.value)) + .collect(), + } + } + + fn decode(op: LowLevelILFlagWriteOp) -> Option { + match op { + LowLevelILFlagWriteOp::Fsub(size, left, right) => Some(Self::new( + BNLowLevelILOperation::LLIL_FSUB, + size, + &[left.into(), right.into()], + )), + LowLevelILFlagWriteOp::Pop(size) => { + Some(Self::new(BNLowLevelILOperation::LLIL_POP, size, &[])) + } + LowLevelILFlagWriteOp::Raw { + operation, + size, + operands, + } => Some(Self::new(operation, size, &operands)), + _ => None, + } + } +} + +struct FlagArchitecture { + handle: CustomArchitectureHandle, + core: CoreArchitecture, + base: CoreArchitecture, + calls: Mutex>>, +} + +impl AsRef for FlagArchitecture { + fn as_ref(&self) -> &CoreArchitecture { + &self.core + } +} + +impl Architecture for FlagArchitecture { + type Handle = CustomArchitectureHandle; + type RegisterInfo = CoreRegisterInfo; + type Register = CoreRegister; + type RegisterStackInfo = UnusedRegisterStack; + type RegisterStack = UnusedRegisterStack; + type Flag = CoreFlag; + type FlagWrite = CoreFlagWrite; + type FlagClass = CoreFlagClass; + type FlagGroup = CoreFlagGroup; + type Intrinsic = UnusedIntrinsic; + + fn endianness(&self) -> Endianness { + Endianness::LittleEndian + } + fn address_size(&self) -> usize { + 8 + } + fn default_integer_size(&self) -> usize { + 8 + } + fn instruction_alignment(&self) -> usize { + 1 + } + fn max_instr_len(&self) -> usize { + 1 + } + fn handle(&self) -> Self::Handle { + self.handle + } + + fn instruction_info(&self, _data: &[u8], _addr: u64) -> Option { + None + } + fn instruction_text( + &self, + _data: &[u8], + _addr: u64, + ) -> Option<(usize, Vec)> { + None + } + fn instruction_llil( + &self, + _data: &[u8], + _addr: u64, + _il: &LowLevelILMutableFunction, + ) -> Option<(usize, bool)> { + None + } + fn registers_all(&self) -> Vec { + Vec::new() + } + fn registers_full_width(&self) -> Vec { + Vec::new() + } + fn register_from_id(&self, _id: RegisterId) -> Option { + None + } + fn stack_pointer_reg(&self) -> Option { + None + } + + fn flags(&self) -> Vec { + self.base.flags() + } + fn flag_from_id(&self, id: FlagId) -> Option { + self.base.flag_from_id(id) + } + fn flag_write_types(&self) -> Vec { + self.base.flag_write_types() + } + fn flag_write_from_id(&self, id: FlagWriteId) -> Option { + self.base.flag_write_from_id(id) + } + + fn flag_write_llil<'a>( + &self, + _flag: CoreFlag, + _write: CoreFlagWrite, + op: LowLevelILFlagWriteOp, + il: &'a LowLevelILMutableFunction, + ) -> Option> { + self.calls + .lock() + .unwrap() + .push(CapturedOperation::decode(op)); + Some(il.const_int(0, 1)) + } +} + +struct PassThroughHook; +impl ArchitectureHook for PassThroughHook {} + +#[derive(Default)] +struct OverrideHook { + calls: Mutex>>, +} + +impl ArchitectureHook for OverrideHook { + fn flag_write_llil<'a>( + &self, + _base: &CoreArchitecture, + _flag: CoreFlag, + _write: CoreFlagWrite, + op: LowLevelILFlagWriteOp, + il: &'a LowLevelILMutableFunction, + ) -> Option> { + self.calls + .lock() + .unwrap() + .push(CapturedOperation::decode(op)); + Some(il.const_int(0, 2)) + } +} + +fn invoke_flag_write( + arch: CoreArchitecture, + write: CoreFlagWrite, + flag: CoreFlag, + operation: BNLowLevelILOperation, + size: usize, + operands: &[BNRegisterOrConstant], +) -> u64 { + let name = CString::new(arch.name()).unwrap(); + let mut operands = operands.to_vec(); + unsafe { + let arch = BNGetArchitectureByName(name.as_ptr()); + let il = BNCreateLowLevelILFunction(arch, std::ptr::null_mut()); + assert!(!il.is_null()); + // The core legitimately passes null for operations with no operands, such as POP. + let operands_ptr = if operands.is_empty() { + std::ptr::null_mut() + } else { + operands.as_mut_ptr() + }; + let root = BNGetArchitectureFlagWriteLowLevelIL( + arch, + operation, + size, + write.id().0, + flag.id().0, + operands_ptr, + operands.len(), + il, + ); + let result = BNGetLowLevelILByIndex(il, root); + BNFreeLowLevelILFunction(il); + assert_eq!(result.operation, BNLowLevelILOperation::LLIL_CONST); + result.operands[0] + } +} + +#[test] +fn test_flag_write_dispatch_and_forwarding() { + use BNLowLevelILOperation::*; + let _session = Session::new().expect("Failed to initialize session"); + let base = CoreArchitecture::by_name("x86_64").unwrap(); + let arch = register_architecture("rust_flag_write_operations", |handle, core| { + FlagArchitecture { + handle, + core, + base, + calls: Mutex::new(Vec::new()), + } + }); + let write = base + .flag_write_types() + .into_iter() + .find(|w| w.name() == "vcomi") + .unwrap(); + let flag = base.flags().into_iter().find(|f| f.name() == "s").unwrap(); + let constant = BNRegisterOrConstant { + constant: true, + reg: 0, + value: 1.25f64.to_bits(), + }; + let temporary = BNRegisterOrConstant { + constant: false, + reg: 0x8000_0007, + value: 0, + }; + let cases = [ + (LLIL_FSUB, 8, vec![constant, temporary]), + // Intrinsic flag writes start with the intrinsic ID and can have more than five operands. + (LLIL_INTRINSIC, 0, vec![constant; 9]), + (LLIL_POP, 4, vec![]), + // An unexpected operand count and an unresolved register are exposed through Raw. + (LLIL_FSUB, 8, vec![constant]), + ( + LLIL_FSUB, + 8, + vec![ + constant, + BNRegisterOrConstant { + constant: false, + reg: 12345, + value: 0, + }, + ], + ), + ]; + + // Ordinary architectures and stacked hooks must receive the same argument values. + for hooked in [false, true] { + if hooked { + register_architecture_hook(arch.core, PassThroughHook); + register_architecture_hook(arch.core, PassThroughHook); + } + for (operation, size, operands) in &cases { + assert_eq!( + invoke_flag_write(arch.core, write, flag, *operation, *size, operands), + 1 + ); + let captured = arch.calls.lock().unwrap().pop().unwrap(); + assert_eq!( + captured, + Some(CapturedOperation::new(*operation, *size, operands)) + ); + } + } + + let hook = register_architecture_hook(arch.core, OverrideHook::default()); + // Another default hook must preserve the override as well as all arguments. + register_architecture_hook(arch.core, PassThroughHook); + for (operation, size, operands) in &cases { + assert_eq!( + invoke_flag_write(arch.core, write, flag, *operation, *size, operands), + 2 + ); + let captured = hook.calls.lock().unwrap().pop().unwrap(); + assert_eq!( + captured, + Some(CapturedOperation::new(*operation, *size, operands)) + ); + assert!(arch.calls.lock().unwrap().is_empty()); + } +} diff --git a/rust/tests/architecture_hook.rs b/rust/tests/architecture_hook.rs index 1f9d856abe..71caccc3cd 100644 --- a/rust/tests/architecture_hook.rs +++ b/rust/tests/architecture_hook.rs @@ -142,7 +142,7 @@ fn flag_write_il( reg: 0, value, }); - // Use the core API because LLIL_FSUB has no LowLevelILFlagWriteOp variant. + // Exercise decoding from the raw C callback as well as forwarding back to the core. unsafe { let arch = BNGetArchitectureByName(name.as_ptr()); let il = BNCreateLowLevelILFunction(arch, std::ptr::null_mut()); @@ -221,15 +221,13 @@ fn check_floating_point_flags() { } register_architecture_hook(arch, TypedFlagHook); - // An unsupported operation delegates through both hooks with its raw arguments intact. - for (flag, expected) in &originals { - assert_eq!( - flag_write_il(arch, write, *flag, LLIL_FSUB), - *expected, - "{name}" - ); + // Floating-point operations must reach the typed hook override too. + for (flag, _) in &originals { + let (root, expressions) = flag_write_il(arch, write, *flag, LLIL_FSUB); + assert_eq!(expressions[root].0, LLIL_CONST, "{name}"); + assert_eq!(expressions[root].3[0], 1, "{name}"); } - // Supported operations must still call the typed hook override. + // Existing integer operations still reach the same callback. let (root, expressions) = flag_write_il(arch, write, originals[0].0, LLIL_ADD); assert_eq!(expressions[root].0, LLIL_CONST); assert_eq!(expressions[root].3[0], 1); From c9e91c48b666052c64d975b5b1ee93687d49010d Mon Sep 17 00:00:00 2001 From: Brandon Miller Date: Tue, 20 Oct 2026 14:49:19 -0400 Subject: [PATCH 6/8] Preserve function-context destruction through stacked Rust hooks Install an explicit freeFunctionArchContext callback that forwards to the stored previous implementation. This avoids relying on the core fallback chain, which is lost when another hook copies the callback table. --- rust/src/architecture/hook.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/rust/src/architecture/hook.rs b/rust/src/architecture/hook.rs index ab8ad55262..39c5b41162 100644 --- a/rust/src/architecture/hook.rs +++ b/rust/src/architecture/hook.rs @@ -189,10 +189,22 @@ pub fn register_architecture_hook( base: CoreArchitecture, hook: H, ) -> &'static H { + unsafe extern "C" fn free_function_arch_context( + context: *mut c_void, + function_context: *mut c_void, + ) { + let arch = unsafe { &*context.cast::>() }; + // Each hook must explicitly forward cleanup: copied callback tables do not retain + // the core's fallback chain when another hook is registered. + unsafe { BNArchitectureFreeFunctionArchContext(arch.base.handle, function_context) }; + } + let registered = super::register_architecture_impl( ArchitectureRegistration::Hook(base), |handle, base| HookArchitecture { hook, base, handle }, - |_| {}, + |callbacks| { + callbacks.freeFunctionArchContext = Some(free_function_arch_context::); + }, ); ®istered.hook } From 7acd3390ed38d8fd44f45d801e1f3002f80c61a3 Mon Sep 17 00:00:00 2001 From: Brandon Miller Date: Tue, 20 Oct 2026 14:57:52 -0400 Subject: [PATCH 7/8] Preserve decoded length when Rust LLIL lifting fails Forward the native length and success result together through CoreArchitecture::instruction_llil, preserving length updates when lifting returns false. --- rust/src/architecture.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/rust/src/architecture.rs b/rust/src/architecture.rs index 56daea3bca..2c6a33a0f5 100644 --- a/rust/src/architecture.rs +++ b/rust/src/architecture.rs @@ -239,11 +239,11 @@ pub trait Architecture: 'static + Sized + AsRef { self.instruction_text(data, addr) } - // TODO: Why do we need to return a boolean here? Does `None` not represent the same thing? /// Appends arbitrary low-level il instructions to `il`. /// - /// If `None` is returned, no instructions were appended and the data is invalid. If `Some` is returned, - /// the instructions consumed length is returned (necessary for variable length instruction decoding). + /// Returns the decoded length and whether lifting succeeded. `Some((length, false))` preserves + /// the decoded length even when lifting fails. `None` reports failure without updating the + /// caller's length. fn instruction_llil( &self, data: &[u8], @@ -996,11 +996,7 @@ impl Architecture for CoreArchitecture { ) }; - if !success { - None - } else { - Some((size, true)) - } + Some((size, success)) } /// Performs basic block recovery and commits the results to the function analysis. From ef08cf32b724b956a97f27e193297ad314e3a791 Mon Sep 17 00:00:00 2001 From: Brandon Miller Date: Tue, 20 Oct 2026 15:13:31 -0400 Subject: [PATCH 8/8] Preserve instruction-token metadata through Rust architecture hooks Store native token fields directly and serialize them without reconstructing metadata from the typed kind. Preserve widths, operands, values, type names, and floating-point payload bits, matching C++ behavior. Keep typed constructors and expose kind() as a convenience view. Add raw C token comparisons before and after stacked empty hooks, covering native x86, synthetic metadata, and contextual disassembly. --- rust/src/disassembly.rs | 110 ++++++++++++++++++++----------------- rust/src/lib.rs | 1 + rust/tests/architecture.rs | 54 ++++-------------- 3 files changed, 71 insertions(+), 94 deletions(-) diff --git a/rust/src/disassembly.rs b/rust/src/disassembly.rs index 9aa790cd87..23d5824161 100644 --- a/rust/src/disassembly.rs +++ b/rust/src/disassembly.rs @@ -23,7 +23,7 @@ use crate::low_level_il as llil; use crate::medium_level_il as mlil; use crate::string::IntoCStr; use crate::string::{raw_to_string, strings_to_string_list, BnString}; -use crate::{high_level_il as hlil, BN_INVALID_EXPR}; +use crate::{high_level_il as hlil, BN_INVALID_EXPR, BN_INVALID_OPERAND}; use crate::rc::*; @@ -267,46 +267,67 @@ pub struct InstructionTextToken { pub confidence: u8, pub context: InstructionTextTokenContext, pub expr_index: Option, - pub kind: InstructionTextTokenKind, + /// Native token type. Use [`Self::kind`] for the typed interpretation. + pub type_: InstructionTextTokenType, + /// Native value, including the unmodified payload bits of floating-point tokens. + pub value: u64, + /// Display width in columns. Zero selects the UTF-8 byte length of `text`. + pub width: u64, + pub size: usize, + pub operand: usize, + pub type_names: Vec, } impl InstructionTextToken { pub(crate) fn from_raw(value: &BNInstructionTextToken) -> Self { + let text = raw_to_string(value.text).unwrap(); + let type_names = if value.namesCount == 0 { + Vec::new() + } else { + let names = unsafe { std::slice::from_raw_parts(value.typeNames, value.namesCount) }; + names + .iter() + .filter_map(|&name| raw_to_string(name)) + .collect() + }; Self { address: value.address, - text: raw_to_string(value.text).unwrap(), + width: if value.width == 0 { + text.len() as u64 + } else { + value.width + }, + text, confidence: value.confidence, context: value.context.into(), expr_index: match value.exprIndex { BN_INVALID_EXPR => None, index => Some(index), }, - kind: InstructionTextTokenKind::from_raw(value), + type_: value.type_, + value: value.value, + size: value.size, + operand: value.operand, + type_names, } } pub(crate) fn into_raw(value: Self) -> BNInstructionTextToken { let bn_text = BnString::new(value.text); - // These can be gathered from value.kind - let kind_value = value.kind.try_value().unwrap_or(0); - let operand = value.kind.try_operand().unwrap_or(0); - let size = value.kind.try_size().unwrap_or(0); - let type_names = value.kind.try_type_names().unwrap_or_default(); BNInstructionTextToken { - type_: value.kind.into(), + type_: value.type_, // NOTE: Expected to be freed with `InstructionTextToken::free_raw`. text: BnString::into_raw(bn_text), - value: kind_value, - // TODO: Where is this even used? - width: 0, - size, - operand, + value: value.value, + width: value.width, + size: value.size, + operand: value.operand, context: value.context.into(), confidence: value.confidence, address: value.address, // NOTE: Expected to be freed with `InstructionTextToken::free_raw`. - typeNames: strings_to_string_list(&type_names), - namesCount: type_names.len(), + typeNames: strings_to_string_list(&value.type_names), + namesCount: value.type_names.len(), exprIndex: value.expr_index.unwrap_or(BN_INVALID_EXPR), } } @@ -323,14 +344,7 @@ impl InstructionTextToken { /// You most likely want to call [`InstructionTextToken::new_with_address`], while also adjusting /// the [`InstructionTextToken::expr_index`] field where applicable. pub fn new(text: impl Into, kind: InstructionTextTokenKind) -> Self { - Self { - address: 0, - text: text.into(), - confidence: MAX_CONFIDENCE, - context: InstructionTextTokenContext::Normal, - expr_index: None, - kind, - } + Self::new_with_address(0, text, kind) } pub fn new_with_address( @@ -338,15 +352,27 @@ impl InstructionTextToken { text: impl Into, kind: InstructionTextTokenKind, ) -> Self { + let text = text.into(); Self { address, - text: text.into(), + width: text.len() as u64, + text, confidence: MAX_CONFIDENCE, context: InstructionTextTokenContext::Normal, expr_index: None, - kind, + value: kind.try_value().unwrap_or(0), + size: kind.try_size().unwrap_or(0), + operand: kind.try_operand().unwrap_or(BN_INVALID_OPERAND), + type_names: kind.try_type_names().unwrap_or_default(), + type_: kind.into(), } } + + /// Interprets the native fields as a typed token kind without discarding ancillary metadata. + /// Mutate the token's fields directly to change its metadata. + pub fn kind(&self) -> InstructionTextTokenKind { + InstructionTextTokenKind::from_token(self) + } } impl Display for InstructionTextToken { @@ -600,7 +626,7 @@ pub enum InstructionTextTokenKind { } impl InstructionTextTokenKind { - pub(crate) fn from_raw(value: &BNInstructionTextToken) -> Self { + fn from_token(value: &InstructionTextToken) -> Self { match value.type_ { BNInstructionTextTokenType::TextToken => Self::Text, BNInstructionTextTokenType::InstructionToken => Self::Instruction, @@ -625,7 +651,7 @@ impl InstructionTextTokenKind { BNInstructionTextTokenType::BeginMemoryOperandToken => Self::BeginMemoryOperand, BNInstructionTextTokenType::EndMemoryOperandToken => Self::EndMemoryOperand, BNInstructionTextTokenType::FloatingPointToken => Self::FloatingPoint { - value: value.value as f64, + value: f64::from_bits(value.value), size: match value.size { 0 => None, size => Some(size), @@ -652,7 +678,7 @@ impl InstructionTextTokenKind { Self::HexDumpText { width: value.value } } BNInstructionTextTokenType::OpcodeToken => Self::Opcode, - BNInstructionTextTokenType::StringToken => match value.context { + BNInstructionTextTokenType::StringToken => match value.context.into() { BNInstructionTextTokenContext::StringReferenceTokenContext | BNInstructionTextTokenContext::StringDisplayTokenContext => { match value.value { @@ -683,24 +709,14 @@ impl InstructionTextTokenKind { BNInstructionTextTokenType::TypeNameToken => Self::TypeName, BNInstructionTextTokenType::FieldNameToken => Self::FieldName { offset: value.value, - type_names: { - // NOTE: Do not need to free, this is a part of the From<&> impl - let raw_names = - unsafe { std::slice::from_raw_parts(value.typeNames, value.namesCount) }; - raw_names.iter().filter_map(|&r| raw_to_string(r)).collect() - }, + type_names: value.type_names.clone(), }, BNInstructionTextTokenType::NameSpaceToken => Self::NameSpace, BNInstructionTextTokenType::NameSpaceSeparatorToken => Self::NameSpaceSeparator, BNInstructionTextTokenType::TagToken => Self::Tag, BNInstructionTextTokenType::StructOffsetToken => Self::StructOffset { offset: value.value, - type_names: { - // NOTE: Do not need to free, this is a part of the From<&> impl - let raw_names = - unsafe { std::slice::from_raw_parts(value.typeNames, value.namesCount) }; - raw_names.iter().filter_map(|&r| raw_to_string(r)).collect() - }, + type_names: value.type_names.clone(), }, BNInstructionTextTokenType::StructOffsetByteValueToken => Self::StructOffsetByteValue, BNInstructionTextTokenType::StructureHexDumpTextToken => { @@ -722,13 +738,7 @@ impl InstructionTextTokenKind { BNInstructionTextTokenType::UnknownMemoryToken => Self::UnknownMemory, BNInstructionTextTokenType::EnumerationMemberToken => Self::EnumerationMember { value: value.value, - type_id: { - // NOTE: Type id comes from value.typeNames, it should be the first one (hence the .next) - // NOTE: Do not need to free, this is a part of the From<&> impl - let raw_names = - unsafe { std::slice::from_raw_parts(value.typeNames, value.namesCount) }; - raw_names.iter().filter_map(|&r| raw_to_string(r)).next() - }, + type_id: value.type_names.first().cloned(), }, BNInstructionTextTokenType::OperationToken => Self::Operation, BNInstructionTextTokenType::BaseStructureNameToken => Self::BaseStructureName, @@ -790,7 +800,7 @@ impl InstructionTextTokenKind { InstructionTextTokenKind::Integer { value, .. } => Some(*value), InstructionTextTokenKind::PossibleAddress { value, .. } => Some(*value), InstructionTextTokenKind::PossibleValue { value, .. } => Some(*value), - InstructionTextTokenKind::FloatingPoint { value, .. } => Some(*value as u64), + InstructionTextTokenKind::FloatingPoint { value, .. } => Some(value.to_bits()), InstructionTextTokenKind::CodeRelativeAddress { value, .. } => Some(*value), InstructionTextTokenKind::ArgumentName { value, .. } => Some(*value), InstructionTextTokenKind::HexDumpByteValue { value, .. } => Some(*value as u64), diff --git a/rust/src/lib.rs b/rust/src/lib.rs index bf39e8a7e1..49b5a9961e 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -115,6 +115,7 @@ pub use binaryninjacore_sys::BNILBranchDependence as ILBranchDependence; pub const BN_FULL_CONFIDENCE: u8 = u8::MAX; pub const BN_INVALID_EXPR: usize = usize::MAX; +pub const BN_INVALID_OPERAND: usize = u32::MAX as usize; /// Fuzzy match a string against a query string. Returns a score that is higher for /// a more confident match, or `None` if the query does not match the target string. diff --git a/rust/tests/architecture.rs b/rust/tests/architecture.rs index 4d195b69b1..28360f77ae 100644 --- a/rust/tests/architecture.rs +++ b/rust/tests/architecture.rs @@ -1,7 +1,5 @@ use binaryninja::architecture::{Architecture, CoreArchitecture}; -use binaryninja::disassembly::{ - InstructionTextToken, InstructionTextTokenContext, InstructionTextTokenKind, -}; +use binaryninja::disassembly::{InstructionTextToken, InstructionTextTokenKind}; use binaryninja::headless::Session; #[test] @@ -38,51 +36,19 @@ fn test_architecture_disassembly() { .expect("Failed to disassemble instruction"); assert_eq!(instr_len, 7); - let expected_tokens: Vec = vec![ - InstructionTextToken { - address: 0, - text: "mov".to_string(), - confidence: 255, - context: InstructionTextTokenContext::Normal, - expr_index: None, - kind: InstructionTextTokenKind::Instruction, - }, - InstructionTextToken { - address: 0, - text: " ".to_string(), - confidence: 255, - context: InstructionTextTokenContext::Normal, - expr_index: None, - kind: InstructionTextTokenKind::Text, - }, - InstructionTextToken { - address: 0, - text: "rax".to_string(), - confidence: 255, - context: InstructionTextTokenContext::Normal, - expr_index: None, - kind: InstructionTextTokenKind::Register, - }, - InstructionTextToken { - address: 0, - text: ", ".to_string(), - confidence: 255, - context: InstructionTextTokenContext::Normal, - expr_index: None, - kind: InstructionTextTokenKind::OperandSeparator, - }, - InstructionTextToken { - address: 0, - text: "0x10".to_string(), - confidence: 255, - context: InstructionTextTokenContext::Normal, - expr_index: None, - kind: InstructionTextTokenKind::PossibleAddress { + let expected_tokens = vec![ + InstructionTextToken::new("mov", InstructionTextTokenKind::Instruction), + InstructionTextToken::new(" ", InstructionTextTokenKind::Text), + InstructionTextToken::new("rax", InstructionTextTokenKind::Register), + InstructionTextToken::new(", ", InstructionTextTokenKind::OperandSeparator), + InstructionTextToken::new( + "0x10", + InstructionTextTokenKind::PossibleAddress { value: 16, size: Some(8), operand: None, }, - }, + ), ]; assert_eq!(tokens, expected_tokens);