feat: Add lifetime to pcodestore (#147)

* feat: parameterize pcodestore with a lifetime for borrowed ops

* clippy

Author: toolCHAINZ

jingle/src/analysis/cfg/mod.rs

@@ -1,4 +1,4 @@
-use crate::analysis::pcode_store::PcodeStore;
+use crate::analysis::pcode_store::{PcodeOpRef, PcodeStore};
 use crate::modeling::machine::cpu::concrete::ConcretePcodeAddress;
 use jingle_sleigh::{PcodeOperation, SleighArchInfo};
 pub use model::{CfgState, CfgStateModel, ModelTransition};
@@ -402,17 +402,31 @@ impl<N: CfgState, D: ModelTransition<N::Model>> PcodeCfg<N, D> {
     }
 }
 
-impl<L: CfgState> PcodeStore for PcodeCfg<L, PcodeOperation> {
-    fn get_pcode_op_at<'a, T: Borrow<ConcretePcodeAddress>>(
-        &'a self,
+impl<'op, L: CfgState> PcodeStore<'op> for PcodeCfg<L, PcodeOperation> {
+    fn get_pcode_op_at<T: Borrow<ConcretePcodeAddress>>(
+        &'op self,
         addr: T,
-    ) -> Option<crate::analysis::pcode_store::PcodeOpRef<'a>> {
+    ) -> Option<crate::analysis::pcode_store::PcodeOpRef<'op>> {
         let addr = *addr.borrow();
         self.get_op_at_address(addr)
             .map(crate::analysis::pcode_store::PcodeOpRef::from)
     }
 }
 
+impl<'op, L: CfgState> PcodeStore<'op> for PcodeCfg<L, PcodeOpRef<'op>> {
+    fn get_pcode_op_at<T: Borrow<ConcretePcodeAddress>>(
+        &'op self,
+        addr: T,
+    ) -> Option<crate::analysis::pcode_store::PcodeOpRef<'op>> {
+        let addr = *addr.borrow();
+        // The CFG stores `PcodeOpRef<'static>` values. We can return a borrowed
+        // `PcodeOpRef<'op>` that references the inner operation without cloning.
+        self.get_op_at_address(addr).map(|op_ref_static| {
+            crate::analysis::pcode_store::PcodeOpRef::from(op_ref_static.as_ref())
+        })
+    }
+}
+
 impl<N: CfgState> PcodeCfg<N, PcodeOperation> {
     pub fn basic_blocks(&self) -> PcodeCfg<N, Vec<PcodeOperation>> {
         use petgraph::visit::EdgeRef;

jingle/src/analysis/compound/state.rs

@@ -157,10 +157,10 @@ macro_rules! named_tuple {
             $F: 'static,
             $( $T: 'static ),+
         {
-            fn get_operation<'a, P: crate::analysis::pcode_store::PcodeStore + ?Sized>(
+            fn get_operation<'op, P: crate::analysis::pcode_store::PcodeStore<'op> + ?Sized>(
                 &self,
-                t: &'a P,
-            ) -> Option<crate::analysis::pcode_store::PcodeOpRef<'a>> {
+                t: &'op P,
+            ) -> Option<crate::analysis::pcode_store::PcodeOpRef<'op>> {
                 self.$first_field.get_operation(t)
             }
 

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

@@ -175,7 +175,7 @@ impl AbstractState for PcodeAddressLattice {
 }
 
 impl LocationState for PcodeAddressLattice {
-    fn get_operation<'a, T: crate::analysis::pcode_store::PcodeStore + ?Sized>(
+    fn get_operation<'a, T: crate::analysis::pcode_store::PcodeStore<'a> + ?Sized>(
         &self,
         t: &'a T,
     ) -> Option<crate::analysis::pcode_store::PcodeOpRef<'a>> {

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

@@ -81,10 +81,10 @@ impl<S: AbstractState + PartialJoinSemiLattice> AbstractState for SimpleLattice<
 }
 
 impl<S: LocationState + AbstractState + PartialJoinSemiLattice> LocationState for SimpleLattice<S> {
-    fn get_operation<'a, T: crate::analysis::pcode_store::PcodeStore + ?Sized>(
+    fn get_operation<'op, T: crate::analysis::pcode_store::PcodeStore<'op> + ?Sized>(
         &self,
-        t: &'a T,
-    ) -> Option<crate::analysis::pcode_store::PcodeOpRef<'a>> {
+        t: &'op T,
+    ) -> Option<crate::analysis::pcode_store::PcodeOpRef<'op>> {
         match self {
             SimpleLattice::Value(a) => a.get_operation(t),
             SimpleLattice::Top => None,

jingle/src/analysis/cpa/mod.rs

@@ -58,7 +58,7 @@ where
     /// p-code operation references returned by the `PcodeStore` (i.e. the store's
     /// borrow lifetime). The reducer type is instantiated for that same `'op`
     /// lifetime so it can accept `PcodeOpRef<'op>` without cloning.
-    fn run_cpa<'op, I: Borrow<Self::State>, P: PcodeStore + ?Sized>(
+    fn run_cpa<'op, I: Borrow<Self::State>, P: PcodeStore<'op> + ?Sized>(
         &self,
         initial: I,
         pcode_store: &'op P,

jingle/src/analysis/cpa/state.rs

@@ -203,6 +203,7 @@ pub trait LocationState: AbstractState {
     /// `PcodeStore` reference only; the state (`&self`) is not required to be
     /// borrowed for the same lifetime. This allows callers to obtain an op
     /// tied to the store borrow without forcing the state to outlive that borrow.
-    fn get_operation<'a, T: PcodeStore + ?Sized>(&self, t: &'a T) -> Option<PcodeOpRef<'a>>;
+    fn get_operation<'op, T: PcodeStore<'op> + ?Sized>(&self, t: &'op T)
+    -> Option<PcodeOpRef<'op>>;
     fn get_location(&self) -> Option<ConcretePcodeAddress>;
 }

jingle/src/analysis/location/basic/state.rs

@@ -164,10 +164,10 @@ impl AbstractState for BasicLocationState {
 }
 
 impl LocationState for BasicLocationState {
-    fn get_operation<'a, T: crate::analysis::pcode_store::PcodeStore + ?Sized>(
+    fn get_operation<'op, T: crate::analysis::pcode_store::PcodeStore<'op> + ?Sized>(
         &self,
-        t: &'a T,
-    ) -> Option<crate::analysis::pcode_store::PcodeOpRef<'a>> {
+        t: &'op T,
+    ) -> Option<crate::analysis::pcode_store::PcodeOpRef<'op>> {
         self.inner.get_operation(t)
     }
 

jingle/src/analysis/mod.rs

@@ -32,7 +32,7 @@ where
     /// The default implementation uses the standard CPA algorithm and delegates
     /// to `make_output` for any post-processing. Types can override this to provide
     /// custom run behavior.
-    fn run<'op, T: PcodeStore + ?Sized, I: IntoState<Self>>(
+    fn run<'op, T: PcodeStore<'op> + ?Sized, I: IntoState<Self>>(
         &self,
         store: &'op T,
         initial_state: I,
@@ -61,7 +61,7 @@ where
 {
 }
 
-pub trait AnalyzableEntry: PcodeStore + EntryPoint + Sized {
+pub trait AnalyzableEntry: for<'op> PcodeStore<'op> + EntryPoint + Sized {
     fn run_analysis<'op, T: Analysis>(
         &'op self,
         t: T,
@@ -76,4 +76,4 @@ pub trait AnalyzableEntry: PcodeStore + EntryPoint + Sized {
     }
 }
 
-impl<T: PcodeStore + EntryPoint> AnalyzableEntry for T {}
+impl<T: for<'op> PcodeStore<'op> + EntryPoint> AnalyzableEntry for T {}

jingle/src/analysis/pcode_store.rs

@@ -73,25 +73,28 @@ impl<'a> Debug for PcodeOpRef<'a> {
 
 /// A store of p-code operations.
 ///
-/// Implementations return `Option<PcodeOpRef<'a>>`. Callers that only need to
+/// Implementations return `Option<PcodeOpRef<'op>>`. Callers that only need to
 /// observe/borrow the operation should use `op_ref.as_ref()` to get a `&PcodeOperation`.
 /// If an owned operation is required, clone via `.as_ref().clone()`.
-pub trait PcodeStore {
-    fn get_pcode_op_at<'a, T: Borrow<ConcretePcodeAddress>>(
-        &'a self,
+///
+/// The trait is lifetime-parameterized so stores can tie the returned operation
+/// reference lifetime to the borrow of the store, avoiding unnecessary cloning.
+pub trait PcodeStore<'op> {
+    fn get_pcode_op_at<T: Borrow<ConcretePcodeAddress>>(
+        &'op self,
         addr: T,
-    ) -> Option<PcodeOpRef<'a>>;
+    ) -> Option<PcodeOpRef<'op>>;
 }
 
 pub trait EntryPoint {
     fn get_entry(&self) -> ConcretePcodeAddress;
 }
 
-impl<'a> PcodeStore for LoadedSleighContext<'a> {
-    fn get_pcode_op_at<'b, T: Borrow<ConcretePcodeAddress>>(
-        &'b self,
+impl<'a> PcodeStore<'a> for LoadedSleighContext<'a> {
+    fn get_pcode_op_at<T: Borrow<ConcretePcodeAddress>>(
+        &'a self,
         addr: T,
-    ) -> Option<PcodeOpRef<'b>> {
+    ) -> Option<PcodeOpRef<'a>> {
         let addr = addr.borrow();
         // `instruction_at` produces an owned `Instruction` per call. Its `ops`
         // are owned inside that `Instruction`, so we cannot return references
@@ -105,11 +108,11 @@ impl<'a> PcodeStore for LoadedSleighContext<'a> {
     }
 }
 
-impl<T: PcodeStore + ?Sized> PcodeStore for &T {
-    fn get_pcode_op_at<'a, B: Borrow<ConcretePcodeAddress>>(
-        &'a self,
+impl<'op, T: PcodeStore<'op> + ?Sized> PcodeStore<'op> for &T {
+    fn get_pcode_op_at<B: Borrow<ConcretePcodeAddress>>(
+        &'op self,
         addr: B,
-    ) -> Option<PcodeOpRef<'a>> {
+    ) -> Option<PcodeOpRef<'op>> {
         (*self).get_pcode_op_at(addr)
     }
 }