From 4a787423f4e3a333bfcbc063c667863127a48d27 Mon Sep 17 00:00:00 2001 From: Mark Langen Date: Sat, 5 Jul 2025 21:42:15 -0700 Subject: [PATCH 1/5] Fix data flow analysis for multiple text sections * Data flow analysis results were only keyed by the symbol (function) address. That doen't work if there are multiple text sections, the result from the first function in one section will stomp the result from the first function in another because both have address zero. * Remove the ambiguity by keying off of the section address as well. --- objdiff-core/src/diff/display.rs | 2 +- objdiff-core/src/obj/mod.rs | 12 +++++++++++- objdiff-core/src/obj/read.rs | 11 ++++++++--- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/objdiff-core/src/diff/display.rs b/objdiff-core/src/diff/display.rs index 439847d..0f333a7 100644 --- a/objdiff-core/src/diff/display.rs +++ b/objdiff-core/src/diff/display.rs @@ -189,7 +189,7 @@ pub fn display_row( let mut arg_idx = 0; let mut displayed_relocation = false; let analysis_result = if diff_config.show_data_flow { - obj.flow_analysis_results.get(&resolved.symbol.address) + obj.get_flow_analysis_result(&resolved.symbol) } else { None }; diff --git a/objdiff-core/src/obj/mod.rs b/objdiff-core/src/obj/mod.rs index 94f0228..3c00a31 100644 --- a/objdiff-core/src/obj/mod.rs +++ b/objdiff-core/src/obj/mod.rs @@ -273,7 +273,7 @@ pub struct Object { pub path: Option, #[cfg(feature = "std")] pub timestamp: Option, - pub flow_analysis_results: BTreeMap>, + flow_analysis_results: BTreeMap>, } impl Default for Object { @@ -328,6 +328,16 @@ impl Object { self.symbols.iter().position(|symbol| symbol.section.is_some() && symbol.name == name) } + pub fn get_flow_analysis_result(&self, symbol: &Symbol) -> Option<&Box> { + let key = symbol.section.unwrap_or_default() as u64 + symbol.address; + self.flow_analysis_results.get(&key) + } + + pub fn add_flow_analysis_result(&mut self, symbol: &Symbol, result: Box) { + let key = symbol.section.unwrap_or_default() as u64 + symbol.address; + self.flow_analysis_results.insert(key, result); + } + pub fn has_flow_analysis_result(&self) -> bool { !self.flow_analysis_results.is_empty() } } diff --git a/objdiff-core/src/obj/read.rs b/objdiff-core/src/obj/read.rs index f693487..bce3b8b 100644 --- a/objdiff-core/src/obj/read.rs +++ b/objdiff-core/src/obj/read.rs @@ -8,6 +8,7 @@ use core::{cmp::Ordering, num::NonZeroU64}; use anyhow::{Context, Result, anyhow, bail, ensure}; use object::{Object as _, ObjectSection as _, ObjectSymbol as _}; +use crate::obj::FlowAnalysisResult; use crate::{ arch::{Arch, new_arch}, @@ -439,6 +440,7 @@ fn perform_data_flow_analysis(obj: &mut Object, config: &DiffObjConfig) -> Resul } let mut generated_relocations = Vec::<(usize, Vec)>::new(); + let mut generated_flow_results = Vec::<(Symbol, Box)>::new(); for (section_index, section) in obj.sections.iter().enumerate() { if section.kind != SectionKind::Code { continue; @@ -474,12 +476,15 @@ fn perform_data_flow_analysis(obj: &mut Object, config: &DiffObjConfig) -> Resul // Optional full data flow analysis if config.analyze_data_flow { - obj.arch.data_flow_analysis(obj, symbol, code, §ion.relocations).and_then( - |flow_result| obj.flow_analysis_results.insert(symbol.address, flow_result), - ); + if let Some(flow_result) = obj.arch.data_flow_analysis(obj, symbol, code, §ion.relocations) { + generated_flow_results.push((symbol.clone(), flow_result)); + } } } } + for (symbol, flow_result) in generated_flow_results { + obj.add_flow_analysis_result(&symbol, flow_result); + } for (section_index, mut relocations) in generated_relocations { obj.sections[section_index].relocations.append(&mut relocations); } From d0aa51b07b2491659659394daeff444aed12c900 Mon Sep 17 00:00:00 2001 From: Mark Langen Date: Sat, 5 Jul 2025 21:49:25 -0700 Subject: [PATCH 2/5] Formatting --- objdiff-core/src/obj/mod.rs | 11 +++++++++-- objdiff-core/src/obj/read.rs | 9 +++++---- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/objdiff-core/src/obj/mod.rs b/objdiff-core/src/obj/mod.rs index 3c00a31..99a451f 100644 --- a/objdiff-core/src/obj/mod.rs +++ b/objdiff-core/src/obj/mod.rs @@ -328,12 +328,19 @@ impl Object { self.symbols.iter().position(|symbol| symbol.section.is_some() && symbol.name == name) } - pub fn get_flow_analysis_result(&self, symbol: &Symbol) -> Option<&Box> { + pub fn get_flow_analysis_result( + &self, + symbol: &Symbol, + ) -> Option<&Box> { let key = symbol.section.unwrap_or_default() as u64 + symbol.address; self.flow_analysis_results.get(&key) } - pub fn add_flow_analysis_result(&mut self, symbol: &Symbol, result: Box) { + pub fn add_flow_analysis_result( + &mut self, + symbol: &Symbol, + result: Box, + ) { let key = symbol.section.unwrap_or_default() as u64 + symbol.address; self.flow_analysis_results.insert(key, result); } diff --git a/objdiff-core/src/obj/read.rs b/objdiff-core/src/obj/read.rs index bce3b8b..205ee81 100644 --- a/objdiff-core/src/obj/read.rs +++ b/objdiff-core/src/obj/read.rs @@ -8,14 +8,13 @@ use core::{cmp::Ordering, num::NonZeroU64}; use anyhow::{Context, Result, anyhow, bail, ensure}; use object::{Object as _, ObjectSection as _, ObjectSymbol as _}; -use crate::obj::FlowAnalysisResult; use crate::{ arch::{Arch, new_arch}, diff::DiffObjConfig, obj::{ - Object, Relocation, RelocationFlags, Section, SectionData, SectionFlag, SectionKind, - Symbol, SymbolFlag, SymbolKind, + FlowAnalysisResult, Object, Relocation, RelocationFlags, Section, SectionData, SectionFlag, + SectionKind, Symbol, SymbolFlag, SymbolKind, split_meta::{SPLITMETA_SECTION, SplitMeta}, }, util::{align_data_slice_to, align_u64_to, read_u16, read_u32}, @@ -476,7 +475,9 @@ fn perform_data_flow_analysis(obj: &mut Object, config: &DiffObjConfig) -> Resul // Optional full data flow analysis if config.analyze_data_flow { - if let Some(flow_result) = obj.arch.data_flow_analysis(obj, symbol, code, §ion.relocations) { + if let Some(flow_result) = + obj.arch.data_flow_analysis(obj, symbol, code, §ion.relocations) + { generated_flow_results.push((symbol.clone(), flow_result)); } } From e157d5f62bdbd2095a78f6ac3525c348d8b1b399 Mon Sep 17 00:00:00 2001 From: Mark Langen Date: Sat, 5 Jul 2025 21:58:51 -0700 Subject: [PATCH 3/5] Satisfy wasm build --- objdiff-core/src/obj/read.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/objdiff-core/src/obj/read.rs b/objdiff-core/src/obj/read.rs index 205ee81..6034e1c 100644 --- a/objdiff-core/src/obj/read.rs +++ b/objdiff-core/src/obj/read.rs @@ -1,4 +1,5 @@ use alloc::{ + boxed::Box, collections::BTreeMap, format, string::{String, ToString}, From 61820cbfcdc65fa75509331d801f852a74f5e407 Mon Sep 17 00:00:00 2001 From: Mark Langen Date: Sat, 5 Jul 2025 22:06:50 -0700 Subject: [PATCH 4/5] Clippy --- objdiff-core/src/diff/display.rs | 4 ++-- objdiff-core/src/obj/mod.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/objdiff-core/src/diff/display.rs b/objdiff-core/src/diff/display.rs index 0f333a7..d782018 100644 --- a/objdiff-core/src/diff/display.rs +++ b/objdiff-core/src/diff/display.rs @@ -189,7 +189,7 @@ pub fn display_row( let mut arg_idx = 0; let mut displayed_relocation = false; let analysis_result = if diff_config.show_data_flow { - obj.get_flow_analysis_result(&resolved.symbol) + obj.get_flow_analysis_result(resolved.symbol) } else { None }; @@ -217,7 +217,7 @@ pub fn display_row( } let data_flow_value = analysis_result.and_then(|result| - result.as_ref().get_argument_value_at_address( + result.get_argument_value_at_address( ins_ref.address, (arg_idx - 1) as u8)); match (arg, data_flow_value, resolved.ins_ref.branch_dest) { // If we have a flow analysis result, always use that over anything else. diff --git a/objdiff-core/src/obj/mod.rs b/objdiff-core/src/obj/mod.rs index 99a451f..1bc1edb 100644 --- a/objdiff-core/src/obj/mod.rs +++ b/objdiff-core/src/obj/mod.rs @@ -331,9 +331,9 @@ impl Object { pub fn get_flow_analysis_result( &self, symbol: &Symbol, - ) -> Option<&Box> { + ) -> Option<&dyn FlowAnalysisResult> { let key = symbol.section.unwrap_or_default() as u64 + symbol.address; - self.flow_analysis_results.get(&key) + self.flow_analysis_results.get(&key).map(|result| result.as_ref()) } pub fn add_flow_analysis_result( From 391cb19fa69b1e29c40798de71f71421a518e0cd Mon Sep 17 00:00:00 2001 From: Mark Langen Date: Sat, 5 Jul 2025 22:09:54 -0700 Subject: [PATCH 5/5] Formatting again --- objdiff-core/src/obj/mod.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/objdiff-core/src/obj/mod.rs b/objdiff-core/src/obj/mod.rs index 1bc1edb..9a3e3a7 100644 --- a/objdiff-core/src/obj/mod.rs +++ b/objdiff-core/src/obj/mod.rs @@ -328,10 +328,7 @@ impl Object { self.symbols.iter().position(|symbol| symbol.section.is_some() && symbol.name == name) } - pub fn get_flow_analysis_result( - &self, - symbol: &Symbol, - ) -> Option<&dyn FlowAnalysisResult> { + pub fn get_flow_analysis_result(&self, symbol: &Symbol) -> Option<&dyn FlowAnalysisResult> { let key = symbol.section.unwrap_or_default() as u64 + symbol.address; self.flow_analysis_results.get(&key).map(|result| result.as_ref()) }