From fb24063c54eea82317b8426762190651d90676e1 Mon Sep 17 00:00:00 2001 From: Luke Street Date: Tue, 27 Feb 2024 22:52:18 -0700 Subject: [PATCH] objdiff-cli diff: Click-to-highlight & build fixes --- objdiff-cli/src/cmd/diff.rs | 124 ++++++++++++++++++++++--- objdiff-cli/src/cmd/report.rs | 2 +- objdiff-core/src/diff/display.rs | 20 ++-- objdiff-gui/src/app.rs | 12 +-- objdiff-gui/src/jobs/objdiff.rs | 2 +- objdiff-gui/src/views/config.rs | 14 +-- objdiff-gui/src/views/function_diff.rs | 2 +- 7 files changed, 130 insertions(+), 46 deletions(-) diff --git a/objdiff-cli/src/cmd/diff.rs b/objdiff-cli/src/cmd/diff.rs index e9c61f8..312bee3 100644 --- a/objdiff-cli/src/cmd/diff.rs +++ b/objdiff-cli/src/cmd/diff.rs @@ -9,7 +9,8 @@ use crossterm::{ cursor::{Hide, MoveRight, MoveTo, Show}, event, event::{ - DisableMouseCapture, EnableMouseCapture, Event, KeyCode, KeyEventKind, MouseEventKind, + DisableMouseCapture, EnableMouseCapture, Event, KeyCode, KeyEventKind, MouseButton, + MouseEventKind, }, style::{Color, PrintStyledContent, Stylize}, terminal::{ @@ -22,7 +23,7 @@ use objdiff_core::{ diff, diff::display::{display_diff, DiffText}, obj, - obj::{ObjInfo, ObjInsDiffKind, ObjSection, ObjSectionKind, ObjSymbol}, + obj::{ObjInfo, ObjInsArgValue, ObjInsDiffKind, ObjSection, ObjSectionKind, ObjSymbol}, }; use crate::util::term::crossterm_panic_handler; @@ -69,17 +70,22 @@ pub fn run(args: Args) -> Result<()> { EnableMouseCapture, )?; + let mut clear = true; let mut redraw = true; let mut skip = 0; + let mut click_xy = None; + let mut highlight = HighlightKind::None; + let (mut sx, mut sy) = terminal_size()?; loop { let y_offset = 2; - let (sx, sy) = terminal_size()?; let per_page = sy as usize - y_offset; if redraw { let mut w = stdout().lock(); + if clear { + crossterm::queue!(w, Clear(ClearType::All))?; + } crossterm::queue!( w, - Clear(ClearType::All), MoveTo(0, 0), PrintStyledContent(args.symbol.clone().with(Color::White)), MoveTo(0, 1), @@ -103,14 +109,51 @@ pub fn run(args: Args) -> Result<()> { if skip > max_len - per_page { skip = max_len - per_page; } + let mut new_highlight = None; if let Some((_, symbol)) = left_sym { - print_sym(&mut w, symbol, 0, y_offset as u16, sx / 2 - 1, sy, skip)?; + let h = print_sym( + &mut w, + symbol, + 0, + y_offset as u16, + sx / 2 - 1, + sy, + skip, + &mut highlight, + click_xy, + )?; + if let Some(h) = h { + new_highlight = Some(h); + } } if let Some((_, symbol)) = right_sym { - print_sym(&mut w, symbol, sx / 2, y_offset as u16, sx, sy, skip)?; + let h = print_sym( + &mut w, + symbol, + sx / 2, + y_offset as u16, + sx, + sy, + skip, + &mut highlight, + click_xy, + )?; + if let Some(h) = h { + new_highlight = Some(h); + } } w.flush()?; - redraw = false; + if let Some(new_highlight) = new_highlight { + highlight = new_highlight; + redraw = true; + click_xy = None; + clear = false; + continue; // Redraw now + } else { + redraw = false; + click_xy = None; + clear = true; + } } match event::read()? { @@ -167,9 +210,17 @@ pub fn run(args: Args) -> Result<()> { skip = skip.saturating_sub(3); redraw = true; } + MouseEventKind::Down(MouseButton::Left) => { + click_xy = Some((event.column, event.row)); + redraw = true; + } _ => {} }, - Event::Resize(_, _) => redraw = true, + Event::Resize(x, y) => { + sx = x; + sy = y; + redraw = true; + } _ => {} } } @@ -194,6 +245,7 @@ fn find_function<'a>(obj: &'a ObjInfo, name: &str) -> Option<(&'a ObjSection, &' None } +#[allow(clippy::too_many_arguments)] fn print_sym( w: &mut W, symbol: &ObjSymbol, @@ -202,11 +254,14 @@ fn print_sym( max_sx: u16, max_sy: u16, skip: usize, -) -> Result<()> + highlight: &mut HighlightKind, + click_xy: Option<(u16, u16)>, +) -> Result> where W: Write, { let base_addr = symbol.address as u32; + let mut new_highlight = None; for ins_diff in symbol.instructions.iter().skip(skip) { let mut sx = sx; if ins_diff.kind != ObjInsDiffKind::None && sx > 2 { @@ -220,7 +275,7 @@ where } else { crossterm::queue!(w, MoveTo(sx, sy))?; } - display_diff(ins_diff, base_addr, |text| { + display_diff(ins_diff, base_addr, |text| -> Result<()> { let mut label_text; let mut base_color = match ins_diff.kind { ObjInsDiffKind::None | ObjInsDiffKind::OpMismatch | ObjInsDiffKind::ArgMismatch => { @@ -231,6 +286,7 @@ where ObjInsDiffKind::Insert => Color::DarkGreen, }; let mut pad_to = 0; + let mut highlight_kind = HighlightKind::None; match text { DiffText::Basic(text) => { label_text = text.to_string(); @@ -247,27 +303,32 @@ where DiffText::Address(addr) => { label_text = format!("{:x}:", addr); pad_to = 5; + highlight_kind = HighlightKind::Address(addr); } - DiffText::Opcode(mnemonic, _op) => { + DiffText::Opcode(mnemonic, op) => { label_text = mnemonic.to_string(); if ins_diff.kind == ObjInsDiffKind::OpMismatch { base_color = Color::Blue; } pad_to = 8; + highlight_kind = HighlightKind::Opcode(op); } DiffText::Argument(arg, diff) => { label_text = arg.to_string(); if let Some(diff) = diff { base_color = COLOR_ROTATION[diff.idx % COLOR_ROTATION.len()] } + highlight_kind = HighlightKind::Arg(arg.clone()); } DiffText::BranchTarget(addr) => { label_text = format!("{addr:x}"); + highlight_kind = HighlightKind::Address(addr); } DiffText::Symbol(sym) => { let name = sym.demangled_name.as_ref().unwrap_or(&sym.name); label_text = name.clone(); base_color = Color::White; + highlight_kind = HighlightKind::Symbol(name.clone()); } DiffText::Spacing(n) => { crossterm::queue!(w, MoveRight(n as u16))?; @@ -283,8 +344,22 @@ where if sx >= max_sx { return Ok(()); } + let highlighted = highlight == &highlight_kind; + if let Some((cx, cy)) = click_xy { + if cx >= sx && cx < sx + len as u16 && cy == sy { + if highlighted { + new_highlight = Some(HighlightKind::None); + } else { + new_highlight = Some(highlight_kind); + } + } + } label_text.truncate(max_sx as usize - sx as usize); - crossterm::queue!(w, PrintStyledContent(label_text.with(base_color)))?; + let mut content = label_text.with(base_color); + if highlighted { + content = content.on_dark_grey(); + } + crossterm::queue!(w, PrintStyledContent(content))?; sx += len as u16; if pad_to > len { let pad = (pad_to - len) as u16; @@ -297,7 +372,7 @@ where break; } } - Ok(()) + Ok(new_highlight) } pub const COLOR_ROTATION: [Color; 8] = [ @@ -320,3 +395,26 @@ pub fn match_percent_color(match_percent: f32) -> Color { Color::Red } } + +#[derive(Default)] +pub enum HighlightKind { + #[default] + None, + Opcode(u8), + Arg(ObjInsArgValue), + Symbol(String), + Address(u32), +} + +impl PartialEq for HighlightKind { + fn eq(&self, other: &Self) -> bool { + match (self, other) { + (HighlightKind::None, HighlightKind::None) => false, + (HighlightKind::Opcode(a), HighlightKind::Opcode(b)) => a == b, + (HighlightKind::Arg(a), HighlightKind::Arg(b)) => a.loose_eq(b), + (HighlightKind::Symbol(a), HighlightKind::Symbol(b)) => a == b, + (HighlightKind::Address(a), HighlightKind::Address(b)) => a == b, + _ => false, + } + } +} diff --git a/objdiff-cli/src/cmd/report.rs b/objdiff-cli/src/cmd/report.rs index c0d7f2f..f38c5ae 100644 --- a/objdiff-cli/src/cmd/report.rs +++ b/objdiff-cli/src/cmd/report.rs @@ -178,7 +178,7 @@ fn report_object( .as_ref() .map(|p| obj::elf::read(p).with_context(|| format!("Failed to open {}", p.display()))) .transpose()?; - let config = diff::DiffObjConfig { relax_reloc_diffs: true, ..Default::default() }; + let config = diff::DiffObjConfig { relax_reloc_diffs: true }; diff::diff_objs(&config, target.as_mut(), base.as_mut())?; let mut unit = ReportUnit { name: object.name().to_string(), ..Default::default() }; let obj = target.as_ref().or(base.as_ref()).unwrap(); diff --git a/objdiff-core/src/diff/display.rs b/objdiff-core/src/diff/display.rs index 25f4b0b..5c4abd8 100644 --- a/objdiff-core/src/diff/display.rs +++ b/objdiff-core/src/diff/display.rs @@ -1,7 +1,5 @@ use std::cmp::Ordering; -use anyhow::{bail, Result}; - use crate::obj::{ ObjInsArg, ObjInsArgDiff, ObjInsArgValue, ObjInsDiff, ObjReloc, ObjRelocKind, ObjSymbol, }; @@ -30,11 +28,11 @@ pub enum DiffText<'a> { Eol, } -pub fn display_diff( +pub fn display_diff( ins_diff: &ObjInsDiff, base_addr: u32, - mut cb: impl FnMut(DiffText) -> Result<()>, -) -> Result<()> { + mut cb: impl FnMut(DiffText) -> Result<(), E>, +) -> Result<(), E> { let Some(ins) = &ins_diff.ins else { cb(DiffText::Eol)?; return Ok(()); @@ -94,7 +92,10 @@ pub fn display_diff( Ok(()) } -fn display_reloc_name(reloc: &ObjReloc, mut cb: impl FnMut(DiffText) -> Result<()>) -> Result<()> { +fn display_reloc_name( + reloc: &ObjReloc, + mut cb: impl FnMut(DiffText) -> Result<(), E>, +) -> Result<(), E> { cb(DiffText::Symbol(&reloc.target))?; match reloc.target.addend.cmp(&0i64) { Ordering::Greater => cb(DiffText::Basic(&format!("+{:#X}", reloc.target.addend))), @@ -103,7 +104,10 @@ fn display_reloc_name(reloc: &ObjReloc, mut cb: impl FnMut(DiffText) -> Result<( } } -fn display_reloc(reloc: &ObjReloc, mut cb: impl FnMut(DiffText) -> Result<()>) -> Result<()> { +fn display_reloc( + reloc: &ObjReloc, + mut cb: impl FnMut(DiffText) -> Result<(), E>, +) -> Result<(), E> { match reloc.kind { #[cfg(feature = "ppc")] ObjRelocKind::PpcAddr16Lo => { @@ -165,7 +169,7 @@ fn display_reloc(reloc: &ObjReloc, mut cb: impl FnMut(DiffText) -> Result<()>) - } #[cfg(feature = "mips")] ObjRelocKind::MipsGpRel32 => { - bail!("unimplemented: mips gp_rel32"); + todo!("unimplemented: mips gp_rel32"); } ObjRelocKind::Absolute => { cb(DiffText::Basic("[INVALID]"))?; diff --git a/objdiff-gui/src/app.rs b/objdiff-gui/src/app.rs index 26b46e8..5bf3c9f 100644 --- a/objdiff-gui/src/app.rs +++ b/objdiff-gui/src/app.rs @@ -12,11 +12,8 @@ use std::{ use filetime::FileTime; use globset::{Glob, GlobSet}; use notify::{RecursiveMode, Watcher}; -use objdiff_core::{ - config::{ - build_globset, ProjectConfigInfo, ProjectObject, ScratchConfig, DEFAULT_WATCH_PATTERNS, - }, - diff::DiffAlg, +use objdiff_core::config::{ + build_globset, ProjectConfigInfo, ProjectObject, ScratchConfig, DEFAULT_WATCH_PATTERNS, }; use time::UtcOffset; @@ -29,9 +26,7 @@ use crate::{ }, views::{ appearance::{appearance_window, Appearance}, - config::{ - config_ui, diff_options_window, project_window, ConfigViewState, CONFIG_DISABLED_TEXT, - }, + config::{config_ui, project_window, ConfigViewState, CONFIG_DISABLED_TEXT}, data_diff::data_diff_ui, debug::debug_window, demangle::{demangle_window, DemangleViewState}, @@ -529,7 +524,6 @@ impl eframe::App for App { project_window(ctx, config, show_project_config, config_state, appearance); appearance_window(ctx, show_appearance_config, appearance); demangle_window(ctx, show_demangle, demangle_state, appearance); - diff_options_window(ctx, config, show_diff_options, appearance); debug_window(ctx, show_debug, frame_history, appearance); self.post_update(ctx); diff --git a/objdiff-gui/src/jobs/objdiff.rs b/objdiff-gui/src/jobs/objdiff.rs index e2bc074..d09e626 100644 --- a/objdiff-gui/src/jobs/objdiff.rs +++ b/objdiff-gui/src/jobs/objdiff.rs @@ -7,7 +7,7 @@ use std::{ use anyhow::{anyhow, Context, Error, Result}; use objdiff_core::{ - diff::{diff_objs, DiffAlg, DiffObjConfig}, + diff::{diff_objs, DiffObjConfig}, obj::{elf, ObjInfo}, }; use time::OffsetDateTime; diff --git a/objdiff-gui/src/views/config.rs b/objdiff-gui/src/views/config.rs index cb1b050..a90c430 100644 --- a/objdiff-gui/src/views/config.rs +++ b/objdiff-gui/src/views/config.rs @@ -11,7 +11,7 @@ use anyhow::{Context, Result}; use const_format::formatcp; use egui::{ output::OpenUrl, text::LayoutJob, CollapsingHeader, FontFamily, FontId, RichText, - SelectableLabel, TextFormat, Widget, WidgetText, + SelectableLabel, TextFormat, Widget, }; use globset::Glob; use objdiff_core::config::{ProjectObject, DEFAULT_WATCH_PATTERNS}; @@ -838,15 +838,3 @@ fn split_obj_config_ui( } }); } - -pub fn diff_options_window( - ctx: &egui::Context, - config: &AppConfigRef, - show: &mut bool, - appearance: &Appearance, -) { - let mut config_guard = config.write().unwrap(); - egui::Window::new("Diff Options").open(show).show(ctx, |ui| { - diff_options_ui(ui, &mut config_guard, appearance); - }); -} diff --git a/objdiff-gui/src/views/function_diff.rs b/objdiff-gui/src/views/function_diff.rs index 0830419..7bc7b07 100644 --- a/objdiff-gui/src/views/function_diff.rs +++ b/objdiff-gui/src/views/function_diff.rs @@ -247,7 +247,7 @@ fn asm_row_ui( let space_width = ui.fonts(|f| f.glyph_width(&appearance.code_font, ' ')); display_diff(ins_diff, symbol.address as u32, |text| { diff_text_ui(ui, text, ins_diff, appearance, ins_view_state, space_width); - Ok(()) + Ok::<_, ()>(()) }) .unwrap(); }