Skip to content

Commit 11ec4c9

Browse files
authored
refactor: generalize display impls, rename trait, move it to jingle_sleigh (#151)
* Move OpRef to jingle_sleigh * feat: Rename JingleDisplayable to JingleDisplay and move it to jingle_sleigh crate - Rename trait `JingleDisplayable` to `JingleDisplay` for clearer naming - Move `JingleDisplay` trait and related `JingleDisplayWrapper` struct implementations from `jingle` crate to `jingle_sleigh` crate - Update all imports and implementations accordingly to use the new trait location - Simplify `jingle` crate display module to re-export `JingleDisplay` from `jingle_sleigh` - Remove unused imports and redundant trait definitions related to the old trait name * impl JingleDisplay for PcodeOpRef
1 parent b713c4e commit 11ec4c9

File tree

17 files changed

+292
-284
lines changed

17 files changed

+292
-284
lines changed

jingle/examples/stack_offset.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use jingle::analysis::pcode_store::{self, PcodeOpRef};
1212
use jingle::analysis::valuation::{
1313
MergeBehavior, SimpleValuation, SimpleValuationAnalysis, SimpleValuationState,
1414
};
15-
use jingle::display::JingleDisplayable;
15+
use jingle::display::JingleDisplay;
1616
use jingle::modeling::machine::cpu::concrete::ConcretePcodeAddress;
1717
use jingle_sleigh::VarNode;
1818
use jingle_sleigh::context::image::gimli::load_with_gimli;

jingle/examples/stack_offset_smt.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use jingle::analysis::location::{BasicLocationAnalysis, CallBehavior};
1010
use jingle::analysis::pcode_store::PcodeStore;
1111
use jingle::analysis::pcode_store::{self, PcodeOpRef};
1212
use jingle::analysis::valuation::{MergeBehavior, SmtVal, SmtValuationAnalysis, SmtValuationState};
13-
use jingle::display::JingleDisplayable;
13+
use jingle::display::JingleDisplay;
1414
use jingle::modeling::machine::cpu::concrete::ConcretePcodeAddress;
1515
use jingle_sleigh::VarNode;
1616
use jingle_sleigh::context::image::gimli::load_with_gimli;

jingle/src/analysis/cpa/lattice/pcode.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::analysis::cpa::lattice::JoinSemiLattice;
22
use crate::analysis::cpa::state::{AbstractState, LocationState, MergeOutcome, Successor};
3-
use crate::display::JingleDisplayable;
3+
use crate::display::JingleDisplay;
44
use crate::modeling::machine::cpu::concrete::ConcretePcodeAddress;
55
use jingle_sleigh::{IndirectVarNode, PcodeOperation};
66
use std::borrow::Borrow;
@@ -22,7 +22,7 @@ pub enum PcodeAddressLattice {
2222
Top,
2323
}
2424

25-
impl JingleDisplayable for PcodeAddressLattice {
25+
impl JingleDisplay for PcodeAddressLattice {
2626
fn fmt_jingle(
2727
&self,
2828
f: &mut Formatter<'_>,

jingle/src/analysis/pcode_store.rs

Lines changed: 2 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1,75 +1,8 @@
11
use crate::modeling::machine::cpu::concrete::ConcretePcodeAddress;
2-
use jingle_sleigh::PcodeOperation;
32
use jingle_sleigh::context::loaded::LoadedSleighContext;
4-
use std::{
5-
borrow::{Borrow, Cow},
6-
fmt::Debug,
7-
};
3+
use std::borrow::Borrow;
84

9-
/// PcodeOpRef — a small, ergonomic wrapper for p-code operations
10-
///
11-
/// `PcodeOpRef` encapsulates either a borrowed reference to a `PcodeOperation`
12-
/// or an owned `PcodeOperation` (internally using `Cow`). This hides the `Cow`
13-
/// type from the rest of the codebase and provides simple helper methods so
14-
/// callers don't need to care about ownership when they only need a `&PcodeOperation`.
15-
///
16-
/// Why this exists
17-
/// - Some stores (e.g., an in-memory CFG) can return a reference to an operation
18-
/// stored inside the structure (no clone required).
19-
/// - Other stores (e.g., `LoadedSleighContext::instruction_at`) construct an
20-
/// `Instruction` on each call and therefore must return an owned `PcodeOperation`.
21-
/// - `PcodeOpRef` lets the store return either without exposing `Cow` to callers.
22-
///
23-
/// Basic usage
24-
/// ```ignore
25-
/// // Get an op from a pcode store (may be borrowed or owned internally)
26-
/// if let Some(op_ref) = store.get_pcode_op_at(addr) {
27-
/// // Use the borrowed reference for transfer/inspection:
28-
/// let op: &PcodeOperation = op_ref.as_ref();
29-
/// // When you need an owned op, clone the reference:
30-
/// let owned_op: PcodeOperation = op_ref.as_ref().clone();
31-
/// }
32-
/// ```
33-
///
34-
/// Note: there is no
35-
/// `into_owned` method on `PcodeOpRef` in order to keep the
36-
/// abstraction minimal; callers that need an owned value can call `.as_ref().clone()`.
37-
#[derive(Clone)]
38-
pub struct PcodeOpRef<'a>(std::borrow::Cow<'a, PcodeOperation>);
39-
40-
impl<'a> AsRef<PcodeOperation> for PcodeOpRef<'a> {
41-
fn as_ref(&self) -> &PcodeOperation {
42-
self.0.as_ref()
43-
}
44-
}
45-
46-
impl<'a> std::ops::Deref for PcodeOpRef<'a> {
47-
type Target = PcodeOperation;
48-
fn deref(&self) -> &Self::Target {
49-
&self.0
50-
}
51-
}
52-
53-
impl<'a> From<PcodeOperation> for PcodeOpRef<'a> {
54-
fn from(op: PcodeOperation) -> Self {
55-
PcodeOpRef(Cow::Owned(op))
56-
}
57-
}
58-
59-
impl<'a> From<&'a PcodeOperation> for PcodeOpRef<'a> {
60-
fn from(op: &'a PcodeOperation) -> Self {
61-
PcodeOpRef(Cow::Borrowed(op))
62-
}
63-
}
64-
65-
impl<'a> Debug for PcodeOpRef<'a> {
66-
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
67-
match &self.0 {
68-
Cow::Borrowed(o) => o.fmt(f),
69-
Cow::Owned(o) => o.fmt(f),
70-
}
71-
}
72-
}
5+
pub use jingle_sleigh::PcodeOpRef;
736

747
/// A store of p-code operations.
758
///

jingle/src/analysis/valuation/simple.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use crate::analysis::cpa::residue::EmptyResidue;
33
use crate::analysis::cpa::state::{AbstractState, MergeOutcome, Successor};
44
use crate::analysis::cpa::{ConfigurableProgramAnalysis, IntoState};
55
use crate::analysis::varnode_map::VarNodeMap;
6-
use crate::display::JingleDisplayable;
6+
use crate::display::JingleDisplay;
77
use crate::modeling::machine::cpu::concrete::ConcretePcodeAddress;
88
use jingle_sleigh::{GeneralizedVarNode, PcodeOperation, SleighArchInfo, SpaceType, VarNode};
99
use std::borrow::Borrow;
@@ -333,7 +333,7 @@ impl SimpleValuation {
333333
}
334334
}
335335

336-
impl JingleDisplayable for SimpleValuation {
336+
impl JingleDisplay for SimpleValuation {
337337
fn fmt_jingle(&self, f: &mut Formatter<'_>, info: &SleighArchInfo) -> std::fmt::Result {
338338
match self {
339339
SimpleValuation::Entry(vn) => write!(f, "Entry({})", vn.display(info)),

jingle/src/analysis/valuation/smt.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use crate::analysis::cpa::residue::EmptyResidue;
33
use crate::analysis::cpa::state::{AbstractState, MergeOutcome, Successor};
44
use crate::analysis::cpa::{ConfigurableProgramAnalysis, IntoState};
55
use crate::analysis::varnode_map::VarNodeMap;
6-
use crate::display::JingleDisplayable;
6+
use crate::display::JingleDisplay;
77
use crate::modeling::machine::cpu::concrete::ConcretePcodeAddress;
88
use jingle_sleigh::{GeneralizedVarNode, PcodeOperation, SleighArchInfo, SpaceType, VarNode};
99
use std::borrow::Borrow;
@@ -64,7 +64,7 @@ impl Hash for SmtVal {
6464
}
6565
}
6666

67-
impl JingleDisplayable for SmtVal {
67+
impl JingleDisplay for SmtVal {
6868
fn fmt_jingle(&self, f: &mut Formatter<'_>, info: &SleighArchInfo) -> std::fmt::Result {
6969
match self {
7070
SmtVal::Top => write!(f, "⊤"),

jingle/src/display.rs

Lines changed: 5 additions & 196 deletions
Original file line numberDiff line numberDiff line change
@@ -1,99 +1,10 @@
11
use crate::varnode::{ResolvedIndirectVarNode, ResolvedVarnode};
2-
use jingle_sleigh::{
3-
GeneralizedVarNode, IndirectVarNode, Instruction, PcodeOperation, SleighArchInfo, SpaceType,
4-
VarNode,
5-
};
6-
use std::fmt::{Display, Formatter};
2+
use jingle_sleigh::SleighArchInfo;
3+
pub use jingle_sleigh::{JingleDisplay, JingleDisplayWrapper};
4+
use std::fmt::Formatter;
75
use z3::ast::Ast;
86

9-
pub trait JingleDisplayable: Sized + Clone {
10-
fn fmt_jingle(&self, f: &mut Formatter<'_>, info: &SleighArchInfo) -> std::fmt::Result;
11-
12-
fn display(&self, info: &SleighArchInfo) -> JingleDisplay<Self> {
13-
JingleDisplay {
14-
info: info.clone(),
15-
inner: self.clone(),
16-
}
17-
}
18-
}
19-
20-
#[derive(Clone)]
21-
pub struct JingleDisplay<T> {
22-
info: SleighArchInfo,
23-
inner: T,
24-
}
25-
26-
impl<T> JingleDisplay<T> {
27-
pub fn inner(&self) -> &T {
28-
&self.inner
29-
}
30-
31-
pub fn info(&self) -> &SleighArchInfo {
32-
&self.info
33-
}
34-
}
35-
36-
impl<T: JingleDisplayable> Display for JingleDisplay<T> {
37-
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
38-
self.inner.fmt_jingle(f, &self.info)
39-
}
40-
}
41-
42-
impl JingleDisplay<ResolvedIndirectVarNode> {
43-
pub fn space_name(&self) -> &str {
44-
self.info
45-
.get_space(self.inner.pointer_space_idx)
46-
.unwrap()
47-
.name
48-
.as_str()
49-
}
50-
}
51-
52-
impl JingleDisplayable for VarNode {
53-
fn fmt_jingle(&self, f: &mut Formatter<'_>, ctx: &SleighArchInfo) -> std::fmt::Result {
54-
if self.space_index == VarNode::CONST_SPACE_INDEX {
55-
write!(f, "{:#x}:{}", self.offset, self.size)
56-
} else if let Some(name) = ctx.register_name(self) {
57-
write!(f, "{name}")
58-
} else if let Some(SpaceType::IPTR_INTERNAL) =
59-
ctx.get_space(self.space_index).map(|s| s._type)
60-
{
61-
write!(f, "$U{:x}:{}", self.offset, self.size)
62-
} else {
63-
write!(
64-
f,
65-
"[{}]{:#x}:{}",
66-
ctx.get_space(self.space_index).ok_or(std::fmt::Error)?.name,
67-
self.offset,
68-
self.size
69-
)
70-
}
71-
}
72-
}
73-
74-
impl JingleDisplayable for IndirectVarNode {
75-
fn fmt_jingle(&self, f: &mut Formatter<'_>, ctx: &SleighArchInfo) -> std::fmt::Result {
76-
write!(
77-
f,
78-
"{}({})",
79-
ctx.get_space(self.pointer_space_index)
80-
.ok_or(std::fmt::Error)?
81-
.name,
82-
self.pointer_location.display(ctx)
83-
)
84-
}
85-
}
86-
87-
impl JingleDisplayable for GeneralizedVarNode {
88-
fn fmt_jingle(&self, f: &mut Formatter<'_>, ctx: &SleighArchInfo) -> std::fmt::Result {
89-
match self {
90-
GeneralizedVarNode::Direct(d) => d.fmt_jingle(f, ctx),
91-
GeneralizedVarNode::Indirect(i) => i.fmt_jingle(f, ctx),
92-
}
93-
}
94-
}
95-
96-
impl JingleDisplayable for ResolvedIndirectVarNode {
7+
impl JingleDisplay for ResolvedIndirectVarNode {
978
fn fmt_jingle(&self, f: &mut Formatter<'_>, ctx: &SleighArchInfo) -> std::fmt::Result {
989
write!(
9910
f,
@@ -106,113 +17,11 @@ impl JingleDisplayable for ResolvedIndirectVarNode {
10617
}
10718
}
10819

109-
impl JingleDisplayable for ResolvedVarnode {
20+
impl JingleDisplay for ResolvedVarnode {
11021
fn fmt_jingle(&self, f: &mut Formatter<'_>, ctx: &SleighArchInfo) -> std::fmt::Result {
11122
match self {
11223
ResolvedVarnode::Direct(a) => a.fmt_jingle(f, ctx),
11324
ResolvedVarnode::Indirect(i) => i.fmt_jingle(f, ctx),
11425
}
11526
}
11627
}
117-
118-
impl JingleDisplayable for PcodeOperation {
119-
fn fmt_jingle(&self, f: &mut Formatter<'_>, ctx: &SleighArchInfo) -> std::fmt::Result {
120-
if let Some(o) = self.output() {
121-
write!(f, "{} = ", o.display(ctx))?;
122-
}
123-
write!(f, "{} ", self.opcode())?;
124-
let mut args: Vec<String> = vec![];
125-
for x in self.inputs() {
126-
args.push(format!("{}", x.display(ctx)));
127-
}
128-
write!(f, "{}", args.join(", "))?;
129-
Ok(())
130-
}
131-
}
132-
133-
impl JingleDisplayable for Instruction {
134-
fn fmt_jingle(&self, f: &mut Formatter<'_>, ctx: &SleighArchInfo) -> std::fmt::Result {
135-
writeln!(f, "{} {}", self.disassembly.mnemonic, self.disassembly.args)?;
136-
for x in &self.ops {
137-
writeln!(f, "\t{}", x.display(ctx))?;
138-
}
139-
Ok(())
140-
}
141-
}
142-
143-
#[cfg(test)]
144-
mod tests {
145-
use super::*;
146-
use jingle_sleigh::context::SleighContextBuilder;
147-
use jingle_sleigh::{PcodeOperation, VarNode};
148-
149-
fn make_sleigh() -> jingle_sleigh::context::SleighContext {
150-
// Mirrors other tests in the repo which expect a local Ghidra checkout at this path.
151-
let ctx_builder =
152-
SleighContextBuilder::load_ghidra_installation("/Applications/ghidra").unwrap();
153-
ctx_builder.build("x86:LE:64:default").unwrap()
154-
}
155-
156-
#[test]
157-
fn test_varnode_display_cases() {
158-
let sleigh = make_sleigh();
159-
let info = sleigh.arch_info().clone();
160-
161-
// const varnode should display as "<hex_offset>:<size>"
162-
let const_vn = VarNode {
163-
space_index: VarNode::CONST_SPACE_INDEX,
164-
offset: 0x42,
165-
size: 1,
166-
};
167-
assert_eq!(format!("{}", const_vn.display(&info)), "0x42:1");
168-
169-
// registers: if registers exist, pretty display should equal the register name
170-
let regs: Vec<_> = sleigh.arch_info().registers().collect();
171-
if !regs.is_empty() {
172-
let (vn, name) = regs[0].clone();
173-
assert_eq!(format!("{}", vn.display(&info)), name);
174-
}
175-
176-
// other space: we expect a bracketed form like "[<space>]offset:size" or internal $U...
177-
let other = VarNode {
178-
space_index: 1,
179-
offset: 0x10,
180-
size: 4,
181-
};
182-
let s = format!("{}", other.display(&info));
183-
// be permissive: ensure it contains the size and either bracket or unique prefix
184-
assert!(s.contains(":4"));
185-
}
186-
187-
#[test]
188-
fn test_pcode_display_and_round_trip_copy() {
189-
let sleigh = make_sleigh();
190-
let info = sleigh.arch_info().clone();
191-
192-
let output = VarNode {
193-
space_index: 4,
194-
offset: 0x20,
195-
size: 4,
196-
};
197-
let input = VarNode {
198-
space_index: VarNode::CONST_SPACE_INDEX,
199-
offset: 0x1,
200-
size: 4,
201-
};
202-
203-
let op = PcodeOperation::Copy {
204-
input: input.clone(),
205-
output: output.clone(),
206-
};
207-
208-
// display formatting contains the opcode name and operands
209-
let s = format!("{}", op.display(&info));
210-
assert_eq!(s, "ESP = COPY 0x1:4");
211-
// Try to round-trip: render to textual pcode and parse with SleighContext.parse_pcode_listing
212-
// Use the same textual format that `display` produces for varnodes.
213-
214-
let parsed = sleigh.parse_pcode_listing(s).unwrap();
215-
assert_eq!(parsed.len(), 1);
216-
assert_eq!(parsed[0], op);
217-
}
218-
}

jingle/src/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use anyhow::Context;
22
use clap::{Parser, Subcommand};
33
use hex::decode;
4-
use jingle::display::JingleDisplayable;
4+
use jingle::display::JingleDisplay;
55
use jingle::modeling::{ModeledBlock, ModelingContext, State};
66
use jingle_sleigh::context::SleighContextBuilder;
77
use jingle_sleigh::context::loaded::LoadedSleighContext;

jingle/src/python/instruction.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
use crate::display::JingleDisplayable;
21
use crate::sleigh::context::loaded::LoadedSleighContext;
32
use crate::sleigh::{Instruction, PcodeOperation};
43
use jingle_sleigh::SleighArchInfo;
4+
use jingle_sleigh::display::JingleDisplay;
55
use pyo3::{pyclass, pymethods};
66
use std::borrow::Borrow;
77
use std::fmt::{Display, Formatter};

jingle/src/python/modeled_block.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
use crate::display::JingleDisplayable;
21
use crate::modeling::{ModeledBlock, ModelingContext};
32
use crate::python::instruction::PythonInstruction;
43
use crate::python::resolved_varnode::PythonResolvedVarNode;
54
use crate::python::state::PythonState;
65
use crate::python::varode_iterator::VarNodeIterator;
76
use crate::sleigh::Instruction;
87
use jingle_sleigh::SleighArchInfo;
8+
use jingle_sleigh::display::JingleDisplay;
99
use pyo3::{PyResult, pyclass, pymethods};
1010
use std::borrow::Borrow;
1111

0 commit comments

Comments
 (0)