Skip to content

Commit

Permalink
Ignore PlainText segments when diffing
Browse files Browse the repository at this point in the history
  • Loading branch information
encounter committed Oct 31, 2024
1 parent c5da7f7 commit 7f14b68
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 12 deletions.
15 changes: 5 additions & 10 deletions objdiff-core/src/diff/code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,15 +299,10 @@ fn compare_ins(
) -> Result<InsDiffResult> {
let mut result = InsDiffResult::default();
if let (Some(left_ins), Some(right_ins)) = (&left.ins, &right.ins) {
if left_ins.args.len() != right_ins.args.len()
|| left_ins.op != right_ins.op
// Check if any PlainText segments differ (punctuation and spacing)
// This indicates a more significant difference than a simple arg mismatch
|| !left_ins.args.iter().zip(&right_ins.args).all(|(a, b)| match (a, b) {
(ObjInsArg::PlainText(l), ObjInsArg::PlainText(r)) => l == r,
_ => true,
})
{
// Count only non-PlainText args
let left_args_count = left_ins.iter_args().count();
let right_args_count = right_ins.iter_args().count();
if left_args_count != right_args_count || left_ins.op != right_ins.op {
// Totally different op
result.kind = ObjInsDiffKind::Replace;
state.diff_count += 1;
Expand All @@ -318,7 +313,7 @@ fn compare_ins(
result.kind = ObjInsDiffKind::OpMismatch;
state.diff_count += 1;
}
for (a, b) in left_ins.args.iter().zip(&right_ins.args) {
for (a, b) in left_ins.iter_args().zip(right_ins.iter_args()) {
if arg_eq(config, left_obj, right_obj, a, b, left, right) {
result.left_args_diff.push(None);
result.right_args_diff.push(None);
Expand Down
6 changes: 5 additions & 1 deletion objdiff-core/src/diff/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,27 +58,31 @@ pub fn display_diff<E>(
cb(DiffText::Spacing(4))?;
}
cb(DiffText::Opcode(&ins.mnemonic, ins.op))?;
let mut arg_diff_idx = 0; // non-PlainText index
for (i, arg) in ins.args.iter().enumerate() {
if i == 0 {
cb(DiffText::Spacing(1))?;
}
let diff = ins_diff.arg_diff.get(i).and_then(|o| o.as_ref());
let diff = ins_diff.arg_diff.get(arg_diff_idx).and_then(|o| o.as_ref());
match arg {
ObjInsArg::PlainText(s) => {
cb(DiffText::Basic(s))?;
}
ObjInsArg::Arg(v) => {
cb(DiffText::Argument(v, diff))?;
arg_diff_idx += 1;
}
ObjInsArg::Reloc => {
display_reloc_name(ins.reloc.as_ref().unwrap(), &mut cb, diff)?;
arg_diff_idx += 1;
}
ObjInsArg::BranchDest(dest) => {
if let Some(dest) = dest.checked_sub(base_addr) {
cb(DiffText::BranchDest(dest, diff))?;
} else {
cb(DiffText::Basic("<unknown>"))?;
}
arg_diff_idx += 1;
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion objdiff-core/src/diff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ pub struct ObjInsDiff {
pub branch_from: Option<ObjInsBranchFrom>,
/// Branches to instruction
pub branch_to: Option<ObjInsBranchTo>,
/// Arg diffs
/// Arg diffs (only contains non-PlainText args)
pub arg_diff: Vec<Option<ObjInsArgDiff>>,
}

Expand Down
11 changes: 11 additions & 0 deletions objdiff-core/src/obj/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ pub enum ObjInsArg {
}

impl ObjInsArg {
#[inline]
pub fn is_plain_text(&self) -> bool { matches!(self, ObjInsArg::PlainText(_)) }

pub fn loose_eq(&self, other: &ObjInsArg) -> bool {
match (self, other) {
(ObjInsArg::Arg(a), ObjInsArg::Arg(b)) => a.loose_eq(b),
Expand Down Expand Up @@ -112,6 +115,14 @@ pub struct ObjIns {
pub orig: Option<String>,
}

impl ObjIns {
/// Iterate over non-PlainText arguments.
#[inline]
pub fn iter_args(&self) -> impl DoubleEndedIterator<Item = &ObjInsArg> {
self.args.iter().filter(|a| !a.is_plain_text())
}
}

#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash, Default)]
pub enum ObjSymbolKind {
#[default]
Expand Down

0 comments on commit 7f14b68

Please sign in to comment.