Skip to content

Fix data flow analysis for multiple text sections #220

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions objdiff-core/src/diff/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
Expand Down Expand Up @@ -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.
Expand Down
16 changes: 15 additions & 1 deletion objdiff-core/src/obj/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ pub struct Object {
pub path: Option<std::path::PathBuf>,
#[cfg(feature = "std")]
pub timestamp: Option<filetime::FileTime>,
pub flow_analysis_results: BTreeMap<u64, Box<dyn FlowAnalysisResult>>,
flow_analysis_results: BTreeMap<u64, Box<dyn FlowAnalysisResult>>,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep this pub, otherwise Object isn't constructible outside of the crate

}

impl Default for Object {
Expand Down Expand Up @@ -328,6 +328,20 @@ 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> {
let key = symbol.section.unwrap_or_default() as u64 + symbol.address;
self.flow_analysis_results.get(&key).map(|result| result.as_ref())
}

pub fn add_flow_analysis_result(
&mut self,
symbol: &Symbol,
result: Box<dyn FlowAnalysisResult>,
) {
let key = symbol.section.unwrap_or_default() as u64 + symbol.address;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This key seems like it could easily overlap, the section indices are small numbers

self.flow_analysis_results.insert(key, result);
}

pub fn has_flow_analysis_result(&self) -> bool { !self.flow_analysis_results.is_empty() }
}

Expand Down
17 changes: 12 additions & 5 deletions objdiff-core/src/obj/read.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use alloc::{
boxed::Box,
collections::BTreeMap,
format,
string::{String, ToString},
Expand All @@ -13,8 +14,8 @@ 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},
Expand Down Expand Up @@ -439,6 +440,7 @@ fn perform_data_flow_analysis(obj: &mut Object, config: &DiffObjConfig) -> Resul
}

let mut generated_relocations = Vec::<(usize, Vec<Relocation>)>::new();
let mut generated_flow_results = Vec::<(Symbol, Box<dyn FlowAnalysisResult>)>::new();
for (section_index, section) in obj.sections.iter().enumerate() {
if section.kind != SectionKind::Code {
continue;
Expand Down Expand Up @@ -474,12 +476,17 @@ 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, &section.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, &section.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);
}
Expand Down