-
Notifications
You must be signed in to change notification settings - Fork 6
Expose structure of serialized state #148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -110,9 +110,60 @@ and t = { | |
| } | ||
| [@@deriving show { with_path = false }] | ||
|
|
||
| type serialized = Tree_block.serialized Freeable.serialized SPmap.serialized | ||
| type serialized = | ||
| (serialized_atom list, serialized_globals) State_intf.Template.t | ||
| [@@deriving show { with_path = false }] | ||
|
|
||
| and serialized_atom = T.sloc Typed.t * Tree_block.serialized Freeable.serialized | ||
| [@@deriving show { with_path = false }] | ||
|
|
||
| and serialized_globals = T.sloc Typed.t list | ||
| [@@deriving show { with_path = false }] | ||
|
|
||
| let serialize_globals globals : serialized_globals = | ||
| ListLabels.fold_left (GlobMap.bindings globals) ~init:[] | ||
| ~f:(fun acc (_, ((ptr : Sptr.t), _)) -> Typed.Ptr.loc ptr.ptr :: acc) | ||
|
Comment on lines
+120
to
+125
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this doesn't make sense to me -- just knowing "a global" is in a pointer isn't enough; what you should have is probably
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should glob_map be an instance of
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmmm that'd require changing the state to be |
||
|
|
||
| let lift_fix_globals globals res = | ||
| let+? heap = res in | ||
| State_intf.Template.{ heap; globals = serialize_globals globals } | ||
|
Comment on lines
+127
to
+129
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also here youre adding all globals to all fixes raised -- i don't think this is normal? i wouldve expected
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. instead (with the change im recommending), youd just have |
||
|
|
||
| let serialize st : serialized = | ||
| let heap = | ||
| match st.state with | ||
| | None -> [] | ||
| | Some st -> | ||
| let serialize_freeable (b, _) = Tree_block.serialize b in | ||
| SPmap.serialize (Freeable.serialize serialize_freeable) st | ||
| in | ||
| let globals = serialize_globals st.globals in | ||
| { heap; globals } | ||
|
|
||
| let subst_serialized (subst_var : Svalue.Var.t -> Svalue.Var.t) | ||
| (serialized : serialized) : serialized = | ||
| let heap = | ||
| let subst_serialized_block subst_var f = | ||
| Freeable.subst_serialized Tree_block.subst_serialized subst_var f | ||
| in | ||
| SPmap.subst_serialized subst_serialized_block subst_var serialized.heap | ||
| in | ||
| let globals = List.map (Typed.subst subst_var) serialized.globals in | ||
| { heap; globals } | ||
|
|
||
| let iter_vars_serialized (s : serialized) : | ||
| (Svalue.Var.t * [< Typed.T.cval ] Typed.ty -> unit) -> unit = | ||
| let iter_heap = | ||
| let iter_vars_serialized_block s = | ||
| Freeable.iter_vars_serialized Tree_block.iter_vars_serialized s | ||
| in | ||
| SPmap.iter_vars_serialized iter_vars_serialized_block s.heap | ||
| in | ||
| let iter_globals = | ||
| let append acc x = Iter.append acc (Typed.iter_vars x) in | ||
| List.fold_left append Iter.empty s.globals | ||
| in | ||
|
Comment on lines
+161
to
+164
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. instead of using list and calling |
||
| Iter.append iter_heap iter_globals | ||
|
|
||
| let pp_pretty ~ignore_freed ft { state; _ } = | ||
| let ignore = | ||
| if ignore_freed then function _, Freeable.Freed -> true | _ -> false | ||
|
|
@@ -190,7 +241,10 @@ let with_ptr (ptr : Sptr.t) (st : t) | |
| in | ||
| let loc, ofs = Typed.Ptr.decompose ptr.ptr in | ||
| let@ state = with_state st in | ||
| let* res = (SPmap.wrap (Freeable.wrap (fun st -> f (ofs, st)))) loc state in | ||
| let* res = | ||
| (SPmap.wrap (Freeable.wrap (fun st -> f (ofs, st)))) loc state | ||
| |> lift_fix_globals st.globals | ||
| in | ||
| match res with | ||
| | Missing _ as miss -> | ||
| if%sat Sptr.is_at_null_loc ptr then Result.error `UBDanglingPointer | ||
|
|
@@ -365,7 +419,9 @@ let alloc ?zeroed size align st = | |
| let tb = Tree_borrow.init ~state:Unique () in | ||
| let block = Tree_block.alloc ?zeroed size in | ||
| let block = Freeable.Alive (block, tb) in | ||
| let** loc, state = SPmap.alloc ~new_codom:block state in | ||
| let** loc, state = | ||
| SPmap.alloc ~new_codom:block state |> lift_fix_globals st.globals | ||
| in | ||
| let ptr = Typed.Ptr.mk loc Usize.(0s) in | ||
| let ptr : Sptr.t Rust_val.full_ptr = | ||
| ({ ptr; tag = tb.tag; align; size }, None) | ||
|
|
@@ -404,6 +460,7 @@ let alloc_tys tys st = | |
| } | ||
| in | ||
| (block, (ptr, None))) | ||
| |> lift_fix_globals st.globals | ||
|
|
||
| let free (({ ptr; _ } : Sptr.t), _) ({ state; _ } as st) = | ||
| let** () = assert_ (Typed.Ptr.ofs ptr ==@ Usize.(0s)) `InvalidFree st in | ||
|
|
@@ -415,6 +472,7 @@ let free (({ ptr; _ } : Sptr.t), _) ({ state; _ } as st) = | |
| (Freeable.free ~assert_exclusively_owned:(fun t -> | ||
| Tree_block.assert_exclusively_owned @@ Option.map fst t)) | ||
| (Typed.Ptr.loc ptr) state | ||
| |> lift_fix_globals st.globals | ||
| in | ||
| ((), { st with state }) | ||
|
|
||
|
|
@@ -559,6 +617,7 @@ let leak_check st = | |
| Result.ok (k :: leaks) | ||
| else Result.ok leaks) | ||
| [] state | ||
| |> lift_fix_globals st.globals | ||
| in | ||
| if List.is_empty leaks then Result.ok ((), state) | ||
| else ( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,12 @@ open T | |
| open Rustsymex | ||
| open Charon | ||
| open Rust_val | ||
| open Soteria.Sym_states | ||
|
|
||
| module Template = struct | ||
| type ('a, 'b) t = { heap : 'a; globals : 'b } | ||
| [@@deriving show { with_path = false }] | ||
| end | ||
|
Comment on lines
+8
to
+11
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also this doesn't necessarily apply to all states either, so define it within |
||
|
|
||
| module type S = sig | ||
| module Sptr : Sptr.S | ||
|
|
@@ -11,7 +17,14 @@ module type S = sig | |
|
|
||
| (* state *) | ||
| type t | ||
| type serialized | ||
|
|
||
| type serialized = (serialized_atom list, sloc Typed.t list) Template.t | ||
|
|
||
| and serialized_atom = | ||
| sloc Typed.t | ||
| * (Sptr.t Rtree_block.serialized_val, sint Value.t) Tree_block.serialized | ||
| Freeable.serialized | ||
|
Comment on lines
+21
to
+26
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to extract the types from the functors because otherwise I can't write this in the Unfortunately, that is still not enough because Basically, as is, I can't expose the serialized structure in the .mli in a clean way. I'm honestly alright with adding the modules
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does this not work ? 2fef10c
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (the example doesn't compile bc i was lazy but supposedly the interface works fine -- maybe i missed sth)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I get the idea. My main issue with this is that ArithPtr is being hardcoded, which I was trying to avoid as I should probably only rely on the Sptr interface. But if that just sounds like overthinking of my part, then that should do the work, thanks!
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i mean you're hardcoding the entire serialised structure anyways so adding Sptr doesn't matter much -- this is a particular module that is made for ArithPtr, not State_intf, which is the general one it's ok to specialise State because all the engine relies on State_intf, it just allows to expose information for your particular tool
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alright, thanks! I'll make things work like this for RUXt then, so I guess this PR can be ignored for now
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure! if you want to edit this / re-open a PR to expose |
||
|
|
||
|
Comment on lines
+20
to
+27
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can't do this -- if we want to add a new state model for polymorphic analysis the state shape will be different. Instead, in |
||
| type 'a err | ||
|
|
||
| val add_to_call_trace : | ||
|
|
@@ -22,6 +35,15 @@ module type S = sig | |
| (** Prettier but expensive printing. *) | ||
| val pp_pretty : ignore_freed:bool -> t Fmt.t | ||
|
|
||
| val serialize : t -> serialized | ||
| val pp_serialized : Format.formatter -> serialized -> unit | ||
|
|
||
| val iter_vars_serialized : | ||
| serialized -> (Svalue.Var.t * [< cval ] Typed.ty -> unit) -> unit | ||
|
|
||
| val subst_serialized : | ||
| (Svalue.Var.t -> Svalue.Var.t) -> serialized -> serialized | ||
|
|
||
| val empty : t | ||
|
|
||
| val load : | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,14 @@ | ||
| (* TODO: Build this with the Sum transformer and `Excl(Freed)` *) | ||
|
|
||
| type 'a t = Freed | Alive of 'a | ||
| type 'a serialized = 'a t | ||
|
|
||
| let iter_vars_serialized iter_inner serialized f = | ||
| match serialized with Freed -> () | Alive a -> iter_inner a f | ||
|
|
||
| let subst_serialized subst_inner subst_var = function | ||
| | Freed -> Freed | ||
| | Alive a -> Alive (subst_inner subst_var a) | ||
|
|
||
| let pp ?(alive_prefix = "") pp_alive ft = function | ||
| | Freed -> Fmt.pf ft "Freed" | ||
|
|
@@ -10,25 +18,19 @@ module Make (Symex : Symex.S) = struct | |
| open Symex.Syntax | ||
|
|
||
| type nonrec 'a t = 'a t = Freed | Alive of 'a | ||
| type 'a serialized = 'a t | ||
| type nonrec 'a serialized = 'a serialized | ||
|
|
||
| let lift_fix fix = Alive fix | ||
| let lift_fix_s r = Symex.Result.map_missing r lift_fix | ||
| let subst_serialized = subst_serialized | ||
| let iter_vars_serialized = iter_vars_serialized | ||
| let pp = pp | ||
| let pp_serialized = pp | ||
|
|
||
| let serialize serialize_inner = function | ||
| | Freed -> Freed | ||
| | Alive a -> Alive (serialize_inner a) | ||
|
|
||
| let subst_serialized subst_inner subst_var = function | ||
| | Freed -> Freed | ||
| | Alive a -> Alive (subst_inner subst_var a) | ||
|
|
||
| let iter_vars_serialized iter_inner serialized f = | ||
| match serialized with Freed -> () | Alive a -> iter_inner a f | ||
|
|
||
| let pp = pp | ||
| let pp_serialized = pp | ||
|
|
||
|
Comment on lines
-22
to
-31
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. another thing :p i'm not sure these should be moved out of the functor; they could be but really im not sure why you'd need it; instead you need to e.g. use |
||
| let unwrap_alive = function | ||
| | None -> Symex.Result.ok None | ||
| | Some (Alive s) -> Symex.Result.ok (Some s) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,33 @@ and 'a node = NotOwned of node_qty | Owned of 'a | |
| and node_qty = Partially | Totally | ||
| [@@deriving show { with_path = false }, make] | ||
|
|
||
| type ('a, 'sint) serialized_atom = | ||
| | MemVal of { offset : 'sint; len : 'sint; v : 'a } | ||
| | Bound of 'sint | ||
| [@@deriving show { with_path = false }] | ||
|
|
||
| type ('a, 'sint) serialized = ('a, 'sint) serialized_atom list | ||
|
|
||
| let iter_vars_serialized iter_vars_serialized_val iter_vars_sint serialized f = | ||
| List.iter | ||
| (function | ||
| | MemVal { offset; len; v } -> | ||
| iter_vars_sint offset f; | ||
| iter_vars_sint len f; | ||
| iter_vars_serialized_val v f | ||
| | Bound v -> iter_vars_sint v f) | ||
| serialized | ||
|
|
||
| let subst_serialized subst_serialized_val subst_sint subst_var serialized = | ||
| let v_subst = subst_sint subst_var in | ||
| let subst_atom = function | ||
| | MemVal { offset; len; v } -> | ||
| let v = subst_serialized_val subst_var v in | ||
| MemVal { offset = v_subst offset; len = v_subst len; v } | ||
| | Bound v -> Bound (v_subst v) | ||
| in | ||
| List.map subst_atom serialized | ||
|
|
||
| (** The input module of [Tree_block]. A memory value [t] represents an owned | ||
| part of the tree block, with the property that it can be split or merged as | ||
| needed. | ||
|
|
@@ -534,16 +561,7 @@ struct | |
|
|
||
| (** Logic *) | ||
|
|
||
| type serialized_atom = | ||
| | MemVal of { | ||
| offset : sint; [@printer Symex.Value.ppa] | ||
| len : sint; [@printer Symex.Value.ppa] | ||
| v : MemVal.serialized; | ||
| } | ||
| | Bound of sint [@printer fun f v -> Fmt.pf f "Bound(%a)" Symex.Value.ppa v] | ||
| [@@deriving show { with_path = false }] | ||
|
|
||
| type serialized = serialized_atom list | ||
| type nonrec serialized = (MemVal.serialized, sint) serialized | ||
|
|
||
| let lift_miss ~offset ~len symex = | ||
| let+? fix = symex in | ||
|
|
@@ -613,27 +631,28 @@ struct | |
|
|
||
| (** Logic *) | ||
|
|
||
| let iter_vars_serialized serialized f = | ||
| let iter_vars_serialized = | ||
| iter_vars_serialized MemVal.iter_vars_serialized Symex.Value.iter_vars | ||
| in | ||
| iter_vars_serialized serialized f | ||
|
|
||
| let subst_serialized subst_var (serialized : serialized) = | ||
| let v_subst v = Symex.Value.subst subst_var v in | ||
| let subst_atom = function | ||
| | MemVal { offset; len; v } -> | ||
| let v = MemVal.subst_serialized subst_var v in | ||
| MemVal { offset = v_subst offset; len = v_subst len; v } | ||
| | Bound v -> Bound (v_subst v) | ||
| let subst_serialized = | ||
| subst_serialized MemVal.subst_serialized Symex.Value.subst | ||
| in | ||
| List.map subst_atom serialized | ||
| subst_serialized subst_var serialized | ||
|
|
||
| let iter_vars_serialized serialized f = | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same comment here -- i dont think any of these definitions should be moved out of the functor, just use the right module instead |
||
| List.iter | ||
| (function | ||
| | MemVal { offset; len; v } -> | ||
| Symex.Value.iter_vars offset f; | ||
| Symex.Value.iter_vars len f; | ||
| MemVal.iter_vars_serialized v f | ||
| | Bound v -> Symex.Value.iter_vars v f) | ||
| serialized | ||
|
|
||
| let pp_serialized = Fmt.Dump.list pp_serialized_atom | ||
| let pp_serialized_atom ft serialized = | ||
| let pp_serialized_atom = | ||
| pp_serialized_atom MemVal.pp_serialized Symex.Value.ppa | ||
| in | ||
| match serialized with | ||
| | Bound bound -> Fmt.pf ft "Bound(%a)" Symex.Value.ppa bound | ||
| | _ -> pp_serialized_atom ft serialized | ||
|
|
||
| let pp_serialized ft serialized = | ||
| Fmt.Dump.list pp_serialized_atom ft serialized | ||
|
|
||
| let serialize t = | ||
| let bound = | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than
State_intf.Template.t, with{ heap; globals }i would much rather haveWe'll probably need more information later on: for treeborrows maybe, for pointer decays (mapping a location to it's exposed integer value), for functions (mapping a function reference to a pointer? though i have other plans there anyways), and for errors (if the pre/post condition imply unwinding, what the error is)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this also corresponds much more closely to work on CSE with core predicates, so I'd prefer we stick to what we know works :P
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this change is directly inspired by a similar thing in Coteria. A serialized heap need not be decomposed in disjunctions of things? As long as I can produce/consume it it's fine
But, now that we're discussing this, maybe Opale is correct here, in that we have discovered with Pedro's work that we want atoms of the assertion to be as small as possible as to enable feasible matching plans
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to me it never made sense to have more than an atom is
serialized-- it makes using them harder; it should just beMissing of serialized list list, whereserializedis just atoms and is built to be kept small :PThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my intuition comes mostly from using core predicates within Gillian but i don't see a reason to complexify it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Efficiency. It might be that knowing at one time what you are going to produce/consume is cheaper than consuming atom by atom.
E.g. if I know that I'm going to produce a whole sub-tree, or maybe a whole object into memory, it's cheaper to do it in one step rather than doing it bit by bit and re-going through pmap and treeblock operations for each bit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fair enough :) but in that case id rather this only be done where it's relevant (
PMapandPListonly) and nowhere else; e.g. there is no advantage in producing globals and heap data "together" vs splitting it into twoUh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also to ease processing i wonder if as a utility function in e.g. the matching plan one can have some
flatten : serialized -> serialized listandmerge : serialized list -> serialized listthat e.g. converts(loc, [ ser1; ser2; ser3 ])to[(loc, [ ser1 ]); (loc, [ ser2 ]); (loc [ ser3 ])]and inversely (purely syntactically for themerge) -- maybe Pedro already has this