diff --git a/objdiff-cli/src/cmd/diff.rs b/objdiff-cli/src/cmd/diff.rs index 062e0b89..4e35b3f8 100644 --- a/objdiff-cli/src/cmd/diff.rs +++ b/objdiff-cli/src/cmd/diff.rs @@ -198,8 +198,8 @@ fn run_oneshot( .transpose()?; let result = diff::diff_objs(target.as_ref(), base.as_ref(), None, &diff_config, &mapping_config)?; - let left = target.as_ref().and_then(|o| result.left.as_ref().map(|d| (o, d))); - let right = base.as_ref().and_then(|o| result.right.as_ref().map(|d| (o, d))); + let left = target.as_ref().zip(result.left.as_ref()); + let right = base.as_ref().zip(result.right.as_ref()); let diff_result = DiffResult::new(left, right, &diff_config)?; write_output(&diff_result, Some(output), output_format)?; Ok(()) diff --git a/objdiff-core/config-schema.json b/objdiff-core/config-schema.json index b1bca011..cbe1703a 100644 --- a/objdiff-core/config-schema.json +++ b/objdiff-core/config-schema.json @@ -117,6 +117,51 @@ "name": "Combine text sections", "description": "Combines all text sections into one." }, + { + "id": "preferredStringEncoding", + "type": "choice", + "default": "auto", + "name": "Preferred string encoding", + "description": "Which encoding to use when diffing string literals used in functions.", + "items": [ + { + "value": "auto", + "name": "Auto" + }, + { + "value": "ascii", + "name": "ASCII" + }, + { + "value": "utf_8", + "name": "UTF-8" + }, + { + "value": "shift_jis", + "name": "Shift JIS" + }, + { + "value": "windows_1252", + "name": "Windows-1252" + }, + { + "value": "euc_jp", + "name": "EUC-JP" + }, + { + "value": "big5", + "name": "Big5" + }, + { + "value": "utf_16be", + "name": "UTF-16BE" + }, + { + "value": "utf_16le", + "name": "UTF-16LE" + } + ] + }, { "id": "arm.archVersion", "type": "choice", @@ -338,6 +383,7 @@ "id": "general", "name": "General", "properties": [ + "preferredStringEncoding", "functionRelocDiffs", "demangler", "showSymbolSizes", diff --git a/objdiff-core/src/arch/mod.rs b/objdiff-core/src/arch/mod.rs index 5d44aaea..afa6f21e 100644 --- a/objdiff-core/src/arch/mod.rs +++ b/objdiff-core/src/arch/mod.rs @@ -15,7 +15,7 @@ use object::Endian as _; use crate::{ diff::{ - DiffObjConfig, DiffSide, + ConfigEnum, DiffObjConfig, DiffSide, PreferredStringEncoding, display::{ContextItem, HoverItem, InstructionPart}, }, obj::{ @@ -52,6 +52,33 @@ const SUPPORTED_ENCODINGS_WITH_NULL_TERM: [(&encoding_rs::Encoding, &str); 5] = const SUPPORTED_ENCODINGS_NO_NULL_TERM: [(&encoding_rs::Encoding, &str); 2] = [(encoding_rs::UTF_16BE, "UTF-16BE"), (encoding_rs::UTF_16LE, "UTF-16LE")]; +#[derive(Debug, Clone, Default, PartialEq)] +pub struct LiteralInfo { + pub literal: String, + pub label_override: Option, + pub copy_string: Option, + pub hidden: bool, // Only used when the user hasn't set a preferred string encoding + pub is_string: bool, +} + +impl LiteralInfo { + pub fn hidden(&self, diff_config: Option<&DiffObjConfig>) -> bool { + let Some(diff_config) = diff_config else { + return self.hidden; + }; + if !self.is_string { + return self.hidden; + } + if diff_config.preferred_string_encoding == PreferredStringEncoding::Auto { + return self.hidden; + } + let Some(ref label) = self.label_override else { + return self.hidden; + }; + *label != diff_config.preferred_string_encoding.name() + } +} + /// Represents the type of data associated with an instruction #[derive(PartialEq)] pub enum DataType { @@ -83,18 +110,14 @@ impl fmt::Display for DataType { impl DataType { pub fn display_labels(&self, endian: object::Endianness, bytes: &[u8]) -> Vec { let mut strs = Vec::new(); - for (literal, label_override, _) in self.display_literals(endian, bytes) { - let label = label_override.unwrap_or_else(|| self.to_string()); - strs.push(format!("{label}: {literal:?}")) + for lit_info in self.display_literals(endian, bytes) { + let label = lit_info.label_override.unwrap_or_else(|| self.to_string()); + strs.push(format!("{}: {:?}", label, lit_info.literal)) } strs } - pub fn display_literals( - &self, - endian: object::Endianness, - bytes: &[u8], - ) -> Vec<(String, Option, Option)> { + pub fn display_literals(&self, endian: object::Endianness, bytes: &[u8]) -> Vec { let mut strs = Vec::new(); if self.required_len().is_some_and(|l| bytes.len() < l) { log::warn!( @@ -118,60 +141,70 @@ impl DataType { match self { DataType::Int8 => { let i = i8::from_ne_bytes(bytes.try_into().unwrap()); - strs.push((format!("{i:#x}"), None, None)); + strs.push(LiteralInfo { literal: format!("{i:#x}"), ..Default::default() }); if i < 0 { - strs.push((format!("{:#x}", ReallySigned(i)), None, None)); + strs.push(LiteralInfo { + literal: format!("{:#x}", ReallySigned(i)), + ..Default::default() + }); } } DataType::Int16 => { let i = endian.read_i16_bytes(bytes.try_into().unwrap()); - strs.push((format!("{i:#x}"), None, None)); + strs.push(LiteralInfo { literal: format!("{i:#x}"), ..Default::default() }); if i < 0 { - strs.push((format!("{:#x}", ReallySigned(i)), None, None)); + strs.push(LiteralInfo { + literal: format!("{:#x}", ReallySigned(i)), + ..Default::default() + }); } } DataType::Int32 => { let i = endian.read_i32_bytes(bytes.try_into().unwrap()); - strs.push((format!("{i:#x}"), None, None)); + strs.push(LiteralInfo { literal: format!("{i:#x}"), ..Default::default() }); if i < 0 { - strs.push((format!("{:#x}", ReallySigned(i)), None, None)); + strs.push(LiteralInfo { + literal: format!("{:#x}", ReallySigned(i)), + ..Default::default() + }); } } DataType::Int64 => { let i = endian.read_i64_bytes(bytes.try_into().unwrap()); - strs.push((format!("{i:#x}"), None, None)); + strs.push(LiteralInfo { literal: format!("{i:#x}"), ..Default::default() }); if i < 0 { - strs.push((format!("{:#x}", ReallySigned(i)), None, None)); + strs.push(LiteralInfo { + literal: format!("{:#x}", ReallySigned(i)), + ..Default::default() + }); } } DataType::Float => { let bytes: [u8; 4] = bytes.try_into().unwrap(); - strs.push(( - format!("{:?}f", match endian { + strs.push(LiteralInfo { + literal: format!("{:?}f", match endian { object::Endianness::Little => f32::from_le_bytes(bytes), object::Endianness::Big => f32::from_be_bytes(bytes), }), - None, - None, - )); + ..Default::default() + }); } DataType::Double => { let bytes: [u8; 8] = bytes.try_into().unwrap(); - strs.push(( - format!("{:?}", match endian { + strs.push(LiteralInfo { + literal: format!("{:?}", match endian { object::Endianness::Little => f64::from_le_bytes(bytes), object::Endianness::Big => f64::from_be_bytes(bytes), }), - None, - None, - )); + ..Default::default() + }); } DataType::Bytes => { - strs.push((format!("{bytes:#?}"), None, None)); + strs.push(LiteralInfo { literal: format!("{bytes:#?}"), ..Default::default() }); } DataType::String => { if let Some(nul_idx) = bytes.iter().position(|&c| c == b'\0') { @@ -181,15 +214,28 @@ impl DataType { if !had_errors && cow.is_ascii() { let string = format!("{cow}"); let copy_string = escape_special_ascii_characters(&string); - strs.push((string, Some("ASCII".into()), Some(copy_string))); + strs.push(LiteralInfo { + literal: string, + label_override: Some("ASCII".into()), + copy_string: Some(copy_string), + hidden: false, + is_string: true, + }); } for (encoding, encoding_name) in SUPPORTED_ENCODINGS_WITH_NULL_TERM { let (cow, _, had_errors) = encoding.decode(str_bytes); - // Avoid showing ASCII-only strings more than once if the encoding is ASCII-compatible. - if !had_errors && (!encoding.is_ascii_compatible() || !cow.is_ascii()) { + if !had_errors { let string = format!("{cow}"); let copy_string = escape_special_ascii_characters(&string); - strs.push((string, Some(encoding_name.into()), Some(copy_string))); + // Avoid showing ASCII-only strings more than once if the encoding is ASCII-compatible. + let hidden = encoding.is_ascii_compatible() && cow.is_ascii(); + strs.push(LiteralInfo { + literal: string, + label_override: Some(encoding_name.into()), + copy_string: Some(copy_string), + hidden, + is_string: true, + }); } } } @@ -202,11 +248,13 @@ impl DataType { let trimmed = cow.trim_end_matches('\0'); if !trimmed.is_empty() { let copy_string = escape_special_ascii_characters(trimmed); - strs.push(( - trimmed.to_string(), - Some(encoding_name.into()), - Some(copy_string), - )); + strs.push(LiteralInfo { + literal: trimmed.to_string(), + label_override: Some(encoding_name.into()), + copy_string: Some(copy_string), + hidden: false, + is_string: true, + }); } } } diff --git a/objdiff-core/src/diff/code.rs b/objdiff-core/src/diff/code.rs index ecaad621..aaef971f 100644 --- a/objdiff-core/src/diff/code.rs +++ b/objdiff-core/src/diff/code.rs @@ -9,8 +9,8 @@ use anyhow::{Context, Result, anyhow, ensure}; use super::{ DiffObjConfig, FunctionRelocDiffs, InstructionArgDiffIndex, InstructionBranchFrom, - InstructionBranchTo, InstructionDiffKind, InstructionDiffRow, SymbolDiff, - display::display_ins_data_literals, + InstructionBranchTo, InstructionDiffKind, InstructionDiffRow, PreferredStringEncoding, + SymbolDiff, display::display_ins_data_literals, }; use crate::obj::{ InstructionArg, InstructionArgValue, InstructionRef, Object, ResolvedInstructionRef, @@ -296,6 +296,26 @@ pub(crate) fn section_name_eq( }) } +fn ins_data_literals_eq( + left_obj: &Object, + right_obj: &Object, + left_ins: ResolvedInstructionRef, + right_ins: ResolvedInstructionRef, + diff_config: &DiffObjConfig, +) -> bool { + let mut left_literals = display_ins_data_literals(left_obj, left_ins); + let mut right_literals = display_ins_data_literals(right_obj, right_ins); + if left_literals == right_literals { + return true; + } + if diff_config.preferred_string_encoding == PreferredStringEncoding::Auto { + return left_literals == right_literals; + } + left_literals.retain(|lit_info| !lit_info.hidden(Some(diff_config))); + right_literals.retain(|lit_info| !lit_info.hidden(Some(diff_config))); + left_literals == right_literals +} + fn reloc_eq( left_obj: &Object, right_obj: &Object, @@ -330,8 +350,7 @@ fn reloc_eq( && (diff_config.function_reloc_diffs == FunctionRelocDiffs::NameAddress || left_reloc.symbol.kind != SymbolKind::Object || right_reloc.symbol.size == 0 // Likely a pool symbol like ...data, don't treat this as a diff - || display_ins_data_literals(left_obj, left_ins) - == display_ins_data_literals(right_obj, right_ins)) + || ins_data_literals_eq(left_obj, right_obj, left_ins, right_ins, diff_config)) } (Some(_), None) | (None, Some(_)) | (None, None) => symbol_name_addend_matches, } diff --git a/objdiff-core/src/diff/display.rs b/objdiff-core/src/diff/display.rs index 11b72bac..d5c7920e 100644 --- a/objdiff-core/src/diff/display.rs +++ b/objdiff-core/src/diff/display.rs @@ -12,6 +12,7 @@ use itertools::Itertools; use regex::Regex; use crate::{ + arch::LiteralInfo, diff::{ DataDiffKind, DataDiffRow, DiffObjConfig, InstructionDiffKind, InstructionDiffRow, ObjectDiff, SymbolDiff, data::resolve_relocation, @@ -494,15 +495,21 @@ pub fn relocation_context( obj: &Object, reloc: ResolvedRelocation, ins: Option, + diff_config: Option<&DiffObjConfig>, ) -> Vec { let mut out = Vec::new(); out.append(&mut symbol_context(obj, reloc.relocation.target_symbol)); if let Some(ins) = ins { - let literals = display_ins_data_literals(obj, ins); + let mut literals = display_ins_data_literals(obj, ins); + literals.retain(|lit_info| !lit_info.hidden(diff_config)); if !literals.is_empty() { out.push(ContextItem::Separator); - for (literal, label_override, copy_string) in literals { - out.push(ContextItem::Copy { value: literal, label: label_override, copy_string }); + for lit_info in literals { + out.push(ContextItem::Copy { + value: lit_info.literal, + label: lit_info.label_override, + copy_string: lit_info.copy_string, + }); } } } @@ -555,7 +562,7 @@ pub fn data_row_context(obj: &Object, diff_row: &DataDiffRow) -> Vec Vec { let mut out = Vec::new(); let mut hex_string = String::new(); @@ -636,7 +644,7 @@ pub fn instruction_context( } if let Some(reloc) = resolved.relocation { out.push(ContextItem::Separator); - out.append(&mut relocation_context(obj, reloc, Some(resolved))); + out.append(&mut relocation_context(obj, reloc, Some(resolved), Some(diff_config))); } out } @@ -645,6 +653,7 @@ pub fn instruction_hover( obj: &Object, resolved: ResolvedInstructionRef, ins: &ParsedInstruction, + diff_config: &DiffObjConfig, ) -> Vec { let mut out = Vec::new(); out.push(HoverItem::Text { @@ -687,13 +696,14 @@ pub fn instruction_hover( out.append(&mut relocation_hover(obj, reloc, None)); let bytes = obj.symbol_data(reloc.relocation.target_symbol).unwrap_or(&[]); if let Some(ty) = obj.arch.guess_data_type(resolved, bytes) { - let literals = display_ins_data_literals(obj, resolved); + let mut literals = display_ins_data_literals(obj, resolved); + literals.retain(|lit_info| !lit_info.hidden(Some(diff_config))); if !literals.is_empty() { out.push(HoverItem::Separator); - for (literal, label_override, _) in literals { + for lit_info in literals { out.push(HoverItem::Text { - label: label_override.unwrap_or_else(|| ty.to_string()), - value: format!("{literal:?}"), + label: lit_info.label_override.unwrap_or_else(|| ty.to_string()), + value: format!("{:?}", lit_info.literal), color: HoverItemColor::Normal, }); } @@ -880,7 +890,7 @@ pub fn display_ins_data_labels(obj: &Object, resolved: ResolvedInstructionRef) - pub fn display_ins_data_literals( obj: &Object, resolved: ResolvedInstructionRef, -) -> Vec<(String, Option, Option)> { +) -> Vec { let Some(reloc) = resolved.relocation else { return Vec::new(); }; diff --git a/objdiff-core/src/jobs/objdiff.rs b/objdiff-core/src/jobs/objdiff.rs index 595ba187..02a6b117 100644 --- a/objdiff-core/src/jobs/objdiff.rs +++ b/objdiff-core/src/jobs/objdiff.rs @@ -175,8 +175,8 @@ fn run_build( Ok(Box::new(ObjDiffResult { first_status, second_status, - first_obj: first_obj.and_then(|o| result.left.map(|d| (o, d))), - second_obj: second_obj.and_then(|o| result.right.map(|d| (o, d))), + first_obj: first_obj.zip(result.left), + second_obj: second_obj.zip(result.right), time, })) } diff --git a/objdiff-core/tests/common.rs b/objdiff-core/tests/common.rs index 330a5da2..d0e9d409 100644 --- a/objdiff-core/tests/common.rs +++ b/objdiff-core/tests/common.rs @@ -65,10 +65,9 @@ pub fn assert_literal_value( let ins_ref = diff.instruction_rows[ins_row_idx].ins_ref.unwrap(); let resolved = obj.resolve_instruction_ref(symbol_idx, ins_ref).unwrap(); let literals = objdiff_core::diff::display::display_ins_data_literals(obj, resolved); - let target_literal = literals - .iter() - .find(|(_, label_override, _)| *label_override == Some(literal_label.to_string())); + let target_literal = + literals.iter().find(|lit_info| lit_info.label_override == Some(literal_label.to_string())); assert_ne!(target_literal, None); - let (literal, _, _) = target_literal.unwrap(); - assert_eq!(literal, literal_value); + let lit_info = target_literal.unwrap(); + assert_eq!(lit_info.literal, literal_value); } diff --git a/objdiff-gui/src/views/frame_history.rs b/objdiff-gui/src/views/frame_history.rs index 711fce6c..d7c6031d 100644 --- a/objdiff-gui/src/views/frame_history.rs +++ b/objdiff-gui/src/views/frame_history.rs @@ -94,7 +94,7 @@ impl FrameHistory { let rect = rect.shrink(4.0); let color = ui.visuals().text_color(); - let line_stroke = Stroke::new(1.0, color); + let line_stroke = Stroke::new(1.0_f32, color); if let Some(pointer_pos) = response.hover_pos() { let y = pointer_pos.y; diff --git a/objdiff-gui/src/views/function_diff.rs b/objdiff-gui/src/views/function_diff.rs index eba7f181..b00377f5 100644 --- a/objdiff-gui/src/views/function_diff.rs +++ b/objdiff-gui/src/views/function_diff.rs @@ -97,7 +97,7 @@ fn ins_hover_ui( ui.scope(|ui| { ui.style_mut().override_text_style = Some(egui::TextStyle::Monospace); ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Wrap); - hover_items_ui(ui, instruction_hover(obj, resolved, &ins), appearance); + hover_items_ui(ui, instruction_hover(obj, resolved, &ins, diff_config), appearance); }); } @@ -128,7 +128,12 @@ fn ins_context_menu( ui.scope(|ui| { ui.style_mut().override_text_style = Some(egui::TextStyle::Monospace); ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Truncate); - context_menu_items_ui(ui, instruction_context(obj, resolved, &ins), column, appearance); + context_menu_items_ui( + ui, + instruction_context(obj, resolved, &ins, diff_config), + column, + appearance, + ); }); } diff --git a/objdiff-wasm/src/api.rs b/objdiff-wasm/src/api.rs index ed1c686a..3704626b 100644 --- a/objdiff-wasm/src/api.rs +++ b/objdiff-wasm/src/api.rs @@ -286,7 +286,7 @@ impl GuestDisplay for Component { })]; } }; - diff::display::instruction_context(obj, resolved, &ins) + diff::display::instruction_context(obj, resolved, &ins, &diff_config) .into_iter() .map(ContextItem::from) .collect() @@ -335,7 +335,7 @@ impl GuestDisplay for Component { })]; } }; - diff::display::instruction_hover(obj, resolved, &ins) + diff::display::instruction_hover(obj, resolved, &ins, &diff_config) .into_iter() .map(HoverItem::from) .collect() diff --git a/objdiff-wasm/src/logging.rs b/objdiff-wasm/src/logging.rs index 1c003aa4..4808bf02 100644 --- a/objdiff-wasm/src/logging.rs +++ b/objdiff-wasm/src/logging.rs @@ -44,6 +44,7 @@ pub fn init(level: wasi_logging::Level) { } #[cfg(not(feature = "std"))] +#[cfg(target_arch = "wasm32")] #[panic_handler] fn panic(info: &core::panic::PanicInfo) -> ! { use alloc::string::ToString;