From 717ae16831deae50a031bb6f039b1e5a24becb48 Mon Sep 17 00:00:00 2001 From: plotchy <98172525+plotchy@users.noreply.github.com> Date: Wed, 3 Jul 2024 16:03:13 -0400 Subject: [PATCH 01/10] fix: expression filter_recursion shouldn't arenaize --- crates/graph/src/range/elem/expr/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/graph/src/range/elem/expr/mod.rs b/crates/graph/src/range/elem/expr/mod.rs index 5fd3cb74..4a7d90a7 100644 --- a/crates/graph/src/range/elem/expr/mod.rs +++ b/crates/graph/src/range/elem/expr/mod.rs @@ -384,7 +384,6 @@ impl RangeElem for RangeExpr { analyzer: &mut impl GraphBackend, arena: &mut RangeArena>, ) { - let _ = self.arenaize(analyzer, arena); self.lhs .filter_recursion(node_idx, new_idx, analyzer, arena); self.rhs From 7845f35e9f53ba7556bd6653f98b5177fc5113d0 Mon Sep 17 00:00:00 2001 From: brock elmore Date: Tue, 9 Jul 2024 16:57:06 -0700 Subject: [PATCH 02/10] is_representation_ok impl --- crates/graph/src/graph_elements.rs | 13 ++- crates/graph/src/nodes/context/invariants.rs | 88 +++++++++++++++ crates/graph/src/nodes/context/mod.rs | 1 + .../graph/src/nodes/context/var/invariants.rs | 102 +++++++++++++++++ crates/graph/src/nodes/context/var/mod.rs | 1 + crates/pyrometer/src/analyzer.rs | 106 +++++++++--------- crates/pyrometer/src/analyzer_backend.rs | 30 ++++- crates/shared/src/analyzer_like.rs | 7 +- crates/shared/src/error.rs | 31 +++++ .../src/context_builder/expr.rs | 6 +- .../src/context_builder/stmt.rs | 12 +- 11 files changed, 335 insertions(+), 62 deletions(-) create mode 100644 crates/graph/src/nodes/context/invariants.rs create mode 100644 crates/graph/src/nodes/context/var/invariants.rs diff --git a/crates/graph/src/graph_elements.rs b/crates/graph/src/graph_elements.rs index c365b503..9a5d3a69 100644 --- a/crates/graph/src/graph_elements.rs +++ b/crates/graph/src/graph_elements.rs @@ -1,7 +1,10 @@ use crate::elem::Elem; use crate::{nodes::*, VarType}; -use shared::{AnalyzerLike, GraphDot, GraphLike, Heirarchical, NodeIdx, RangeArena}; +use shared::{ + AnalyzerLike, GraphDot, GraphError, GraphLike, Heirarchical, NodeIdx, RangeArena, + RepresentationErr, +}; use lazy_static::lazy_static; use petgraph::{Directed, Graph}; @@ -9,6 +12,14 @@ use solang_parser::pt::{Identifier, Loc}; use std::collections::HashMap; +pub trait RepresentationInvariant { + fn is_representation_ok( + &self, + g: &impl GraphBackend, + arena: &RangeArena>, + ) -> Result, GraphError>; +} + pub trait GraphBackend: GraphLike> + GraphDot { diff --git a/crates/graph/src/nodes/context/invariants.rs b/crates/graph/src/nodes/context/invariants.rs new file mode 100644 index 00000000..ce8dfb07 --- /dev/null +++ b/crates/graph/src/nodes/context/invariants.rs @@ -0,0 +1,88 @@ +use std::collections::BTreeSet; + +use crate::{ + nodes::{Concrete, ContextNode, ContextVarNode}, + range::elem::Elem, + ContextEdge, Edge, GraphBackend, RepresentationInvariant, +}; +use shared::{ContextReprErr, GraphError, NodeIdx, RangeArena, RepresentationErr}; + +use petgraph::{visit::EdgeRef, Direction}; + +impl ContextNode { + fn cache_matches_edges( + &self, + g: &impl GraphBackend, + ) -> Result, GraphError> { + let mut vars: BTreeSet<_> = self + .underlying(g)? + .cache + .vars + .values() + .cloned() + .collect::>(); + vars.extend( + self.underlying(g)? + .cache + .tmp_vars + .values() + .cloned() + .collect::>(), + ); + let edge_vars: BTreeSet<_> = g + .graph() + .edges_directed(self.0.into(), Direction::Incoming) + .filter(|edge| *edge.weight() == Edge::Context(ContextEdge::Variable)) + .map(|e| ContextVarNode::from(e.source())) + .collect::>(); + if vars != edge_vars { + Ok(Some(RepresentationErr::Context( + self.0.into(), + ContextReprErr::VarCacheErr( + vars.symmetric_difference(&edge_vars) + .map(|i| i.0.into()) + .collect(), + ), + ))) + } else { + Ok(None) + } + } + + fn variables_invariants( + &self, + g: &impl GraphBackend, + arena: &RangeArena>, + ) -> Result, GraphError> { + let vars: Vec<_> = self.vars(g).values().collect(); + Ok(vars + .iter() + .map(|var| var.is_representation_ok(g, arena)) + .collect::>, _>>()? + .into_iter() + .flatten() + .collect::>()) + } +} + +impl RepresentationInvariant for ContextNode { + fn is_representation_ok( + &self, + g: &impl GraphBackend, + arena: &RangeArena>, + ) -> Result, GraphError> { + if let Some(err) = self.cache_matches_edges(g)? { + return Ok(Some(err)); + } + + let bad_vars = self.variables_invariants(g, arena)?; + if !bad_vars.is_empty() { + return Ok(Some(RepresentationErr::Context( + self.0.into(), + ContextReprErr::VarInvariantErr(bad_vars), + ))); + } + + Ok(None) + } +} diff --git a/crates/graph/src/nodes/context/mod.rs b/crates/graph/src/nodes/context/mod.rs index 4e8ac1ac..1d1e13d4 100644 --- a/crates/graph/src/nodes/context/mod.rs +++ b/crates/graph/src/nodes/context/mod.rs @@ -1,5 +1,6 @@ mod context_tys; mod expr_ret; +mod invariants; mod node; mod underlying; mod var; diff --git a/crates/graph/src/nodes/context/var/invariants.rs b/crates/graph/src/nodes/context/var/invariants.rs new file mode 100644 index 00000000..07bd4f3a --- /dev/null +++ b/crates/graph/src/nodes/context/var/invariants.rs @@ -0,0 +1,102 @@ +use crate::{ + nodes::{Builtin, Concrete, ContextNode, ContextVarNode, StructNode}, + range::elem::Elem, + ContextEdge, Edge, GraphBackend, Node, RepresentationInvariant, TypeNode, VarType, +}; +use shared::{GraphError, NodeIdx, RangeArena, RepresentationErr, VarReprErr}; + +use petgraph::{visit::EdgeRef, Direction}; + +impl ContextVarNode { + fn ty_check( + &self, + g: &impl GraphBackend, + arena: &RangeArena>, + ) -> Result, GraphError> { + match self.ty(g)? { + // we should have all types resolved by now + VarType::User(TypeNode::Unresolved(u), range) => { + Ok(Some(VarReprErr::Unresolved(self.0.into()).into())) + } + VarType::User(TypeNode::Contract(con), range) => Ok(None), + VarType::User(TypeNode::Struct(strt), range) => self.struct_has_all_fields(*strt, g), + VarType::User(TypeNode::Enum(enu), range) => Ok(None), + VarType::User(TypeNode::Ty(ty), range) => Ok(None), + VarType::User(TypeNode::Func(func), range) => Ok(None), + VarType::BuiltIn(bn, range) => { + let Node::Builtin(b) = g.node(*bn) else { + panic!("here") + }; + match b { + Builtin::Address | Builtin::Payable | Builtin::AddressPayable => Ok(None), + Builtin::Bool => Ok(None), + Builtin::String => Ok(None), + Builtin::Bytes(size) => Ok(None), + Builtin::DynamicBytes => Ok(None), + Builtin::Int(size) => Ok(None), + Builtin::Uint(size) => Ok(None), + Builtin::Array(inner_ty) => Ok(None), + Builtin::SizedArray(size, inner_ty) => Ok(None), + Builtin::Mapping(key_ty, v_ty) => Ok(None), + Builtin::Rational => Ok(None), + Builtin::Func(inputs, rets) => Ok(None), + } + } + VarType::Concrete(cn) => Ok(None), + } + } + + fn struct_has_all_fields( + &self, + strukt_node: StructNode, + g: &impl GraphBackend, + ) -> Result, GraphError> { + let fields: Vec<_> = g + .graph() + .edges_directed(self.0.into(), Direction::Incoming) + .filter(|e| *e.weight() == Edge::Context(ContextEdge::AttrAccess("field"))) + .map(|e| ContextVarNode::from(e.source())) + .collect(); + + let struct_fields = strukt_node.fields(g); + + let mut real_field_names: Vec = fields + .iter() + .map(|field| field.name(g).unwrap()) + .map(|name| { + name.split('.') + .collect::>() + .last() + .unwrap() + .to_string() + }) + .collect(); + let mut target_field_names: Vec<_> = struct_fields + .iter() + .map(|field| field.name(g).unwrap()) + .collect(); + + real_field_names.sort(); + target_field_names.sort(); + if real_field_names != target_field_names { + Ok(Some( + VarReprErr::StructErr(self.0.into(), "Missing fields").into(), + )) + } else { + Ok(None) + } + } +} + +impl RepresentationInvariant for ContextVarNode { + fn is_representation_ok( + &self, + g: &impl GraphBackend, + arena: &RangeArena>, + ) -> Result, GraphError> { + if let Some(ty_bad) = self.ty_check(g, arena)? { + return Ok(Some(ty_bad)); + } + Ok(None) + } +} diff --git a/crates/graph/src/nodes/context/var/mod.rs b/crates/graph/src/nodes/context/var/mod.rs index 1f665e95..aa5ef126 100644 --- a/crates/graph/src/nodes/context/var/mod.rs +++ b/crates/graph/src/nodes/context/var/mod.rs @@ -1,3 +1,4 @@ +mod invariants; mod node; mod ranging; mod typing; diff --git a/crates/pyrometer/src/analyzer.rs b/crates/pyrometer/src/analyzer.rs index 4825fbf2..f891b054 100644 --- a/crates/pyrometer/src/analyzer.rs +++ b/crates/pyrometer/src/analyzer.rs @@ -566,59 +566,59 @@ impl Analyzer { }); elems.into_iter().for_each(|final_pass_item| { - final_pass_item - .funcs - .iter() - .for_each(|func| self.analyze_fn_calls(*func)); - let mut func_mapping = BTreeMap::default(); - let mut call_dep_graph: StableGraph = StableGraph::default(); - let fn_calls_fns = std::mem::take(&mut self.fn_calls_fns); - fn_calls_fns.iter().for_each(|(func, calls)| { - if !calls.is_empty() { - let func_idx = if let Some(idx) = func_mapping.get(func) { - *idx - } else { - let idx = call_dep_graph.add_node(*func); - func_mapping.insert(func, idx); - idx - }; - - calls.iter().for_each(|call| { - let call_idx = if let Some(idx) = func_mapping.get(call) { - *idx - } else { - let idx = call_dep_graph.add_node(*call); - func_mapping.insert(call, idx); - idx - }; - - call_dep_graph.add_edge(func_idx, call_idx, 0); - }); - } else { - self.handled_funcs.push(*func); - if let Some(body) = &func.underlying(self).unwrap().body.clone() { - self.parse_ctx_statement(arena, body, false, Some(*func)); - } - } - }); - - let mut res = petgraph::algo::toposort(&call_dep_graph, None); - while let Err(cycle) = res { - call_dep_graph.remove_node(cycle.node_id()); - res = petgraph::algo::toposort(&call_dep_graph, None); - } - - let indices = res.unwrap(); + // final_pass_item + // .funcs + // .iter() + // .for_each(|func| self.analyze_fn_calls(*func)); + // let mut func_mapping = BTreeMap::default(); + // let mut call_dep_graph: StableGraph = StableGraph::default(); + // let fn_calls_fns = std::mem::take(&mut self.fn_calls_fns); + // fn_calls_fns.iter().for_each(|(func, calls)| { + // if !calls.is_empty() { + // let func_idx = if let Some(idx) = func_mapping.get(func) { + // *idx + // } else { + // let idx = call_dep_graph.add_node(*func); + // func_mapping.insert(func, idx); + // idx + // }; + + // calls.iter().for_each(|call| { + // let call_idx = if let Some(idx) = func_mapping.get(call) { + // *idx + // } else { + // let idx = call_dep_graph.add_node(*call); + // func_mapping.insert(call, idx); + // idx + // }; + + // call_dep_graph.add_edge(func_idx, call_idx, 0); + // }); + // } else { + // self.handled_funcs.push(*func); + // if let Some(body) = &func.underlying(self).unwrap().body.clone() { + // self.parse_ctx_statement(arena, body, false, Some(*func)); + // } + // } + // }); - indices.iter().for_each(|idx| { - let func = call_dep_graph.node_weight(*idx).unwrap(); - if !self.handled_funcs.contains(func) { - self.handled_funcs.push(*func); - if let Some(body) = &func.underlying(self).unwrap().body.clone() { - self.parse_ctx_statement(arena, body, false, Some(*func)); - } - } - }); + // let mut res = petgraph::algo::toposort(&call_dep_graph, None); + // while let Err(cycle) = res { + // call_dep_graph.remove_node(cycle.node_id()); + // res = petgraph::algo::toposort(&call_dep_graph, None); + // } + + // let indices = res.unwrap(); + + // indices.iter().for_each(|idx| { + // let func = call_dep_graph.node_weight(*idx).unwrap(); + // if !self.handled_funcs.contains(func) { + // self.handled_funcs.push(*func); + // if let Some(body) = &func.underlying(self).unwrap().body.clone() { + // self.parse_ctx_statement(arena, body, false, Some(*func)); + // } + // } + // }); final_pass_item.funcs.into_iter().for_each(|func| { if !self.handled_funcs.contains(&func) { @@ -628,7 +628,7 @@ impl Analyzer { } }); - self.fn_calls_fns = fn_calls_fns; + // self.fn_calls_fns = fn_calls_fns; }); } diff --git a/crates/pyrometer/src/analyzer_backend.rs b/crates/pyrometer/src/analyzer_backend.rs index 5f05e91f..fe6bec61 100644 --- a/crates/pyrometer/src/analyzer_backend.rs +++ b/crates/pyrometer/src/analyzer_backend.rs @@ -2,12 +2,15 @@ use crate::Analyzer; use graph::{ elem::Elem, nodes::{ - BlockNode, Builtin, Concrete, ConcreteNode, ContextVar, Function, FunctionNode, - FunctionParam, FunctionParamNode, FunctionReturn, MsgNode, + BlockNode, Builtin, Concrete, ConcreteNode, ContextNode, ContextVar, Function, + FunctionNode, FunctionParam, FunctionParamNode, FunctionReturn, MsgNode, }, - AnalyzerBackend, Edge, Node, VarType, + AnalyzerBackend, Edge, Node, RepresentationInvariant, VarType, +}; +use shared::{ + AnalyzerLike, ExprErr, GraphError, GraphLike, IntoExprErr, JoinStats, NodeIdx, RangeArena, + RepresentationErr, }; -use shared::{AnalyzerLike, ExprErr, GraphLike, IntoExprErr, JoinStats, NodeIdx, RangeArena}; use ahash::AHashMap; use ethers_core::types::U256; @@ -287,4 +290,23 @@ impl AnalyzerLike for Analyzer { } file_mapping } + + fn is_representation_ok( + &self, + arena: &RangeArena<::RangeElem>, + ) -> Result, GraphError> { + let t: Vec<_> = self + .graph() + .node_indices() + .map(|idx| match self.node(idx) { + Node::Context(..) => ContextNode::from(idx).is_representation_ok(self, arena), + _ => Ok(None), + }) + .collect::>, GraphError>>()? + .into_iter() + .flatten() + .collect(); + + Ok(t) + } } diff --git a/crates/shared/src/analyzer_like.rs b/crates/shared/src/analyzer_like.rs index 1145e4af..e8e73452 100644 --- a/crates/shared/src/analyzer_like.rs +++ b/crates/shared/src/analyzer_like.rs @@ -1,4 +1,4 @@ -use crate::{GraphLike, NodeIdx, RangeArena}; +use crate::{GraphError, GraphLike, NodeIdx, RangeArena, RepresentationErr}; use ahash::AHashMap; @@ -184,4 +184,9 @@ pub trait AnalyzerLike: GraphLike { fn handled_funcs(&self) -> &[Self::FunctionNode]; fn handled_funcs_mut(&mut self) -> &mut Vec; fn file_mapping(&self) -> BTreeMap; + + fn is_representation_ok( + &self, + arena: &RangeArena<::RangeElem>, + ) -> Result, GraphError>; } diff --git a/crates/shared/src/error.rs b/crates/shared/src/error.rs index 4abc5833..194a5a3b 100644 --- a/crates/shared/src/error.rs +++ b/crates/shared/src/error.rs @@ -1,5 +1,36 @@ +use crate::NodeIdx; use solang_parser::pt::Loc; +#[derive(Debug, Clone, Ord, Eq, PartialEq, PartialOrd)] +pub enum RepresentationErr { + Context(NodeIdx, ContextReprErr), + Variable(VarReprErr), + Graph(GraphError), +} + +#[derive(Debug, Clone, Ord, Eq, PartialEq, PartialOrd)] +pub enum ContextReprErr { + VarCacheErr(Vec), + VarInvariantErr(Vec), +} + +#[derive(Debug, Clone, Ord, Eq, PartialEq, PartialOrd)] +pub enum VarReprErr { + Unresolved(NodeIdx), + StructErr(NodeIdx, &'static str), + ContractErr(NodeIdx, &'static str), + EnumErr(NodeIdx, &'static str), + TyErr(NodeIdx, &'static str), + FuncErr(NodeIdx, &'static str), + ArrayErr(NodeIdx, &'static str), +} + +impl From for RepresentationErr { + fn from(vre: VarReprErr) -> Self { + RepresentationErr::Variable(vre) + } +} + #[derive(Debug, Clone, Ord, Eq, PartialEq, PartialOrd)] pub enum GraphError { /// The analyzer thought the node was suppose to be one type, but it was a different one diff --git a/crates/solc-expressions/src/context_builder/expr.rs b/crates/solc-expressions/src/context_builder/expr.rs index 476e980f..61533304 100644 --- a/crates/solc-expressions/src/context_builder/expr.rs +++ b/crates/solc-expressions/src/context_builder/expr.rs @@ -33,7 +33,7 @@ pub trait ExpressionParser: expr: &Expression, ctx: ContextNode, ) -> Result<(), ExprErr> { - if !ctx.killed_or_ret(self).unwrap() { + let res = if !ctx.killed_or_ret(self).unwrap() { let edges = ctx.live_edges(self).into_expr_err(expr.loc())?; if edges.is_empty() { self.parse_ctx_expr_inner(arena, expr, ctx) @@ -45,7 +45,9 @@ pub trait ExpressionParser: } } else { Ok(()) - } + }; + + Ok(()) } #[tracing::instrument(level = "trace", skip_all, fields(ctx = %ctx.path(self).replace('.', "\n\t.")))] diff --git a/crates/solc-expressions/src/context_builder/stmt.rs b/crates/solc-expressions/src/context_builder/stmt.rs index 51f24c90..e8ed2541 100644 --- a/crates/solc-expressions/src/context_builder/stmt.rs +++ b/crates/solc-expressions/src/context_builder/stmt.rs @@ -41,7 +41,7 @@ pub trait StatementParser: ) where Self: Sized, { - if let Some(parent) = parent_ctx { + let res = if let Some(parent) = parent_ctx { match self.node(parent) { Node::Context(_) => { let ctx = ContextNode::from(parent.into()); @@ -68,7 +68,17 @@ pub trait StatementParser: } } else { self.parse_ctx_stmt_inner(arena, stmt, unchecked, parent_ctx) + }; + + if let Some(errs) = + self.add_if_err(self.is_representation_ok(arena).into_expr_err(stmt.loc())) + { + if !errs.is_empty() { + panic!("bad nodes: {errs:#?}"); + } } + + res } #[tracing::instrument(level = "trace", skip_all)] From 52f777bc638797e92cb746bd98b0d25ae363421c Mon Sep 17 00:00:00 2001 From: brock elmore Date: Wed, 10 Jul 2024 11:02:13 -0700 Subject: [PATCH 03/10] debug site fix and invariant work --- crates/graph/src/graph_elements.rs | 10 ++-- crates/graph/src/nodes/context/invariants.rs | 22 +++---- .../graph/src/nodes/context/var/invariants.rs | 13 ++++- crates/pyrometer/src/analyzer_backend.rs | 2 +- crates/pyrometer/src/graph_backend.rs | 26 ++++----- crates/shared/src/error.rs | 58 +++++++++++++++++-- crates/shared/src/graph_like.rs | 17 +++--- .../src/context_builder/expr.rs | 6 +- .../src/context_builder/stmt.rs | 20 ++++--- 9 files changed, 115 insertions(+), 59 deletions(-) diff --git a/crates/graph/src/graph_elements.rs b/crates/graph/src/graph_elements.rs index 9a5d3a69..0618bb3b 100644 --- a/crates/graph/src/graph_elements.rs +++ b/crates/graph/src/graph_elements.rs @@ -405,16 +405,14 @@ impl GraphLike for DummyGraph { impl GraphBackend for DummyGraph {} impl GraphDot for DummyGraph { - type T = Elem; - - fn dot_str(&self, _arena: &mut RangeArena<::T>) -> String { + fn dot_str(&self, _arena: &mut RangeArena<::RangeElem>) -> String { // Provide a basic implementation or a placeholder "digraph DummyGraph {}".to_string() } fn cluster_str( &self, - _arena: &mut RangeArena, + _arena: &mut RangeArena<::RangeElem>, _node: NodeIdx, _cluster_num: &mut usize, _is_killed: bool, @@ -431,14 +429,14 @@ impl GraphDot for DummyGraph { todo!() } - fn dot_str_no_tmps(&self, _arena: &mut RangeArena) -> String + fn dot_str_no_tmps(&self, _arena: &mut RangeArena<::RangeElem>) -> String where Self: std::marker::Sized, { todo!() } - fn mermaid_str(&self, _arena: &mut RangeArena) -> String + fn mermaid_str(&self, _arena: &mut RangeArena<::RangeElem>) -> String where Self: std::marker::Sized, { diff --git a/crates/graph/src/nodes/context/invariants.rs b/crates/graph/src/nodes/context/invariants.rs index ce8dfb07..3a93d252 100644 --- a/crates/graph/src/nodes/context/invariants.rs +++ b/crates/graph/src/nodes/context/invariants.rs @@ -35,15 +35,12 @@ impl ContextNode { .filter(|edge| *edge.weight() == Edge::Context(ContextEdge::Variable)) .map(|e| ContextVarNode::from(e.source())) .collect::>(); - if vars != edge_vars { - Ok(Some(RepresentationErr::Context( - self.0.into(), - ContextReprErr::VarCacheErr( - vars.symmetric_difference(&edge_vars) - .map(|i| i.0.into()) - .collect(), - ), - ))) + + let diff: Vec<_> = edge_vars.difference(&vars).map(|i| i.0.into()).collect(); + if !diff.is_empty() { + Ok(Some( + ContextReprErr::VarCacheErr(self.0.into(), diff).into(), + )) } else { Ok(None) } @@ -77,10 +74,9 @@ impl RepresentationInvariant for ContextNode { let bad_vars = self.variables_invariants(g, arena)?; if !bad_vars.is_empty() { - return Ok(Some(RepresentationErr::Context( - self.0.into(), - ContextReprErr::VarInvariantErr(bad_vars), - ))); + return Ok(Some( + ContextReprErr::VarInvariantErr(self.0.into(), bad_vars).into(), + )); } Ok(None) diff --git a/crates/graph/src/nodes/context/var/invariants.rs b/crates/graph/src/nodes/context/var/invariants.rs index 07bd4f3a..b16b9f23 100644 --- a/crates/graph/src/nodes/context/var/invariants.rs +++ b/crates/graph/src/nodes/context/var/invariants.rs @@ -19,7 +19,7 @@ impl ContextVarNode { Ok(Some(VarReprErr::Unresolved(self.0.into()).into())) } VarType::User(TypeNode::Contract(con), range) => Ok(None), - VarType::User(TypeNode::Struct(strt), range) => self.struct_has_all_fields(*strt, g), + VarType::User(TypeNode::Struct(strt), range) => self.struct_invariants(*strt, g), VarType::User(TypeNode::Enum(enu), range) => Ok(None), VarType::User(TypeNode::Ty(ty), range) => Ok(None), VarType::User(TypeNode::Func(func), range) => Ok(None), @@ -46,6 +46,17 @@ impl ContextVarNode { } } + fn struct_invariants( + &self, + strukt_node: StructNode, + g: &impl GraphBackend, + ) -> Result, GraphError> { + if let Some(err) = self.struct_has_all_fields(strukt_node, g)? { + return Ok(Some(err)); + } + Ok(None) + } + fn struct_has_all_fields( &self, strukt_node: StructNode, diff --git a/crates/pyrometer/src/analyzer_backend.rs b/crates/pyrometer/src/analyzer_backend.rs index fe6bec61..181f1b39 100644 --- a/crates/pyrometer/src/analyzer_backend.rs +++ b/crates/pyrometer/src/analyzer_backend.rs @@ -9,7 +9,7 @@ use graph::{ }; use shared::{ AnalyzerLike, ExprErr, GraphError, GraphLike, IntoExprErr, JoinStats, NodeIdx, RangeArena, - RepresentationErr, + RepresentationErr, USE_DEBUG_SITE, }; use ahash::AHashMap; diff --git a/crates/pyrometer/src/graph_backend.rs b/crates/pyrometer/src/graph_backend.rs index 8fd62682..7a5f5536 100644 --- a/crates/pyrometer/src/graph_backend.rs +++ b/crates/pyrometer/src/graph_backend.rs @@ -475,10 +475,10 @@ impl Elems { } // Ensure the graph does not have a cycle - debug_assert!( - !petgraph::algo::is_cyclic_directed(&graph), - "The graph contains a cycle!" - ); + // debug_assert!( + // !petgraph::algo::is_cyclic_directed(&graph), + // "The graph contains a cycle!" + // ); graph } @@ -603,7 +603,6 @@ fn _calculate_hash(t: &T) -> u64 { impl GraphBackend for Analyzer {} impl GraphDot for Analyzer { - type T = Elem; fn cluster_str( &self, arena: &mut RangeArena>, @@ -1191,11 +1190,9 @@ impl GraphLike for G<'_> { } impl GraphDot for G<'_> { - type T = Elem; - fn cluster_str( &self, - _arena: &mut RangeArena, + _arena: &mut RangeArena<::RangeElem>, _node: NodeIdx, _cluster_num: &mut usize, _is_killed: bool, @@ -1210,7 +1207,7 @@ impl GraphDot for G<'_> { todo!() } - fn dot_str(&self, _arena: &mut RangeArena) -> String + fn dot_str(&self, _arena: &mut RangeArena<::RangeElem>) -> String where Self: std::marker::Sized, Self: shared::AnalyzerLike, @@ -1218,7 +1215,7 @@ impl GraphDot for G<'_> { todo!() } - fn dot_str_no_tmps(&self, _arena: &mut RangeArena) -> String + fn dot_str_no_tmps(&self, _arena: &mut RangeArena<::RangeElem>) -> String where Self: std::marker::Sized, Self: GraphLike + shared::AnalyzerLike, @@ -1226,7 +1223,7 @@ impl GraphDot for G<'_> { todo!() } - fn mermaid_str(&self, _arena: &mut RangeArena) -> String + fn mermaid_str(&self, _arena: &mut RangeArena<::RangeElem>) -> String where Self: std::marker::Sized, Self: shared::AnalyzerLike, @@ -1275,8 +1272,11 @@ pub fn mermaid_node( } // color the forks - let ctx_node = graph::nodes::ContextVarNode::from(current_node).ctx(g); - gather_context_info(g, indent, ctx_node, current_node, &mut node_str); + if let Some(ctx_node) = + graph::nodes::ContextVarNode::from(current_node).maybe_ctx(g) + { + gather_context_info(g, indent, ctx_node, current_node, &mut node_str); + } } Node::Context(ctx) => { // highlight self diff --git a/crates/shared/src/error.rs b/crates/shared/src/error.rs index 194a5a3b..80b1dd5b 100644 --- a/crates/shared/src/error.rs +++ b/crates/shared/src/error.rs @@ -3,15 +3,36 @@ use solang_parser::pt::Loc; #[derive(Debug, Clone, Ord, Eq, PartialEq, PartialOrd)] pub enum RepresentationErr { - Context(NodeIdx, ContextReprErr), + Context(ContextReprErr), Variable(VarReprErr), - Graph(GraphError), +} + +impl RepresentationErr { + fn msg(&self) -> &str { + match self { + Self::Context(c) => c.msg(), + Self::Variable(c) => c.msg(), + } + } } #[derive(Debug, Clone, Ord, Eq, PartialEq, PartialOrd)] pub enum ContextReprErr { - VarCacheErr(Vec), - VarInvariantErr(Vec), + VarCacheErr(NodeIdx, Vec), + VarInvariantErr(NodeIdx, Vec), +} + +impl ContextReprErr { + fn msg(&self) -> &str { + match self { + Self::VarCacheErr(idx, nodes) => string_to_static_str(format!("context node {idx:?}: has a cache-edge mismatch: {nodes:?} are missing in either cache or edges")), + Self::VarInvariantErr(idx, errs) => string_to_static_str(format!("context node {idx:?}: has variable invariant errors: {}", errs.iter().map(|e| e.msg()).collect::>().join(",\n\t"))), + } + } +} + +fn string_to_static_str(s: String) -> &'static str { + Box::leak(s.into_boxed_str()) } #[derive(Debug, Clone, Ord, Eq, PartialEq, PartialOrd)] @@ -25,12 +46,32 @@ pub enum VarReprErr { ArrayErr(NodeIdx, &'static str), } +impl VarReprErr { + fn msg(&self) -> &str { + string_to_static_str(match self { + Self::Unresolved(idx) => format!("context variable node {idx:?}: is unresolved"), + Self::StructErr(idx, str) => format!("context variable node {idx:?}: {str}"), + Self::ContractErr(idx, str) => format!("context variable node {idx:?}: {str}"), + Self::EnumErr(idx, str) => format!("context variable node {idx:?}: {str}"), + Self::TyErr(idx, str) => format!("context variable node {idx:?}: {str}"), + Self::FuncErr(idx, str) => format!("context variable node {idx:?}: {str}"), + Self::ArrayErr(idx, str) => format!("context variable node {idx:?}: {str}"), + }) + } +} + impl From for RepresentationErr { fn from(vre: VarReprErr) -> Self { RepresentationErr::Variable(vre) } } +impl From for RepresentationErr { + fn from(cre: ContextReprErr) -> Self { + RepresentationErr::Context(cre) + } +} + #[derive(Debug, Clone, Ord, Eq, PartialEq, PartialOrd)] pub enum GraphError { /// The analyzer thought the node was suppose to be one type, but it was a different one @@ -104,6 +145,7 @@ pub enum ExprErr { InvalidFunctionInput(Loc, String), TakeFromFork(Loc, String), GraphError(Loc, GraphError), + ReprError(Loc, RepresentationErr), Unresolved(Loc, String), } @@ -112,6 +154,10 @@ impl ExprErr { pub fn from_graph_err(loc: Loc, graph_err: GraphError) -> Self { Self::GraphError(loc, graph_err) } + + pub fn from_repr_err(loc: Loc, repr_err: RepresentationErr) -> Self { + Self::ReprError(loc, repr_err) + } } impl ExprErr { @@ -141,6 +187,7 @@ impl ExprErr { TakeFromFork(loc, ..) => *loc, GraphError(loc, ..) => *loc, Unresolved(loc, ..) => *loc, + ReprError(loc, ..) => *loc, } } @@ -168,6 +215,8 @@ impl ExprErr { ExprErr::InvalidFunctionInput(_, msg, ..) => msg, ExprErr::TakeFromFork(_, msg, ..) => msg, ExprErr::Unresolved(_, msg, ..) => msg, + ExprErr::ReprError(_, RepresentationErr::Context(c)) => c.msg(), + ExprErr::ReprError(_, RepresentationErr::Variable(v)) => v.msg(), ExprErr::GraphError(_, GraphError::NodeConfusion(msg), ..) => msg, ExprErr::GraphError(_, GraphError::MaxStackDepthReached(msg), ..) => msg, ExprErr::GraphError(_, GraphError::MaxStackWidthReached(msg), ..) => msg, @@ -205,6 +254,7 @@ impl ExprErr { ExprErr::IntrinsicNamedArgs(..) => "Arguments in calls to intrinsic functions cannot be named", ExprErr::InvalidFunctionInput(..) => "Arguments to this function call do not match required types", ExprErr::TakeFromFork(..) => "IR Error: Tried to take from an child context that ended up forking", + ExprErr::ReprError(..) => "Representation Error: This is a bug.", ExprErr::GraphError(_, GraphError::NodeConfusion(_), ..) => "Graph IR Error: Node type confusion. This is potentially a bug. Please report it at https://github.com/nascentxyz/pyrometer", ExprErr::GraphError(_, GraphError::MaxStackDepthReached(_), ..) => "Max call depth reached - either recursion or loop", ExprErr::GraphError(_, GraphError::MaxStackWidthReached(_), ..) => "TODO: Max fork width reached - Need to widen variables and remove contexts", diff --git a/crates/shared/src/graph_like.rs b/crates/shared/src/graph_like.rs index ae073436..dbea9204 100644 --- a/crates/shared/src/graph_like.rs +++ b/crates/shared/src/graph_like.rs @@ -70,9 +70,8 @@ pub trait GraphLike { /// A trait that constructs dot-like visualization strings (either mermaid or graphviz) pub trait GraphDot: GraphLike { - type T: Hash; /// Open a dot using graphviz - fn open_dot(&self, arena: &mut RangeArena) + fn open_dot(&self, arena: &mut RangeArena<::RangeElem>) where Self: std::marker::Sized, Self: AnalyzerLike, @@ -102,7 +101,7 @@ pub trait GraphDot: GraphLike { .expect("failed to execute process"); } - fn open_mermaid(&self, arena: &mut RangeArena) + fn open_mermaid(&self, arena: &mut RangeArena<::RangeElem>) where Self: std::marker::Sized, Self: AnalyzerLike, @@ -149,7 +148,7 @@ pub trait GraphDot: GraphLike { /// Creates a subgraph for visually identifying contexts and subcontexts fn cluster_str( &self, - arena: &mut RangeArena, + arena: &mut RangeArena<::RangeElem>, node: NodeIdx, cluster_num: &mut usize, is_killed: bool, @@ -162,18 +161,18 @@ pub trait GraphDot: GraphLike { Self: std::marker::Sized; /// Constructs a dot string - fn dot_str(&self, arena: &mut RangeArena) -> String + fn dot_str(&self, arena: &mut RangeArena<::RangeElem>) -> String where Self: std::marker::Sized, Self: AnalyzerLike; /// Construct a dot string while filtering temporary variables - fn dot_str_no_tmps(&self, arena: &mut RangeArena) -> String + fn dot_str_no_tmps(&self, arena: &mut RangeArena<::RangeElem>) -> String where Self: std::marker::Sized, Self: GraphLike + AnalyzerLike; - fn mermaid_str(&self, arena: &mut RangeArena) -> String + fn mermaid_str(&self, arena: &mut RangeArena<::RangeElem>) -> String where Self: std::marker::Sized, Self: AnalyzerLike; @@ -185,7 +184,7 @@ struct GraphMessage { timestamp: u64, } -pub fn post_to_site(graph: &G, arena: &mut RangeArena) +pub fn post_to_site(graph: &G, arena: &mut RangeArena<::RangeElem>) where G: GraphDot + AnalyzerLike, { @@ -195,7 +194,7 @@ where }); } -async fn post_to_site_async(graph: &G, arena: &mut RangeArena) +async fn post_to_site_async(graph: &G, arena: &mut RangeArena<::RangeElem>) where G: GraphDot + AnalyzerLike, { diff --git a/crates/solc-expressions/src/context_builder/expr.rs b/crates/solc-expressions/src/context_builder/expr.rs index 61533304..476e980f 100644 --- a/crates/solc-expressions/src/context_builder/expr.rs +++ b/crates/solc-expressions/src/context_builder/expr.rs @@ -33,7 +33,7 @@ pub trait ExpressionParser: expr: &Expression, ctx: ContextNode, ) -> Result<(), ExprErr> { - let res = if !ctx.killed_or_ret(self).unwrap() { + if !ctx.killed_or_ret(self).unwrap() { let edges = ctx.live_edges(self).into_expr_err(expr.loc())?; if edges.is_empty() { self.parse_ctx_expr_inner(arena, expr, ctx) @@ -45,9 +45,7 @@ pub trait ExpressionParser: } } else { Ok(()) - }; - - Ok(()) + } } #[tracing::instrument(level = "trace", skip_all, fields(ctx = %ctx.path(self).replace('.', "\n\t.")))] diff --git a/crates/solc-expressions/src/context_builder/stmt.rs b/crates/solc-expressions/src/context_builder/stmt.rs index e8ed2541..53c108f3 100644 --- a/crates/solc-expressions/src/context_builder/stmt.rs +++ b/crates/solc-expressions/src/context_builder/stmt.rs @@ -14,7 +14,9 @@ use graph::{ }, AnalyzerBackend, ContextEdge, Edge, Node, }; -use shared::{ExprErr, IntoExprErr, NodeIdx, RangeArena}; +use shared::{ + post_to_site, AnalyzerLike, ExprErr, GraphDot, IntoExprErr, NodeIdx, RangeArena, USE_DEBUG_SITE, +}; use petgraph::{visit::EdgeRef, Direction}; use solang_parser::{ @@ -41,7 +43,7 @@ pub trait StatementParser: ) where Self: Sized, { - let res = if let Some(parent) = parent_ctx { + if let Some(parent) = parent_ctx { match self.node(parent) { Node::Context(_) => { let ctx = ContextNode::from(parent.into()); @@ -68,17 +70,19 @@ pub trait StatementParser: } } else { self.parse_ctx_stmt_inner(arena, stmt, unchecked, parent_ctx) - }; + } + + if unsafe { USE_DEBUG_SITE } { + post_to_site(&*self, arena); + } if let Some(errs) = self.add_if_err(self.is_representation_ok(arena).into_expr_err(stmt.loc())) { - if !errs.is_empty() { - panic!("bad nodes: {errs:#?}"); - } + errs.into_iter().for_each(|err| { + self.add_expr_err(ExprErr::from_repr_err(stmt.loc(), err)); + }); } - - res } #[tracing::instrument(level = "trace", skip_all)] From e88538249863f378f5c1186752d818d2bff65431 Mon Sep 17 00:00:00 2001 From: brock elmore Date: Wed, 10 Jul 2024 14:11:40 -0700 Subject: [PATCH 04/10] add test commands --- crates/graph/src/lib.rs | 2 + crates/graph/src/nodes/builtin.rs | 11 ++ crates/graph/src/nodes/concrete.rs | 8 + crates/graph/src/nodes/context/invariants.rs | 6 +- crates/graph/src/nodes/context/var/typing.rs | 4 + crates/graph/src/test_command.rs | 19 +++ crates/graph/src/var_type.rs | 8 + crates/pyrometer/tests/test_data/bitwise.sol | 137 +++++++++++++----- .../pyrometer/tests/test_data/dyn_types.sol | 12 +- .../tests/test_data/repros/issue69.sol | 10 +- crates/shared/src/error.rs | 4 + crates/shared/src/search.rs | 5 - .../src/context_builder/expr.rs | 13 +- .../src/context_builder/mod.rs | 2 + .../src/context_builder/stmt.rs | 34 ++++- .../context_builder/test_command_runner.rs | 113 +++++++++++++++ .../src/func_call/intrinsic_call/types.rs | 2 +- crates/solc-expressions/src/literal.rs | 89 ++++++++++-- 18 files changed, 408 insertions(+), 71 deletions(-) create mode 100644 crates/graph/src/test_command.rs create mode 100644 crates/solc-expressions/src/context_builder/test_command_runner.rs diff --git a/crates/graph/src/lib.rs b/crates/graph/src/lib.rs index 2f35381d..01ca3924 100644 --- a/crates/graph/src/lib.rs +++ b/crates/graph/src/lib.rs @@ -1,5 +1,6 @@ mod graph_elements; mod range; +mod test_command; mod var_type; pub mod nodes; @@ -7,4 +8,5 @@ pub mod solvers; pub use graph_elements::*; pub use range::*; +pub use test_command::*; pub use var_type::*; diff --git a/crates/graph/src/nodes/builtin.rs b/crates/graph/src/nodes/builtin.rs index 07968223..98f1080c 100644 --- a/crates/graph/src/nodes/builtin.rs +++ b/crates/graph/src/nodes/builtin.rs @@ -110,6 +110,10 @@ impl BuiltInNode { Ok(self.underlying(analyzer)?.is_indexable()) } + pub fn needs_length(&self, analyzer: &impl GraphBackend) -> Result { + Ok(self.underlying(analyzer)?.needs_length()) + } + /// Returns the zero range for this builtin type, i.e. uint256 -> [0, 0] pub fn zero_range( &self, @@ -353,6 +357,13 @@ impl Builtin { ) } + pub fn needs_length(&self) -> bool { + matches!( + self, + Builtin::DynamicBytes | Builtin::Array(..) | Builtin::SizedArray(..) | Builtin::String + ) + } + /// Checks if self is implicitly castable to another builtin pub fn implicitly_castable_to(&self, other: &Self) -> bool { use Builtin::*; diff --git a/crates/graph/src/nodes/concrete.rs b/crates/graph/src/nodes/concrete.rs index f0521974..88281034 100644 --- a/crates/graph/src/nodes/concrete.rs +++ b/crates/graph/src/nodes/concrete.rs @@ -64,6 +64,10 @@ impl ConcreteNode { pub fn is_indexable(&self, analyzer: &impl GraphBackend) -> Result { Ok(self.underlying(analyzer)?.is_indexable()) } + + pub fn needs_length(&self, analyzer: &impl GraphBackend) -> Result { + Ok(self.underlying(analyzer)?.needs_length()) + } } impl From for ConcreteNode { @@ -284,6 +288,10 @@ impl Concrete { self.is_dyn() || matches!(self, Concrete::Bytes(..)) } + pub fn needs_length(&self) -> bool { + self.is_dyn() + } + /// Convert a U256 back into it's original type. This is used mostly /// for range calculations to improve ergonomics. Basically /// the EVM only operates on U256 words, so most of this should diff --git a/crates/graph/src/nodes/context/invariants.rs b/crates/graph/src/nodes/context/invariants.rs index 3a93d252..d7d82903 100644 --- a/crates/graph/src/nodes/context/invariants.rs +++ b/crates/graph/src/nodes/context/invariants.rs @@ -68,9 +68,9 @@ impl RepresentationInvariant for ContextNode { g: &impl GraphBackend, arena: &RangeArena>, ) -> Result, GraphError> { - if let Some(err) = self.cache_matches_edges(g)? { - return Ok(Some(err)); - } + // if let Some(err) = self.cache_matches_edges(g)? { + // return Ok(Some(err)); + // } let bad_vars = self.variables_invariants(g, arena)?; if !bad_vars.is_empty() { diff --git a/crates/graph/src/nodes/context/var/typing.rs b/crates/graph/src/nodes/context/var/typing.rs index 132054df..0c835d3d 100644 --- a/crates/graph/src/nodes/context/var/typing.rs +++ b/crates/graph/src/nodes/context/var/typing.rs @@ -75,6 +75,10 @@ impl ContextVarNode { self.ty(analyzer)?.is_indexable(analyzer) } + pub fn needs_length(&self, analyzer: &impl GraphBackend) -> Result { + self.ty(analyzer)?.needs_length(analyzer) + } + pub fn is_storage(&self, analyzer: &impl GraphBackend) -> Result { Ok(matches!( self.underlying(analyzer)?.storage, diff --git a/crates/graph/src/test_command.rs b/crates/graph/src/test_command.rs new file mode 100644 index 00000000..99fb0fac --- /dev/null +++ b/crates/graph/src/test_command.rs @@ -0,0 +1,19 @@ +use crate::nodes::Concrete; + +#[derive(Debug, Clone)] +pub enum TestCommand { + Variable(String, VariableCommand), + Constraint(String), + Coverage(CoverageCommand), +} + +#[derive(Debug, Clone)] +pub enum VariableCommand { + RangeAssert { min: Concrete, max: Concrete }, +} + +#[derive(Debug, Clone)] +pub enum CoverageCommand { + OnlyPath, + Unreachable, +} diff --git a/crates/graph/src/var_type.rs b/crates/graph/src/var_type.rs index 3951cd77..ed49f1af 100644 --- a/crates/graph/src/var_type.rs +++ b/crates/graph/src/var_type.rs @@ -705,6 +705,14 @@ impl VarType { } } + pub fn needs_length(&self, analyzer: &impl GraphBackend) -> Result { + match self { + Self::BuiltIn(node, _) => Ok(node.needs_length(analyzer)?), + Self::Concrete(node) => Ok(node.needs_length(analyzer)?), + _ => Ok(false), + } + } + pub fn ty_eq(&self, other: &Self, analyzer: &impl GraphBackend) -> Result { match (self, other) { (VarType::User(s, _), VarType::User(o, _)) => { diff --git a/crates/pyrometer/tests/test_data/bitwise.sol b/crates/pyrometer/tests/test_data/bitwise.sol index 2d198c18..42207a1d 100644 --- a/crates/pyrometer/tests/test_data/bitwise.sol +++ b/crates/pyrometer/tests/test_data/bitwise.sol @@ -4,11 +4,13 @@ contract BitAnd { } function bit_and_conc(bytes32 d) public { - require(bytes32(type(uint256).max) & bytes32(uint(100)) == 100); - require(bytes32(0) & bytes32(uint(100)) == 0); - require(bytes32(101) & bytes32(105) == 97); - require(bytes32(type(uint24).max) & bytes32(1225) == 1225); - require(bit_and(bytes32(50), bytes32(500)) == 48); + require(uint(bytes32(type(uint256).max) & bytes32(uint(100))) == 100); + require(uint(bytes32(0) & bytes32(uint(100))) == 0); + require(uint(bytes32(uint(101)) & bytes32(uint(105))) == 97); + require( + uint(bytes32(uint(type(uint24).max)) & bytes32(uint(1225))) == 1225 + ); + require(uint(bit_and(bytes32(uint(50)), bytes32(uint(500)))) == 48); } function bit_and(uint256 x, uint256 y) public returns (uint256) { @@ -16,7 +18,7 @@ contract BitAnd { } function bit_and_conc(uint256 d) public { - require(type(uint256).max & int(100) == 100); + require(type(uint256).max & 100 == 100); require(0 & uint(100) == 0); require(101 & 105 == 97); require(type(uint24).max & 1225 == 1225); @@ -34,10 +36,6 @@ contract BitAnd { require(type(int24).max & -5 == 8388603); require(int_bit_and(50, 500) == 48); } - - function int_bit_and(int256 x, int256 y) public returns (int256) { - return x & y; - } } contract BitOr { @@ -46,11 +44,17 @@ contract BitOr { } function bit_or_conc(bytes32 d) public { - require(bytes32(type(uint256).max) | bytes32(uint(100)) == bytes32(type(uint256).max)); - require(bytes32(0) | bytes32(uint(100)) == bytes32(100)); - require(bytes32(101) | bytes32(105) == bytes32(109)); - require(bytes32(type(uint24).max) | bytes32(5) == bytes32(type(uint24).max) ); - require(bit_or(bytes32(50), bytes32(500)) == 502); + require( + bytes32(type(uint256).max) | bytes32(uint(100)) == + bytes32(type(uint256).max) + ); + require(bytes32(0) | bytes32(uint(100)) == bytes32(uint(100))); + require(bytes32(uint(101)) | bytes32(uint(105)) == bytes32(uint(109))); + require( + bytes32(uint(type(uint24).max)) | bytes32(uint(5)) == + bytes32(uint(type(uint24).max)) + ); + require(uint(bit_or(bytes32(uint(50)), bytes32(uint(500)))) == 502); } function bit_or(uint256 x, uint256 y) public returns (uint256) { @@ -84,7 +88,10 @@ contract BitXor { } function bit_xor_conc(uint256 d) public { - require(type(uint256).max ^ uint(100) == 115792089237316195423570985008687907853269984665640564039457584007913129639835); + require( + type(uint256).max ^ uint(100) == + 115792089237316195423570985008687907853269984665640564039457584007913129639835 + ); require(0 ^ uint(100) == 100); require(101 ^ 105 == 12); require(type(uint24).max ^ 5 == 16777210); @@ -96,7 +103,10 @@ contract BitXor { } function int_bit_xor_conc(uint256 d) public { - require(type(int256).max ^ int(100) == 57896044618658097711785492504343953926634992332820282019728792003956564819867); + require( + type(int256).max ^ int(100) == + 57896044618658097711785492504343953926634992332820282019728792003956564819867 + ); require(0 ^ int(100) == 100); require(101 ^ 105 == 12); require(type(int24).max ^ -5 == -8388604); @@ -112,16 +122,34 @@ contract BitNot { assembly { x := not(100) } - require(x == 115792089237316195423570985008687907853269984665640564039457584007913129639835); + require( + x == + 115792089237316195423570985008687907853269984665640564039457584007913129639835 + ); } function bit_not(bytes32 d) public view returns (bytes32) { bytes32 x = hex"1111"; - require(~x == bytes32(0xeeeeffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff)); + require( + ~x == + bytes32( + 0xeeeeffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff + ) + ); bytes16 x16 = hex"1111"; - require(~x16 == bytes32(0xeeeeffffffffffffffffffffffffffff00000000000000000000000000000000)); + require( + ~x16 == + bytes32( + 0xeeeeffffffffffffffffffffffffffff00000000000000000000000000000000 + ) + ); bytes8 x8 = hex"1111"; - require(~x8 == bytes32(0xeeeeffffffffffff000000000000000000000000000000000000000000000000)); + require( + ~x8 == + bytes32( + 0xeeeeffffffffffff000000000000000000000000000000000000000000000000 + ) + ); return ~x; } @@ -131,11 +159,23 @@ contract BitNot { function bit_not_conc(uint256 d) public { require(~type(uint256).max == 0); - require(~100 == 115792089237316195423570985008687907853269984665640564039457584007913129639835); - require(~101 == 115792089237316195423570985008687907853269984665640564039457584007913129639834); - require(~105 == 115792089237316195423570985008687907853269984665640564039457584007913129639830); + require( + ~uint(100) == + 115792089237316195423570985008687907853269984665640564039457584007913129639835 + ); + require( + ~uint(101) == + 115792089237316195423570985008687907853269984665640564039457584007913129639834 + ); + require( + ~uint(105) == + 115792089237316195423570985008687907853269984665640564039457584007913129639830 + ); require(~type(uint24).max == 0); - require(bit_not(50) == 115792089237316195423570985008687907853269984665640564039457584007913129639885); + require( + bit_not(50) == + 115792089237316195423570985008687907853269984665640564039457584007913129639885 + ); } function int_bit_not(int256 x) public returns (int256) { @@ -202,14 +242,20 @@ contract BitShl { uint256 a8 = shl(100000000000000000000, 64); require(a8 == 1844674407370955161600000000000000000000); uint256 a9 = shl(100000000000000000000000000000000000, 128); - require(a9 == 34028236692093846346337460743176821145600000000000000000000000000000000000); + require( + a9 == + 34028236692093846346337460743176821145600000000000000000000000000000000000 + ); uint256 a10 = shl(1, 255); - require(a10 == 57896044618658097711785492504343953926634992332820282019728792003956564819968); + require( + a10 == + 57896044618658097711785492504343953926634992332820282019728792003956564819968 + ); } function int_shl_conc() public returns (int256) { int256 a1 = int_shl(100, 1); - require(a1 == 200); + "pyro:variable:a1:range:[200,200]"; int256 a2 = int_shl(100, 2); require(a2 == 400); int256 a3 = int_shl(100, 4); @@ -225,9 +271,15 @@ contract BitShl { int256 a8 = int_shl(100000000000000000000, 64); require(a8 == 1844674407370955161600000000000000000000); int256 a9 = int_shl(100000000000000000000000000000000000, 128); - require(a9 == 34028236692093846346337460743176821145600000000000000000000000000000000000); + require( + a9 == + 34028236692093846346337460743176821145600000000000000000000000000000000000 + ); int256 a10 = int_shl(1, 255); - require(a10 == -57896044618658097711785492504343953926634992332820282019728792003956564819968); + require( + a10 == + -57896044618658097711785492504343953926634992332820282019728792003956564819968 + ); int256 a11 = int_shl(-100, 1); require(a11 == -200); @@ -246,9 +298,15 @@ contract BitShl { int256 a18 = int_shl(-100000000000000000000, 64); require(a18 == -1844674407370955161600000000000000000000); int256 a19 = int_shl(-100000000000000000000000000000000000, 128); - require(a19 == -34028236692093846346337460743176821145600000000000000000000000000000000000); + require( + a19 == + -34028236692093846346337460743176821145600000000000000000000000000000000000 + ); int256 a20 = int_shl(-1, 255); - require(a20 == -57896044618658097711785492504343953926634992332820282019728792003956564819968); + require( + a20 == + -57896044618658097711785492504343953926634992332820282019728792003956564819968 + ); int256 a21 = int_shl(-1, 256); require(a21 == 0); } @@ -282,7 +340,10 @@ contract BitShr { require(a8 == 5); uint256 a9 = shr(1000000000000000000000000000000000000000, 128); require(a9 == 2); - uint256 a10 = shr(1000000000000000000000000000000000000000000000000000000000000000000000000000, 248); + uint256 a10 = shr( + 1000000000000000000000000000000000000000000000000000000000000000000000000000, + 248 + ); require(a10 == 2); } @@ -305,7 +366,10 @@ contract BitShr { require(a8 == 5); int256 a9 = int_shr(1000000000000000000000000000000000000000, 128); require(a9 == 2); - int256 a10 = int_shr(1000000000000000000000000000000000000000000000000000000000000000000000000000, 248); + int256 a10 = int_shr( + 1000000000000000000000000000000000000000000000000000000000000000000000000000, + 248 + ); require(a10 == 2); int256 a11 = int_shr(-100, 1); @@ -326,7 +390,10 @@ contract BitShr { require(a18 == -5); int256 a19 = int_shr(-1000000000000000000000000000000000000000, 128); require(a19 == -2); - int256 a20 = int_shr(-1000000000000000000000000000000000000000000000000000000000000000000000000000, 248); + int256 a20 = int_shr( + -1000000000000000000000000000000000000000000000000000000000000000000000000000, + 248 + ); require(a20 == -2); } } diff --git a/crates/pyrometer/tests/test_data/dyn_types.sol b/crates/pyrometer/tests/test_data/dyn_types.sol index 5931af92..81d4e01e 100644 --- a/crates/pyrometer/tests/test_data/dyn_types.sol +++ b/crates/pyrometer/tests/test_data/dyn_types.sol @@ -6,7 +6,7 @@ contract DynTypes { uint256 b; } - mapping (address => Strukt) public someMapping; + mapping(address => Strukt) public someMapping; function bytes_dyn(bytes calldata x) public { bytes memory y = x; @@ -15,7 +15,7 @@ contract DynTypes { require(y.length == 9); } - function array_dyn(uint256[] calldata x) public { + function array_dyn(uint256[] memory x) public { x[0] = 5; require(x.length < 10); uint256[] memory y = x; @@ -23,7 +23,10 @@ contract DynTypes { require(y.length == 9); } - function nested_bytes_dyn(bytes[] calldata x, uint y) public returns (bytes1) { + function nested_bytes_dyn( + bytes[] memory x, + uint y + ) public returns (bytes1) { bytes memory a = hex"1337"; x[0] = a; require(x[0][0] == hex"13"); @@ -71,11 +74,10 @@ contract DynTypes { h[0] = holder; inLoop(h, tokens); } - + function inLoop(address[] memory holders, address[] memory tokens) public { for (uint j = 0; j < holders.length; j++) { address holder = holders[j]; } } - } diff --git a/crates/pyrometer/tests/test_data/repros/issue69.sol b/crates/pyrometer/tests/test_data/repros/issue69.sol index c653635c..cdf47c95 100644 --- a/crates/pyrometer/tests/test_data/repros/issue69.sol +++ b/crates/pyrometer/tests/test_data/repros/issue69.sol @@ -5,15 +5,23 @@ contract Test { uint256 z = x - 1; y = y - 10 + z; if (y == 69122131241245311234) { + "pyro::constraint::(y == 69122131241245311234)"; + "pyro::variable::y::range::[69122131241245311234,69122131241245311234]"; if (z == 6912213124124531) { + "pyro::constraint::(z == 6912213124124531)"; + "pyro::variable::z::range::[6912213124124531,6912213124124531]"; number = 0; + "pyro::variable::number::range::[0,0]"; } else { + "pyro::constraint::(z != 6912213124124531)"; number = 1; + "pyro::variable::number::range::[1,1]"; } } else { + "pyro::constraint::(y != 69122131241245311234)"; number = 1; } } assert(number != 0); } -} \ No newline at end of file +} diff --git a/crates/shared/src/error.rs b/crates/shared/src/error.rs index 80b1dd5b..84874b36 100644 --- a/crates/shared/src/error.rs +++ b/crates/shared/src/error.rs @@ -146,6 +146,7 @@ pub enum ExprErr { TakeFromFork(Loc, String), GraphError(Loc, GraphError), ReprError(Loc, RepresentationErr), + TestError(Loc, String), Unresolved(Loc, String), } @@ -188,6 +189,7 @@ impl ExprErr { GraphError(loc, ..) => *loc, Unresolved(loc, ..) => *loc, ReprError(loc, ..) => *loc, + TestError(loc, ..) => *loc, } } @@ -215,6 +217,7 @@ impl ExprErr { ExprErr::InvalidFunctionInput(_, msg, ..) => msg, ExprErr::TakeFromFork(_, msg, ..) => msg, ExprErr::Unresolved(_, msg, ..) => msg, + ExprErr::TestError(_, msg) => msg, ExprErr::ReprError(_, RepresentationErr::Context(c)) => c.msg(), ExprErr::ReprError(_, RepresentationErr::Variable(v)) => v.msg(), ExprErr::GraphError(_, GraphError::NodeConfusion(msg), ..) => msg, @@ -255,6 +258,7 @@ impl ExprErr { ExprErr::InvalidFunctionInput(..) => "Arguments to this function call do not match required types", ExprErr::TakeFromFork(..) => "IR Error: Tried to take from an child context that ended up forking", ExprErr::ReprError(..) => "Representation Error: This is a bug.", + ExprErr::TestError(..) => "Test error.", ExprErr::GraphError(_, GraphError::NodeConfusion(_), ..) => "Graph IR Error: Node type confusion. This is potentially a bug. Please report it at https://github.com/nascentxyz/pyrometer", ExprErr::GraphError(_, GraphError::MaxStackDepthReached(_), ..) => "Max call depth reached - either recursion or loop", ExprErr::GraphError(_, GraphError::MaxStackWidthReached(_), ..) => "TODO: Max fork width reached - Need to widen variables and remove contexts", diff --git a/crates/shared/src/search.rs b/crates/shared/src/search.rs index ea5ebb6c..d8adc066 100644 --- a/crates/shared/src/search.rs +++ b/crates/shared/src/search.rs @@ -374,7 +374,6 @@ where max_depth: usize, exclude_edges: &[::Edge], ) -> BTreeSet { - tracing::trace!("children"); let mut seen = Default::default(); self.children_exclude_prevent_cycle(start, 0, max_depth, exclude_edges, &mut seen) } @@ -424,7 +423,6 @@ where /// Gets all children recursively fn children(&self, start: NodeIdx) -> BTreeSet { - tracing::trace!("children"); let mut seen = Default::default(); self.children_prevent_cycle(start, &mut seen) } @@ -466,8 +464,6 @@ where where ::Edge: Ord, { - tracing::trace!("children_edges"); - nodes .iter() .flat_map(|node| { @@ -497,7 +493,6 @@ where where ::Edge: Ord, { - tracing::trace!("children_edges"); let mut seen = Default::default(); self.children_edges_prevent_cycle(start, &mut seen) } diff --git a/crates/solc-expressions/src/context_builder/expr.rs b/crates/solc-expressions/src/context_builder/expr.rs index 476e980f..33708f16 100644 --- a/crates/solc-expressions/src/context_builder/expr.rs +++ b/crates/solc-expressions/src/context_builder/expr.rs @@ -1,6 +1,7 @@ -use crate::func_call::intrinsic_call::IntrinsicFuncCaller; use crate::{ - context_builder::ContextBuilder, func_call::func_caller::FuncCaller, variable::Variable, + context_builder::ContextBuilder, + func_call::{func_caller::FuncCaller, intrinsic_call::IntrinsicFuncCaller}, + variable::Variable, ExprTyParser, }; @@ -9,7 +10,7 @@ use graph::{ nodes::{Builtin, Concrete, ContextNode, ContextVar, ContextVarNode, ExprRet}, AnalyzerBackend, ContextEdge, Edge, Node, }; -use shared::{ExprErr, IntoExprErr, RangeArena}; +use shared::{post_to_site, ExprErr, IntoExprErr, RangeArena, USE_DEBUG_SITE}; use ethers_core::types::I256; use solang_parser::{ @@ -33,7 +34,7 @@ pub trait ExpressionParser: expr: &Expression, ctx: ContextNode, ) -> Result<(), ExprErr> { - if !ctx.killed_or_ret(self).unwrap() { + let res = if !ctx.killed_or_ret(self).unwrap() { let edges = ctx.live_edges(self).into_expr_err(expr.loc())?; if edges.is_empty() { self.parse_ctx_expr_inner(arena, expr, ctx) @@ -45,7 +46,11 @@ pub trait ExpressionParser: } } else { Ok(()) + }; + if unsafe { USE_DEBUG_SITE } { + post_to_site(&*self, arena); } + res } #[tracing::instrument(level = "trace", skip_all, fields(ctx = %ctx.path(self).replace('.', "\n\t.")))] diff --git a/crates/solc-expressions/src/context_builder/mod.rs b/crates/solc-expressions/src/context_builder/mod.rs index e57f28fa..8c3c78c0 100644 --- a/crates/solc-expressions/src/context_builder/mod.rs +++ b/crates/solc-expressions/src/context_builder/mod.rs @@ -16,10 +16,12 @@ impl ContextBuilder for T where mod expr; mod fn_calls; mod stmt; +mod test_command_runner; pub use expr::*; pub use fn_calls::*; pub use stmt::*; +pub use test_command_runner::*; /// Dispatcher for building up a context of a function pub trait ContextBuilder: diff --git a/crates/solc-expressions/src/context_builder/stmt.rs b/crates/solc-expressions/src/context_builder/stmt.rs index 53c108f3..ac5d01ad 100644 --- a/crates/solc-expressions/src/context_builder/stmt.rs +++ b/crates/solc-expressions/src/context_builder/stmt.rs @@ -3,7 +3,7 @@ use crate::{ func_call::{func_caller::FuncCaller, modifier::ModifierCaller}, loops::Looper, yul::YulBuilder, - ExpressionParser, + ExpressionParser, TestCommandRunner, }; use graph::{ @@ -31,7 +31,7 @@ impl StatementParser for T where /// Solidity statement parser pub trait StatementParser: - AnalyzerBackend + Sized + ExpressionParser + AnalyzerBackend + Sized + ExpressionParser + TestCommandRunner { /// Performs setup for parsing a solidity statement fn parse_ctx_statement( @@ -72,10 +72,6 @@ pub trait StatementParser: self.parse_ctx_stmt_inner(arena, stmt, unchecked, parent_ctx) } - if unsafe { USE_DEBUG_SITE } { - post_to_site(&*self, arena); - } - if let Some(errs) = self.add_if_err(self.is_representation_ok(arena).into_expr_err(stmt.loc())) { @@ -97,7 +93,15 @@ pub trait StatementParser: Self: Sized, { use Statement::*; - // tracing::trace!("stmt: {:#?}, node: {:#?}", stmt, if let Some(node) = parent_ctx { Some(self.node(node.into())) } else { None}); + // tracing::trace!( + // "stmt: {:#?}, node: {:#?}", + // stmt, + // if let Some(node) = parent_ctx { + // Some(self.node(node.into())) + // } else { + // None + // } + // ); // at the end of a statement we shouldn't have anything in the stack? if let Some(ctx) = parent_ctx { @@ -423,6 +427,22 @@ pub trait StatementParser: tracing::trace!("parsing expr, {expr:?}"); if let Some(parent) = parent_ctx { let ctx = parent.into().into(); + if let solang_parser::pt::Expression::StringLiteral(lits) = expr { + if lits.len() == 1 { + if let Some(command) = self.test_string_literal(&lits[0].string) { + let _ = self.apply_to_edges( + ctx, + *loc, + arena, + &|analyzer, arena, ctx, loc| { + analyzer.run_test_command(arena, ctx, loc, command.clone()); + Ok(()) + }, + ); + } + } + } + match self.parse_ctx_expr(arena, expr, ctx) { Ok(()) => { let res = self.apply_to_edges( diff --git a/crates/solc-expressions/src/context_builder/test_command_runner.rs b/crates/solc-expressions/src/context_builder/test_command_runner.rs new file mode 100644 index 00000000..18eccaa1 --- /dev/null +++ b/crates/solc-expressions/src/context_builder/test_command_runner.rs @@ -0,0 +1,113 @@ +use crate::{ + context_builder::ContextBuilder, + func_call::{func_caller::FuncCaller, modifier::ModifierCaller}, + loops::Looper, + yul::YulBuilder, + ExpressionParser, +}; + +use graph::{ + elem::{Elem, RangeElem}, + nodes::{ + Concrete, Context, ContextNode, ContextVar, ContextVarNode, ExprRet, FunctionNode, + FunctionParamNode, FunctionReturnNode, KilledKind, + }, + AnalyzerBackend, CallFork, ContextEdge, CoverageCommand, Edge, Node, TestCommand, + VariableCommand, +}; +use shared::{ + post_to_site, AnalyzerLike, ExprErr, GraphDot, IntoExprErr, NodeIdx, RangeArena, USE_DEBUG_SITE, +}; + +use petgraph::{visit::EdgeRef, Direction}; +use solang_parser::{ + helpers::CodeLocation, + pt::{Expression, Loc, Statement, YulStatement}, +}; + +impl TestCommandRunner for T where + T: AnalyzerBackend + Sized + ExpressionParser +{ +} + +/// Solidity statement parser +pub trait TestCommandRunner: + AnalyzerBackend + Sized + ExpressionParser +{ + fn run_test_command( + &mut self, + arena: &mut RangeArena>, + ctx: ContextNode, + loc: Loc, + test_command: TestCommand, + ) -> Option<()> { + match test_command { + TestCommand::Variable(var_name, VariableCommand::RangeAssert { min, max }) => { + if let Some(var) = ctx.var_by_name(self, &var_name) { + let min = Elem::from(min); + let max = Elem::from(max); + let latest = var.latest_version(self); + let eval_min = + self.add_if_err(latest.evaled_range_min(self, arena).into_expr_err(loc))??; + let eval_max = + self.add_if_err(latest.evaled_range_max(self, arena).into_expr_err(loc))??; + if !eval_min.range_eq(&min, arena) { + self.add_expr_err(ExprErr::TestError( + loc, + format!( + "Variable \"{var_name}\"'s minimum was {}, expected {}", + eval_min, + min + ), + )); + } + if !eval_max.range_eq(&max, arena) { + self.add_expr_err(ExprErr::TestError( + loc, + format!( + "Variable \"{var_name}\"'s maximum was {}, expected {}", + eval_max, + max + ), + )); + } + } else { + self.add_expr_err(ExprErr::TestError( + loc, + format!("No variable \"{var_name}\" found in context"), + )); + } + } + TestCommand::Constraint(c) => { + let deps = ctx.ctx_deps(self).ok()?; + if !deps.iter().any(|dep| dep.display_name(self).unwrap() == c) { + self.add_expr_err(ExprErr::TestError( + loc, + format!( + "No dependency \"{c}\" found for context, constraints: {:#?}", + deps.iter() + .map(|i| i.display_name(self).unwrap()) + .collect::>() + ), + )); + } + } + TestCommand::Coverage(CoverageCommand::OnlyPath) => { + if let Some(parent) = ctx.underlying(self).unwrap().parent_ctx { + if !parent.underlying(self).unwrap().child.is_none() { + self.add_expr_err(ExprErr::TestError( + loc, + format!("Expected a single path, but another was reached"), + )); + } + } + } + TestCommand::Coverage(CoverageCommand::Unreachable) => { + self.add_expr_err(ExprErr::TestError(loc, format!("Hit an unreachable path"))); + } + _ => {} + } + + None + } +} diff --git a/crates/solc-expressions/src/func_call/intrinsic_call/types.rs b/crates/solc-expressions/src/func_call/intrinsic_call/types.rs index b1a9357e..823d8809 100644 --- a/crates/solc-expressions/src/func_call/intrinsic_call/types.rs +++ b/crates/solc-expressions/src/func_call/intrinsic_call/types.rs @@ -189,7 +189,7 @@ pub trait TypesCaller: AnalyzerBackend + S .into_expr_err(loc)?; } - if cvar.is_indexable(analyzer).into_expr_err(loc)? { + if cvar.needs_length(analyzer).into_expr_err(loc)? { // input is indexable. get the length attribute, create a new length for the casted type let _ = analyzer.create_length( arena, diff --git a/crates/solc-expressions/src/literal.rs b/crates/solc-expressions/src/literal.rs index dac3bcf6..62c226ae 100644 --- a/crates/solc-expressions/src/literal.rs +++ b/crates/solc-expressions/src/literal.rs @@ -1,7 +1,7 @@ use graph::{ elem::*, nodes::{Builtin, Concrete, ConcreteNode, ContextNode, ContextVar, ContextVarNode, ExprRet}, - AnalyzerBackend, ContextEdge, Edge, Node, + AnalyzerBackend, ContextEdge, Edge, Node, TestCommand, VariableCommand, }; use shared::{ExprErr, IntoExprErr, RangeArena}; @@ -14,15 +14,13 @@ impl Literal for T where T: AnalyzerBackend + Sized {} /// Dealing with literal expression and parsing them into nodes pub trait Literal: AnalyzerBackend + Sized { - fn number_literal( + fn concrete_number_from_str( &mut self, - ctx: ContextNode, - loc: Loc, integer: &str, exponent: &str, negative: bool, unit: &Option, - ) -> Result<(), ExprErr> { + ) -> Concrete { let int = U256::from_dec_str(integer).unwrap(); let val = if !exponent.is_empty() { let exp = U256::from_dec_str(exponent).unwrap(); @@ -38,18 +36,29 @@ pub trait Literal: AnalyzerBackend + Sized { }; let size: u16 = ((32 - (val.leading_zeros() / 8)) * 8).max(8) as u16; - let concrete_node = if negative { + if negative { let val = if val == U256::from(2).pow(255.into()) { // no need to set upper bit I256::from_raw(val) } else { I256::from(-1i32) * I256::from_raw(val) }; - ConcreteNode::from(self.add_node(Node::Concrete(Concrete::Int(size, val)))) + Concrete::Int(size, val) } else { - ConcreteNode::from(self.add_node(Node::Concrete(Concrete::Uint(size, val)))) - }; - + Concrete::Uint(size, val) + } + } + fn number_literal( + &mut self, + ctx: ContextNode, + loc: Loc, + integer: &str, + exponent: &str, + negative: bool, + unit: &Option, + ) -> Result<(), ExprErr> { + let conc = self.concrete_number_from_str(integer, exponent, negative, unit); + let concrete_node = ConcreteNode::from(self.add_node(Node::Concrete(conc))); let ccvar = Node::ContextVar( ContextVar::new_from_concrete(loc, ctx, concrete_node, self).into_expr_err(loc)?, ); @@ -215,6 +224,66 @@ pub trait Literal: AnalyzerBackend + Sized { Ok(()) } + fn test_string_literal(&mut self, s: &str) -> Option { + let split = s.split("::").collect::>(); + if split.get(0).copied() == Some("pyro") { + match split.get(1).copied() { + Some("variable") => { + let name = split.get(2).copied()?; + match split.get(3).copied() { + Some("range") => { + let range = split + .get(4) + .copied()? + .trim_start_matches('[') + .trim_end_matches(']') + .split(',') + .collect::>(); + let mut min_str = *range.get(0)?; + let mut min_neg = false; + if let Some(new_min) = min_str.strip_prefix('-') { + min_neg = true; + min_str = new_min; + } + + let mut max_str = *range.get(1)?; + let mut max_neg = false; + if let Some(new_max) = max_str.strip_prefix('-') { + max_neg = true; + max_str = new_max; + } + + let min = self.concrete_number_from_str(min_str, "", min_neg, &None); + let max = self.concrete_number_from_str(max_str, "", max_neg, &None); + + Some(TestCommand::Variable( + name.to_string(), + VariableCommand::RangeAssert { min, max }, + )) + } + _ => None, + } + } + Some("constraint") => { + let constraint = split.get(2).copied()?; + Some(TestCommand::Constraint(constraint.to_string())) + } + Some("coverage") => match split.get(2).copied() { + Some("onlyPath") => { + Some(TestCommand::Coverage(graph::CoverageCommand::OnlyPath)) + } + Some("unreachable") => { + Some(TestCommand::Coverage(graph::CoverageCommand::Unreachable)) + } + _ => None, + }, + _ => None, + } + } else { + None + } + } + fn string_literal(&mut self, ctx: ContextNode, loc: Loc, s: &str) -> Result<(), ExprErr> { let concrete_node = ConcreteNode::from(self.add_node(Node::Concrete(Concrete::String(s.to_string())))); From 2951a514942d730cbdb3e44ef7d234497a30a169 Mon Sep 17 00:00:00 2001 From: brock elmore Date: Wed, 10 Jul 2024 15:53:36 -0700 Subject: [PATCH 05/10] tiny fixes --- crates/graph/src/var_type.rs | 1 - crates/solc-expressions/src/literal.rs | 29 ++++++++++++++++++++------ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/crates/graph/src/var_type.rs b/crates/graph/src/var_type.rs index 6cbe0607..ed49f1af 100644 --- a/crates/graph/src/var_type.rs +++ b/crates/graph/src/var_type.rs @@ -407,7 +407,6 @@ impl VarType { } pub fn range(&self, analyzer: &impl GraphBackend) -> Result, GraphError> { - println!("here: {:?}", self); match self { Self::User(_, Some(range)) => Ok(Some(range.clone())), Self::BuiltIn(_, Some(range)) => Ok(Some(range.clone())), diff --git a/crates/solc-expressions/src/literal.rs b/crates/solc-expressions/src/literal.rs index 09233d91..ca67d629 100644 --- a/crates/solc-expressions/src/literal.rs +++ b/crates/solc-expressions/src/literal.rs @@ -16,11 +16,12 @@ impl Literal for T where T: AnalyzerBackend + Sized {} pub trait Literal: AnalyzerBackend + Sized { fn concrete_number_from_str( &mut self, + loc: Loc, integer: &str, exponent: &str, negative: bool, unit: &Option, - ) -> Concrete { + ) -> Result { let int = U256::from_dec_str(integer).unwrap(); let val = if !exponent.is_empty() { let exp = U256::from_dec_str(exponent) @@ -51,9 +52,9 @@ pub trait Literal: AnalyzerBackend + Sized { } I256::from(-1i32) * raw }; - Concrete::Int(size, val) + Ok(Concrete::Int(size, val)) } else { - Concrete::Uint(size, val) + Ok(Concrete::Uint(size, val)) } } fn number_literal( @@ -65,7 +66,7 @@ pub trait Literal: AnalyzerBackend + Sized { negative: bool, unit: &Option, ) -> Result<(), ExprErr> { - let conc = self.concrete_number_from_str(integer, exponent, negative, unit); + let conc = self.concrete_number_from_str(loc, integer, exponent, negative, unit)?; let concrete_node = ConcreteNode::from(self.add_node(Node::Concrete(conc))); let ccvar = Node::ContextVar( ContextVar::new_from_concrete(loc, ctx, concrete_node, self).into_expr_err(loc)?, @@ -304,8 +305,24 @@ pub trait Literal: AnalyzerBackend + Sized { max_str = new_max; } - let min = self.concrete_number_from_str(min_str, "", min_neg, &None); - let max = self.concrete_number_from_str(max_str, "", max_neg, &None); + let min = self + .concrete_number_from_str( + Loc::Implicit, + min_str, + "", + min_neg, + &None, + ) + .ok()?; + let max = self + .concrete_number_from_str( + Loc::Implicit, + max_str, + "", + max_neg, + &None, + ) + .ok()?; Some(TestCommand::Variable( name.to_string(), From 50f46ef4e20466f0ab4d582c605efbe08c76367b Mon Sep 17 00:00:00 2001 From: brock elmore Date: Thu, 11 Jul 2024 09:11:15 -0700 Subject: [PATCH 06/10] fix struct var def --- crates/graph/src/nodes/context/invariants.rs | 3 +- .../graph/src/nodes/context/var/invariants.rs | 37 ++-- crates/graph/src/nodes/struct_ty.rs | 29 ++- crates/graph/src/var_type.rs | 8 + crates/pyrometer/src/analyzer.rs | 4 +- crates/pyrometer/src/analyzer_backend.rs | 27 ++- crates/pyrometer/src/graph_backend.rs | 3 +- crates/solc-expressions/src/array.rs | 9 + .../src/context_builder/expr.rs | 21 +- .../src/context_builder/stmt.rs | 30 +-- .../context_builder/test_command_runner.rs | 42 ++-- crates/solc-expressions/src/literal.rs | 13 +- crates/solc-expressions/src/variable.rs | 6 + mini.sol | 180 ++---------------- 14 files changed, 167 insertions(+), 245 deletions(-) diff --git a/crates/graph/src/nodes/context/invariants.rs b/crates/graph/src/nodes/context/invariants.rs index d7d82903..f4522856 100644 --- a/crates/graph/src/nodes/context/invariants.rs +++ b/crates/graph/src/nodes/context/invariants.rs @@ -5,11 +5,12 @@ use crate::{ range::elem::Elem, ContextEdge, Edge, GraphBackend, RepresentationInvariant, }; -use shared::{ContextReprErr, GraphError, NodeIdx, RangeArena, RepresentationErr}; +use shared::{ContextReprErr, GraphError, RangeArena, RepresentationErr}; use petgraph::{visit::EdgeRef, Direction}; impl ContextNode { + #[allow(dead_code)] fn cache_matches_edges( &self, g: &impl GraphBackend, diff --git a/crates/graph/src/nodes/context/var/invariants.rs b/crates/graph/src/nodes/context/var/invariants.rs index b16b9f23..96761e86 100644 --- a/crates/graph/src/nodes/context/var/invariants.rs +++ b/crates/graph/src/nodes/context/var/invariants.rs @@ -1,9 +1,9 @@ use crate::{ - nodes::{Builtin, Concrete, ContextNode, ContextVarNode, StructNode}, + nodes::{Builtin, Concrete, ContextVarNode, StructNode}, range::elem::Elem, ContextEdge, Edge, GraphBackend, Node, RepresentationInvariant, TypeNode, VarType, }; -use shared::{GraphError, NodeIdx, RangeArena, RepresentationErr, VarReprErr}; +use shared::{GraphError, RangeArena, RepresentationErr, VarReprErr}; use petgraph::{visit::EdgeRef, Direction}; @@ -11,19 +11,20 @@ impl ContextVarNode { fn ty_check( &self, g: &impl GraphBackend, - arena: &RangeArena>, + _arena: &RangeArena>, ) -> Result, GraphError> { match self.ty(g)? { // we should have all types resolved by now - VarType::User(TypeNode::Unresolved(u), range) => { + VarType::User(TypeNode::Unresolved(_u), _range) => { Ok(Some(VarReprErr::Unresolved(self.0.into()).into())) } - VarType::User(TypeNode::Contract(con), range) => Ok(None), - VarType::User(TypeNode::Struct(strt), range) => self.struct_invariants(*strt, g), - VarType::User(TypeNode::Enum(enu), range) => Ok(None), - VarType::User(TypeNode::Ty(ty), range) => Ok(None), - VarType::User(TypeNode::Func(func), range) => Ok(None), - VarType::BuiltIn(bn, range) => { + VarType::User(TypeNode::Contract(_con), _range) => Ok(None), + VarType::User(TypeNode::Struct(strt), _range) => self.struct_invariants(*strt, g), + VarType::User(TypeNode::Enum(_enu), _range) => Ok(None), + VarType::User(TypeNode::Ty(_ty), _range) => Ok(None), + VarType::User(TypeNode::Func(_func), _range) => Ok(None), + VarType::User(TypeNode::Error(_err), _) => Ok(None), + VarType::BuiltIn(bn, _range) => { let Node::Builtin(b) = g.node(*bn) else { panic!("here") }; @@ -31,18 +32,18 @@ impl ContextVarNode { Builtin::Address | Builtin::Payable | Builtin::AddressPayable => Ok(None), Builtin::Bool => Ok(None), Builtin::String => Ok(None), - Builtin::Bytes(size) => Ok(None), + Builtin::Bytes(_size) => Ok(None), Builtin::DynamicBytes => Ok(None), - Builtin::Int(size) => Ok(None), - Builtin::Uint(size) => Ok(None), - Builtin::Array(inner_ty) => Ok(None), - Builtin::SizedArray(size, inner_ty) => Ok(None), - Builtin::Mapping(key_ty, v_ty) => Ok(None), + Builtin::Int(_size) => Ok(None), + Builtin::Uint(_size) => Ok(None), + Builtin::Array(_inner_ty) => Ok(None), + Builtin::SizedArray(_size, _inner_ty) => Ok(None), + Builtin::Mapping(_key_ty, _v_ty) => Ok(None), Builtin::Rational => Ok(None), - Builtin::Func(inputs, rets) => Ok(None), + Builtin::Func(_inputs, _rets) => Ok(None), } } - VarType::Concrete(cn) => Ok(None), + VarType::Concrete(_cn) => Ok(None), } } diff --git a/crates/graph/src/nodes/struct_ty.rs b/crates/graph/src/nodes/struct_ty.rs index 1fbd0ca4..5ce87170 100644 --- a/crates/graph/src/nodes/struct_ty.rs +++ b/crates/graph/src/nodes/struct_ty.rs @@ -1,7 +1,7 @@ use crate::{ - nodes::{Concrete, ContractNode}, + nodes::{Concrete, ContextVar, ContextVarNode, ContractNode}, range::elem::Elem, - AnalyzerBackend, AsDotStr, Edge, GraphBackend, Node, VarType, + AnalyzerBackend, AsDotStr, ContextEdge, Edge, GraphBackend, Node, VarType, }; use shared::{GraphError, NodeIdx, RangeArena}; @@ -82,6 +82,31 @@ impl StructNode { .next() .map(ContractNode::from) } + + pub fn add_fields_to_cvar( + &self, + analyzer: &mut impl GraphBackend, + loc: Loc, + cvar: ContextVarNode, + ) -> Result<(), GraphError> { + self.fields(analyzer).iter().try_for_each(|field| { + let field_cvar = ContextVar::maybe_new_from_field( + analyzer, + loc, + cvar.underlying(analyzer)?, + field.underlying(analyzer).unwrap().clone(), + ) + .expect("Invalid struct field"); + + let fc_node = analyzer.add_node(Node::ContextVar(field_cvar)); + analyzer.add_edge( + fc_node, + cvar, + Edge::Context(ContextEdge::AttrAccess("field")), + ); + Ok(()) + }) + } } impl AsDotStr for StructNode { diff --git a/crates/graph/src/var_type.rs b/crates/graph/src/var_type.rs index 4441b372..72ed4c8d 100644 --- a/crates/graph/src/var_type.rs +++ b/crates/graph/src/var_type.rs @@ -798,6 +798,14 @@ impl VarType { ))), } } + + pub fn maybe_struct(&self) -> Option { + if let VarType::User(TypeNode::Struct(sn), _) = self { + Some(*sn) + } else { + None + } + } } #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, Ord, PartialOrd)] diff --git a/crates/pyrometer/src/analyzer.rs b/crates/pyrometer/src/analyzer.rs index d2ad2192..11675376 100644 --- a/crates/pyrometer/src/analyzer.rs +++ b/crates/pyrometer/src/analyzer.rs @@ -6,13 +6,13 @@ use reqwest::Client; use serde::{Deserialize, Serialize}; use shared::{AnalyzerLike, ApplyStats, GraphLike, NodeIdx, Search}; use shared::{ExprErr, IntoExprErr, RangeArena, USE_DEBUG_SITE}; -use solc_expressions::{FnCallBuilder, StatementParser}; +use solc_expressions::StatementParser; use tokio::runtime::Runtime; use tracing::{error, trace, warn}; use ahash::AHashMap; use ariadne::{Cache, Color, Config, Fmt, Label, Report, ReportKind, Source, Span}; -use petgraph::{graph::*, stable_graph::StableGraph, Directed}; +use petgraph::{graph::*, Directed}; use serde_json::Value; use solang_parser::{ diagnostics::Diagnostic, diff --git a/crates/pyrometer/src/analyzer_backend.rs b/crates/pyrometer/src/analyzer_backend.rs index c5a2387b..2d0892c3 100644 --- a/crates/pyrometer/src/analyzer_backend.rs +++ b/crates/pyrometer/src/analyzer_backend.rs @@ -2,14 +2,15 @@ use crate::Analyzer; use graph::{ elem::Elem, nodes::{ - BlockNode, Builtin, Concrete, ConcreteNode, ContextNode, ContextVar, Function, - FunctionNode, FunctionParam, FunctionParamNode, FunctionReturn, MsgNode, + BlockNode, Builtin, Concrete, ConcreteNode, ContextNode, ContextVar, ContractNode, + FuncReconstructionReqs, Function, FunctionNode, FunctionParam, FunctionParamNode, + FunctionReturn, KilledKind, MsgNode, }, - AnalyzerBackend, Edge, Node, RepresentationInvariant, VarType, + AnalyzerBackend, Edge, GraphBackend, Node, RepresentationInvariant, TypeNode, VarType, }; use shared::{ - AnalyzerLike, ExprErr, GraphError, GraphLike, IntoExprErr, JoinStats, NodeIdx, RangeArena, - RepresentationErr, USE_DEBUG_SITE, + AnalyzerLike, ApplyStats, ExprErr, GraphError, GraphLike, IntoExprErr, NodeIdx, RangeArena, + RepresentationErr, }; use ahash::AHashMap; @@ -67,7 +68,8 @@ impl AnalyzerLike for Analyzer { fn minimize_err(&mut self, ctx: ContextNode) -> String { let genesis = ctx.genesis(self).unwrap(); - let family_tree = genesis.family_tree(self).unwrap(); + let mut family_tree = genesis.family_tree(self).unwrap(); + family_tree.push(genesis); let mut needed_functions = family_tree .iter() .map(|c| c.associated_fn(self).unwrap()) @@ -108,6 +110,8 @@ impl AnalyzerLike for Analyzer { Vec<(FunctionNode, FuncReconstructionReqs)>, > = Default::default(); + println!("contract_to_funcs: {contract_to_funcs:#?}"); + let mut tys = vec![]; let mut enums = vec![]; let mut structs = vec![]; @@ -232,7 +236,9 @@ impl AnalyzerLike for Analyzer { fn add_expr_err(&mut self, err: ExprErr) { if self.debug_panic() { + println!("here1"); if let Some(path) = self.minimize_debug().clone() { + println!("here2"); let reconstruction_edge: ContextNode = self .graph .node_indices() @@ -240,6 +246,7 @@ impl AnalyzerLike for Analyzer { Node::Context(context) if context.killed.is_some() => { match context.killed.unwrap() { (_, KilledKind::ParseError) => { + println!("here3"); // println!("found context: {}", context.path); let edges = graph::nodes::ContextNode::from(node) .all_edges(self) @@ -253,14 +260,20 @@ impl AnalyzerLike for Analyzer { Some(reconstruction_edge) } - _ => None, + e => { + println!("here4"); + println!("{e:?}"); + None + } } } _ => None, }) .unwrap(); + println!("here5"); let min_str = self.minimize_err(reconstruction_edge); + println!("here6: {min_str}"); // println!("reconstructed source:\n{} placed in {}", min_str, path); let mut file = std::fs::OpenOptions::new() diff --git a/crates/pyrometer/src/graph_backend.rs b/crates/pyrometer/src/graph_backend.rs index 63dc3848..062e8bdc 100644 --- a/crates/pyrometer/src/graph_backend.rs +++ b/crates/pyrometer/src/graph_backend.rs @@ -546,7 +546,8 @@ flowchart TB format!("LS-{}_LE-{}_{}", from.index(), to.index(), &graph[edge]) }) .collect::>() - .join(";"); + .join(";") + + " "; let invis_data = format!( " {}(\"{}\"):::INVIS\n classDef INVIS display:none", diff --git a/crates/solc-expressions/src/array.rs b/crates/solc-expressions/src/array.rs index cb458b57..017e96ec 100644 --- a/crates/solc-expressions/src/array.rs +++ b/crates/solc-expressions/src/array.rs @@ -200,6 +200,8 @@ pub trait Array: AnalyzerBackend + Sized { let ty = parent.ty(self).into_expr_err(loc)?.clone(); let ty = ty.dynamic_underlying_ty(self).into_expr_err(loc)?; + let maybe_struct = ty.maybe_struct(); + let has_range = ty.ref_range(self).into_expr_err(loc)?.is_some(); let index_access_var = ContextVar { loc: Some(loc), @@ -232,6 +234,13 @@ pub trait Array: AnalyzerBackend + Sized { parent, Edge::Context(ContextEdge::IndexAccess), ); + + if let Some(strukt) = maybe_struct { + strukt + .add_fields_to_cvar(self, loc, ContextVarNode::from(idx_access_node)) + .into_expr_err(loc)?; + } + self.add_edge(idx_access_node, ctx, Edge::Context(ContextEdge::Variable)); ctx.add_var(idx_access_node.into(), self) .into_expr_err(loc)?; diff --git a/crates/solc-expressions/src/context_builder/expr.rs b/crates/solc-expressions/src/context_builder/expr.rs index 977a0728..5f1d7c89 100644 --- a/crates/solc-expressions/src/context_builder/expr.rs +++ b/crates/solc-expressions/src/context_builder/expr.rs @@ -7,7 +7,7 @@ use crate::{ use graph::{ elem::*, - nodes::{Builtin, Concrete, ContextNode, ContextVar, ContextVarNode, ExprRet}, + nodes::{Builtin, Concrete, ContextNode, ContextVar, ContextVarNode, ExprRet, KilledKind}, AnalyzerBackend, ContextEdge, Edge, Node, }; use shared::{post_to_site, ExprErr, IntoExprErr, RangeArena, USE_DEBUG_SITE}; @@ -50,6 +50,25 @@ pub trait ExpressionParser: if unsafe { USE_DEBUG_SITE } { post_to_site(&*self, arena); } + + if ctx + .underlying(self) + .into_expr_err(expr.loc())? + .expr_ret_stack + .is_empty() + { + if let Some(errs) = + self.add_if_err(self.is_representation_ok(arena).into_expr_err(expr.loc())) + { + if !errs.is_empty() { + ctx.kill(self, expr.loc(), KilledKind::ParseError).unwrap(); + errs.into_iter().for_each(|err| { + self.add_expr_err(ExprErr::from_repr_err(expr.loc(), err)); + }); + } + } + } + res } diff --git a/crates/solc-expressions/src/context_builder/stmt.rs b/crates/solc-expressions/src/context_builder/stmt.rs index 16fb0667..7f1a4321 100644 --- a/crates/solc-expressions/src/context_builder/stmt.rs +++ b/crates/solc-expressions/src/context_builder/stmt.rs @@ -14,9 +14,7 @@ use graph::{ }, AnalyzerBackend, ContextEdge, Edge, Node, }; -use shared::{ - post_to_site, AnalyzerLike, ExprErr, GraphDot, IntoExprErr, NodeIdx, RangeArena, USE_DEBUG_SITE, -}; +use shared::{ExprErr, IntoExprErr, NodeIdx, RangeArena}; use petgraph::{visit::EdgeRef, Direction}; use solang_parser::{ @@ -71,14 +69,6 @@ pub trait StatementParser: } else { self.parse_ctx_stmt_inner(arena, stmt, unchecked, parent_ctx) } - - if let Some(errs) = - self.add_if_err(self.is_representation_ok(arena).into_expr_err(stmt.loc())) - { - errs.into_iter().for_each(|err| { - self.add_expr_err(ExprErr::from_repr_err(stmt.loc(), err)); - }); - } } #[tracing::instrument(level = "trace", skip_all)] @@ -107,6 +97,24 @@ pub trait StatementParser: if let Some(ctx) = parent_ctx { if let Node::Context(_) = self.node(ctx) { let c = ContextNode::from(ctx.into()); + if let Some(errs) = + self.add_if_err(self.is_representation_ok(arena).into_expr_err(stmt.loc())) + { + if !errs.is_empty() { + let Some(is_killed) = + self.add_if_err(c.is_killed(self).into_expr_err(stmt.loc())) + else { + return; + }; + if !is_killed { + c.kill(self, stmt.loc(), KilledKind::ParseError).unwrap(); + errs.into_iter().for_each(|err| { + self.add_expr_err(ExprErr::from_repr_err(stmt.loc(), err)); + }); + } + } + } + let _ = c.pop_expr_latest(stmt.loc(), self); if unchecked { let _ = c.set_unchecked(self); diff --git a/crates/solc-expressions/src/context_builder/test_command_runner.rs b/crates/solc-expressions/src/context_builder/test_command_runner.rs index 18eccaa1..84c70b62 100644 --- a/crates/solc-expressions/src/context_builder/test_command_runner.rs +++ b/crates/solc-expressions/src/context_builder/test_command_runner.rs @@ -1,29 +1,13 @@ -use crate::{ - context_builder::ContextBuilder, - func_call::{func_caller::FuncCaller, modifier::ModifierCaller}, - loops::Looper, - yul::YulBuilder, - ExpressionParser, -}; +use crate::ExpressionParser; use graph::{ elem::{Elem, RangeElem}, - nodes::{ - Concrete, Context, ContextNode, ContextVar, ContextVarNode, ExprRet, FunctionNode, - FunctionParamNode, FunctionReturnNode, KilledKind, - }, - AnalyzerBackend, CallFork, ContextEdge, CoverageCommand, Edge, Node, TestCommand, - VariableCommand, -}; -use shared::{ - post_to_site, AnalyzerLike, ExprErr, GraphDot, IntoExprErr, NodeIdx, RangeArena, USE_DEBUG_SITE, + nodes::{Concrete, ContextNode}, + AnalyzerBackend, CoverageCommand, TestCommand, VariableCommand, }; +use shared::{ExprErr, IntoExprErr, RangeArena}; -use petgraph::{visit::EdgeRef, Direction}; -use solang_parser::{ - helpers::CodeLocation, - pt::{Expression, Loc, Statement, YulStatement}, -}; +use solang_parser::pt::{Expression, Loc}; impl TestCommandRunner for T where T: AnalyzerBackend + Sized + ExpressionParser @@ -56,8 +40,7 @@ pub trait TestCommandRunner: loc, format!( "Variable \"{var_name}\"'s minimum was {}, expected {}", - eval_min, - min + eval_min, min ), )); } @@ -66,8 +49,7 @@ pub trait TestCommandRunner: loc, format!( "Variable \"{var_name}\"'s maximum was {}, expected {}", - eval_max, - max + eval_max, max ), )); } @@ -94,18 +76,20 @@ pub trait TestCommandRunner: } TestCommand::Coverage(CoverageCommand::OnlyPath) => { if let Some(parent) = ctx.underlying(self).unwrap().parent_ctx { - if !parent.underlying(self).unwrap().child.is_none() { + if parent.underlying(self).unwrap().child.is_some() { self.add_expr_err(ExprErr::TestError( loc, - format!("Expected a single path, but another was reached"), + "Expected a single path, but another was reached".to_string(), )); } } } TestCommand::Coverage(CoverageCommand::Unreachable) => { - self.add_expr_err(ExprErr::TestError(loc, format!("Hit an unreachable path"))); + self.add_expr_err(ExprErr::TestError( + loc, + "Hit an unreachable path".to_string(), + )); } - _ => {} } None diff --git a/crates/solc-expressions/src/literal.rs b/crates/solc-expressions/src/literal.rs index ca67d629..6ced7c03 100644 --- a/crates/solc-expressions/src/literal.rs +++ b/crates/solc-expressions/src/literal.rs @@ -1,6 +1,6 @@ use graph::{ elem::*, - nodes::{Builtin, Concrete, ConcreteNode, ContextNode, ContextVar, ContextVarNode, ExprRet}, + nodes::{Concrete, ConcreteNode, ContextNode, ContextVar, ContextVarNode, ExprRet}, AnalyzerBackend, ContextEdge, Edge, Node, TestCommand, VariableCommand, }; use shared::{ExprErr, IntoExprErr, RangeArena}; @@ -22,7 +22,12 @@ pub trait Literal: AnalyzerBackend + Sized { negative: bool, unit: &Option, ) -> Result { - let int = U256::from_dec_str(integer).unwrap(); + let Ok(int) = U256::from_dec_str(integer) else { + return Err(ExprErr::ParseError( + loc, + format!("{integer} is too large, it does not fit into a uint256"), + )); + }; let val = if !exponent.is_empty() { let exp = U256::from_dec_str(exponent) .map_err(|e| ExprErr::ParseError(loc, e.to_string()))?; @@ -278,7 +283,7 @@ pub trait Literal: AnalyzerBackend + Sized { fn test_string_literal(&mut self, s: &str) -> Option { let split = s.split("::").collect::>(); - if split.get(0).copied() == Some("pyro") { + if split.first().copied() == Some("pyro") { match split.get(1).copied() { Some("variable") => { let name = split.get(2).copied()?; @@ -291,7 +296,7 @@ pub trait Literal: AnalyzerBackend + Sized { .trim_end_matches(']') .split(',') .collect::>(); - let mut min_str = *range.get(0)?; + let mut min_str = *range.first()?; let mut min_neg = false; if let Some(new_min) = min_str.strip_prefix('-') { min_neg = true; diff --git a/crates/solc-expressions/src/variable.rs b/crates/solc-expressions/src/variable.rs index 59bf4972..31e8e9f7 100644 --- a/crates/solc-expressions/src/variable.rs +++ b/crates/solc-expressions/src/variable.rs @@ -370,6 +370,7 @@ pub trait Variable: AnalyzerBackend + Size (ExprRet::Single(ty), None) => { let name = var_decl.name.clone().expect("Variable wasn't named"); let ty = VarType::try_from_idx(self, *ty).expect("Not a known type"); + let maybe_struct = ty.maybe_struct(); let var = ContextVar { loc: Some(loc), name: name.to_string(), @@ -385,6 +386,11 @@ pub trait Variable: AnalyzerBackend + Size let lhs = ContextVarNode::from(self.add_node(Node::ContextVar(var))); ctx.add_var(lhs, self).into_expr_err(loc)?; self.add_edge(lhs, ctx, Edge::Context(ContextEdge::Variable)); + if let Some(strukt) = maybe_struct { + strukt + .add_fields_to_cvar(self, loc, lhs) + .into_expr_err(loc)?; + } Ok(false) } (l @ ExprRet::Single(_lhs), Some(ExprRet::Multi(rhs_sides))) => Ok(rhs_sides diff --git a/mini.sol b/mini.sol index c1cc7602..2d301fb6 100644 --- a/mini.sol +++ b/mini.sol @@ -1,172 +1,14 @@ -contract ComptrollerErrorReporter { - enum Error { - NO_ERROR, - UNAUTHORIZED, - COMPTROLLER_MISMATCH, - INSUFFICIENT_SHORTFALL, - INSUFFICIENT_LIQUIDITY, - INVALID_CLOSE_FACTOR, - INVALID_COLLATERAL_FACTOR, - INVALID_LIQUIDATION_INCENTIVE, - MARKET_NOT_ENTERED, // no longer possible - MARKET_NOT_LISTED, - MARKET_ALREADY_LISTED, - MATH_ERROR, - NONZERO_BORROW_BALANCE, - PRICE_ERROR, - REJECTION, - SNAPSHOT_ERROR, - TOO_MANY_ASSETS, - TOO_MUCH_REPAY - } - - enum FailureInfo { - ACCEPT_ADMIN_PENDING_ADMIN_CHECK, - ACCEPT_PENDING_IMPLEMENTATION_ADDRESS_CHECK, - EXIT_MARKET_BALANCE_OWED, - EXIT_MARKET_REJECTION, - SET_CLOSE_FACTOR_OWNER_CHECK, - SET_CLOSE_FACTOR_VALIDATION, - SET_COLLATERAL_FACTOR_OWNER_CHECK, - SET_COLLATERAL_FACTOR_NO_EXISTS, - SET_COLLATERAL_FACTOR_VALIDATION, - SET_COLLATERAL_FACTOR_WITHOUT_PRICE, - SET_IMPLEMENTATION_OWNER_CHECK, - SET_LIQUIDATION_INCENTIVE_OWNER_CHECK, - SET_LIQUIDATION_INCENTIVE_VALIDATION, - SET_MAX_ASSETS_OWNER_CHECK, - SET_PENDING_ADMIN_OWNER_CHECK, - SET_PENDING_IMPLEMENTATION_OWNER_CHECK, - SET_PRICE_ORACLE_OWNER_CHECK, - SUPPORT_MARKET_EXISTS, - SUPPORT_MARKET_OWNER_CHECK, - SET_PAUSE_GUARDIAN_OWNER_CHECK - } - - function fail(Error err, FailureInfo info) internal returns (uint) { - emit Failure(uint(err), uint(info), 0); - - return uint(err); - } - -} - -contract CTokenStorage { - address payable public admin; - - -} - -contract ExponentialNoError { - struct Exp { - uint mantissa; - } - - function lessThanExp(Exp memory left, Exp memory right) pure internal returns (bool) { - return left.mantissa < right.mantissa; - } - -} - -abstract contract CToken is ExponentialNoError { - - -} - -contract UnitrollerAdminStorage { - address public admin; - - -} - -contract ComptrollerV1Storage is UnitrollerAdminStorage { - - -} - -contract ComptrollerV2Storage is ComptrollerV1Storage { - struct Market { - // Whether or not this market is listed - bool isListed; - - // Multiplier representing the most one can borrow against their collateral in this market. - // For instance, 0.9 to allow borrowing 90% of collateral value. - // Must be between 0 and 1, and stored as a mantissa. - uint collateralFactorMantissa; - - // Per-market mapping of "accounts in this asset" - mapping(address => bool) accountMembership; - - // Whether or not this market receives COMP - bool isComped; - } - - mapping(address => Market) public markets; - - -} - -contract ComptrollerV3Storage is ComptrollerV2Storage { - - -} - -contract ComptrollerV4Storage is ComptrollerV3Storage { - - -} - -contract ComptrollerV5Storage is ComptrollerV4Storage { - - -} - -contract ComptrollerV6Storage is ComptrollerV5Storage { - - -} - -contract ComptrollerV7Storage is ComptrollerV6Storage { - - -} - -contract Comptroller is ComptrollerV7Storage, ComptrollerErrorReporter, ExponentialNoError { - uint internal constant collateralFactorMaxMantissa = 0.9e18; - function _setCollateralFactor(CToken cToken, uint newCollateralFactorMantissa) external returns (uint) { - // Check caller is admin - if (msg.sender != admin) { - return fail(Error.UNAUTHORIZED, FailureInfo.SET_COLLATERAL_FACTOR_OWNER_CHECK); - } - - // Verify market is listed - Market storage market = markets[address(cToken)]; - if (!market.isListed) { - return fail(Error.MARKET_NOT_LISTED, FailureInfo.SET_COLLATERAL_FACTOR_NO_EXISTS); - } - - Exp memory newCollateralFactorExp = Exp({mantissa: newCollateralFactorMantissa}); - - // Check collateral factor <= 0.9 - Exp memory highLimit = Exp({mantissa: collateralFactorMaxMantissa}); - if (lessThanExp(highLimit, newCollateralFactorExp)) { - return fail(Error.INVALID_COLLATERAL_FACTOR, FailureInfo.SET_COLLATERAL_FACTOR_VALIDATION); - } - - // If collateral factor != 0, fail if price == 0 - if (newCollateralFactorMantissa != 0 && oracle.getUnderlyingPrice(cToken) == 0) { - return fail(Error.PRICE_ERROR, FailureInfo.SET_COLLATERAL_FACTOR_WITHOUT_PRICE); - } - - // Set market's collateral factor to new collateral factor, remember old value - uint oldCollateralFactorMantissa = market.collateralFactorMantissa; - market.collateralFactorMantissa = newCollateralFactorMantissa; - - // Emit event with asset, old collateral factor, and new collateral factor - emit NewCollateralFactor(cToken, oldCollateralFactorMantissa, newCollateralFactorMantissa); - - return uint(Error.NO_ERROR); - } +contract StruktTester { + struct A { + uint256 b; + uint256 c; + } + + function constructStruct() public { + A memory b = A({b: 100, c: 100}); + require(b.b == 100); + require(b.c == 100); + } } \ No newline at end of file From b929e9ca987fad926585e858c547e2701dece836 Mon Sep 17 00:00:00 2001 From: brock elmore Date: Thu, 11 Jul 2024 13:58:55 -0700 Subject: [PATCH 07/10] small fixes --- crates/graph/src/nodes/context/invariants.rs | 15 ++--- crates/graph/src/nodes/context/node.rs | 16 +++++ .../graph/src/nodes/context/var/invariants.rs | 39 ++++++++++- crates/graph/src/nodes/context/var/node.rs | 4 +- crates/graph/src/nodes/context/variables.rs | 67 +++++++------------ crates/graph/src/solvers/atoms.rs | 40 ++++++----- crates/graph/src/solvers/dl.rs | 2 +- 7 files changed, 107 insertions(+), 76 deletions(-) diff --git a/crates/graph/src/nodes/context/invariants.rs b/crates/graph/src/nodes/context/invariants.rs index f4522856..7922c4be 100644 --- a/crates/graph/src/nodes/context/invariants.rs +++ b/crates/graph/src/nodes/context/invariants.rs @@ -52,14 +52,13 @@ impl ContextNode { g: &impl GraphBackend, arena: &RangeArena>, ) -> Result, GraphError> { - let vars: Vec<_> = self.vars(g).values().collect(); - Ok(vars - .iter() - .map(|var| var.is_representation_ok(g, arena)) - .collect::>, _>>()? - .into_iter() - .flatten() - .collect::>()) + let mut res = vec![]; + for var in self.vars(g).values() { + if let Some(err) = var.is_representation_ok(g, arena)? { + res.push(err); + } + } + Ok(res) } } diff --git a/crates/graph/src/nodes/context/node.rs b/crates/graph/src/nodes/context/node.rs index b4b8759e..c8feab5b 100644 --- a/crates/graph/src/nodes/context/node.rs +++ b/crates/graph/src/nodes/context/node.rs @@ -114,6 +114,22 @@ impl ContextNode { Ok(()) } + /// Propogate that this context has ended up the context graph + pub fn propogate_applied( + &self, + applied: FunctionNode, + analyzer: &mut impl AnalyzerBackend, + ) -> Result<(), GraphError> { + let underlying = self.underlying_mut(analyzer)?; + if !underlying.applies.contains(&applied) { + underlying.applies.push(applied); + } + if let Some(parent) = self.underlying(analyzer)?.parent_ctx { + parent.propogate_applied(applied, analyzer)?; + } + Ok(()) + } + /// Gets the return nodes for this context pub fn return_nodes( &self, diff --git a/crates/graph/src/nodes/context/var/invariants.rs b/crates/graph/src/nodes/context/var/invariants.rs index 96761e86..5f2237c1 100644 --- a/crates/graph/src/nodes/context/var/invariants.rs +++ b/crates/graph/src/nodes/context/var/invariants.rs @@ -52,7 +52,10 @@ impl ContextVarNode { strukt_node: StructNode, g: &impl GraphBackend, ) -> Result, GraphError> { - if let Some(err) = self.struct_has_all_fields(strukt_node, g)? { + if let Some(err) = self + .first_version(g) + .struct_has_all_fields(strukt_node, g)? + { return Ok(Some(err)); } Ok(None) @@ -83,6 +86,7 @@ impl ContextVarNode { .to_string() }) .collect(); + let mut target_field_names: Vec<_> = struct_fields .iter() .map(|field| field.name(g).unwrap()) @@ -90,9 +94,38 @@ impl ContextVarNode { real_field_names.sort(); target_field_names.sort(); - if real_field_names != target_field_names { + if real_field_names.len() < target_field_names.len() { + Ok(Some( + VarReprErr::StructErr( + self.0.into(), + Box::leak( + format!("{} has missing fields", self.display_name(g).unwrap()) + .into_boxed_str(), + ), + ) + .into(), + )) + } else if real_field_names.len() > target_field_names.len() { + Ok(Some( + VarReprErr::StructErr( + self.0.into(), + Box::leak( + format!("{} has extra fields", self.display_name(g).unwrap()) + .into_boxed_str(), + ), + ) + .into(), + )) + } else if real_field_names != target_field_names { Ok(Some( - VarReprErr::StructErr(self.0.into(), "Missing fields").into(), + VarReprErr::StructErr( + self.0.into(), + Box::leak( + format!("{} has misnamed fields", self.display_name(g).unwrap()) + .into_boxed_str(), + ), + ) + .into(), )) } else { Ok(None) diff --git a/crates/graph/src/nodes/context/var/node.rs b/crates/graph/src/nodes/context/var/node.rs index efe13bce..9dc9dbc4 100644 --- a/crates/graph/src/nodes/context/var/node.rs +++ b/crates/graph/src/nodes/context/var/node.rs @@ -230,14 +230,14 @@ impl ContextVarNode { } pub fn is_struct(&self, analyzer: &impl GraphBackend) -> Result { - Ok(!self.struct_to_fields(analyzer)?.is_empty()) + Ok(self.ty(analyzer)?.maybe_struct().is_some()) } pub fn struct_to_fields( &self, analyzer: &impl GraphBackend, ) -> Result, GraphError> { - if self.ref_range(analyzer)?.is_none() { + if self.is_struct(analyzer)? { let fields = analyzer .graph() .edges_directed(self.first_version(analyzer).into(), Direction::Incoming) diff --git a/crates/graph/src/nodes/context/variables.rs b/crates/graph/src/nodes/context/variables.rs index ff27c40f..03fdce21 100644 --- a/crates/graph/src/nodes/context/variables.rs +++ b/crates/graph/src/nodes/context/variables.rs @@ -123,56 +123,26 @@ impl ContextNode { full_name: &str, ) -> Result, GraphError> { let split = full_name.split('.').collect::>(); - if split.len() != 2 { + if split.len() < 2 { return Ok(None); } - let member_name = split[0]; - let field_name = split[1]; + let member_name = split[0..=split.len() - 2].join("."); + let field_name = split.last().unwrap(); // get the member - let Some(member) = self.var_by_name_or_recurse(analyzer, member_name)? else { + let Some(member) = self.var_by_name_or_recurse(analyzer, &member_name)? else { return Ok(None); }; // maybe move var into this context let member = self.maybe_move_var(member, loc, analyzer)?; - let global_first = member.global_first_version(analyzer); - - let mut curr = member; - let mut field = None; - // recursively search for the field by looking at all major versions of the member (i.e. first version - // of the variable in a context) - while field.is_none() && curr != global_first { - field = curr.field_of_struct(field_name, analyzer)?; - if let Some(prev) = curr.previous_or_inherited_version(analyzer) { - curr = prev; - } else { - break; - } - } - - if let Some(field) = field { - if let Some(ctx) = curr.maybe_ctx(analyzer) { - if ctx != *self { - tracing::trace!( - "moving field access {} from {} to {}", - field.display_name(analyzer).unwrap(), - ctx.path(analyzer), - self.path(analyzer) - ); - let mut new_cvar = field.latest_version(analyzer).underlying(analyzer)?.clone(); - new_cvar.loc = Some(loc); - - let new_cvarnode = analyzer.add_node(Node::ContextVar(new_cvar)); - analyzer.add_edge( - new_cvarnode, - member, - Edge::Context(ContextEdge::AttrAccess("field")), - ); - } - } - } + let fields = member.struct_to_fields(analyzer)?; + let field = fields.into_iter().find(|field| { + let full_name = field.name(analyzer).unwrap(); + let target_field_name = full_name.split('.').last().unwrap(); + *field_name == target_field_name + }); Ok(field) } @@ -239,6 +209,7 @@ impl ContextNode { } pub fn contract_vars_referenced_global(&self, analyzer: &impl AnalyzerBackend) -> Vec { + println!("getting storage vars for: {}", self.path(analyzer)); let mut reffed_storage = self.contract_vars_referenced(analyzer); analyzer .graph() @@ -460,14 +431,22 @@ impl ContextNode { let mut new_cvar = var.latest_version(analyzer).underlying(analyzer)?.clone(); new_cvar.loc = Some(loc); - let new_cvarnode = analyzer.add_node(Node::ContextVar(new_cvar)); - self.add_var(ContextVarNode::from(new_cvarnode), analyzer)?; - analyzer.add_edge(new_cvarnode, *self, Edge::Context(ContextEdge::Variable)); + let new_cvarnode = + ContextVarNode::from(analyzer.add_node(Node::ContextVar(new_cvar))); + + self.add_var(new_cvarnode, analyzer)?; + analyzer.add_edge(new_cvarnode.0, *self, Edge::Context(ContextEdge::Variable)); analyzer.add_edge( - new_cvarnode, + new_cvarnode.0, var.0, Edge::Context(ContextEdge::InheritedVariable), ); + + let fields = new_cvarnode.struct_to_fields(analyzer)?; + fields.iter().try_for_each(|field| { + let _ = self.maybe_move_var(*field, loc, analyzer)?; + Ok(()) + })?; Ok(new_cvarnode.into()) } else { Ok(var) diff --git a/crates/graph/src/solvers/atoms.rs b/crates/graph/src/solvers/atoms.rs index 27f0d3c3..50d0e02d 100644 --- a/crates/graph/src/solvers/atoms.rs +++ b/crates/graph/src/solvers/atoms.rs @@ -275,6 +275,7 @@ pub static LIA_OPS: &[RangeOp] = &[ pub trait Atomize { fn atoms_or_part( &self, + prev: Option<&Self>, analyzer: &mut impl GraphBackend, arena: &mut RangeArena>, ) -> AtomOrPart; @@ -289,11 +290,20 @@ impl Atomize for Elem { #[tracing::instrument(level = "trace", skip_all)] fn atoms_or_part( &self, + prev: Option<&Self>, analyzer: &mut impl GraphBackend, arena: &mut RangeArena>, ) -> AtomOrPart { + if let Some(prev) = prev { + if *prev == *self { + return AtomOrPart::Part(self.clone()); + } + } match self { - Elem::Arena(_) => self.dearenaize_clone(arena).atoms_or_part(analyzer, arena), + Elem::Arena(_) => { + self.dearenaize_clone(arena) + .atoms_or_part(Some(&self), analyzer, arena) + } Elem::Concrete(_) | Elem::Reference(_) => AtomOrPart::Part(self.clone()), Elem::ConcreteDyn(_) => AtomOrPart::Part(self.clone()), _e @ Elem::Expr(expr) => { @@ -301,17 +311,17 @@ impl Atomize for Elem { match collapse(*expr.lhs.clone(), expr.op, *expr.rhs.clone(), arena) { MaybeCollapsed::Concretes(_l, _r) => { let exec_res = expr.exec_op(true, analyzer, arena).unwrap(); - return exec_res.atoms_or_part(analyzer, arena); + return exec_res.atoms_or_part(Some(&self), analyzer, arena); } MaybeCollapsed::Collapsed(elem) => { - return elem.atoms_or_part(analyzer, arena); + return elem.atoms_or_part(Some(&self), analyzer, arena); } MaybeCollapsed::Not(..) => {} } match ( - expr.lhs.atoms_or_part(analyzer, arena), - expr.rhs.atoms_or_part(analyzer, arena), + expr.lhs.atoms_or_part(Some(&self), analyzer, arena), + expr.rhs.atoms_or_part(Some(&self), analyzer, arena), ) { (ref lp @ AtomOrPart::Part(ref l), ref rp @ AtomOrPart::Part(ref r)) => { // println!("part part"); @@ -348,28 +358,22 @@ impl Atomize for Elem { }; AtomOrPart::Atom(atom) } - (Elem::Expr(_), Elem::Expr(_)) => { - todo!("here"); - } + (Elem::Expr(_), Elem::Expr(_)) => AtomOrPart::Part(self.clone()), (Elem::Expr(_), Elem::Reference(Reference { .. })) => { - todo!("here1"); + AtomOrPart::Part(self.clone()) } (Elem::Reference(Reference { .. }), Elem::Expr(_)) => { - todo!("here2"); - } - (Elem::Expr(_), Elem::Concrete(_)) => { - todo!("here3"); - } - (Elem::Concrete(_), Elem::Expr(_)) => { - todo!("here4"); + AtomOrPart::Part(self.clone()) } + (Elem::Expr(_), Elem::Concrete(_)) => AtomOrPart::Part(self.clone()), + (Elem::Concrete(_), Elem::Expr(_)) => AtomOrPart::Part(self.clone()), (Elem::Concrete(_), Elem::Concrete(_)) => { let _ = expr.clone().arenaize(analyzer, arena); let res = expr.exec_op(true, analyzer, arena).unwrap(); if res == Elem::Expr(expr.clone()) { AtomOrPart::Part(res) } else { - res.atoms_or_part(analyzer, arena) + res.atoms_or_part(Some(&self), analyzer, arena) } } (Elem::ConcreteDyn(_), _) => AtomOrPart::Part(Elem::Null), @@ -412,7 +416,7 @@ impl Atomize for Elem { ConcreteDyn(_) => None, //{ println!("was concDyn"); None}, Expr(_) => { // println!("atomized: was expr"); - let AtomOrPart::Atom(mut a) = self.atoms_or_part(analyzer, arena) else { + let AtomOrPart::Atom(mut a) = self.atoms_or_part(None, analyzer, arena) else { // println!("returning none"); return None; }; diff --git a/crates/graph/src/solvers/dl.rs b/crates/graph/src/solvers/dl.rs index df850525..c7f13c91 100644 --- a/crates/graph/src/solvers/dl.rs +++ b/crates/graph/src/solvers/dl.rs @@ -863,7 +863,7 @@ impl DLSolver { .rhs .into_elem() .wrapping_sub(Elem::from(Concrete::from(U256::one()))) - .atoms_or_part(analyzer, arena); + .atoms_or_part(None, analyzer, arena); Self::dl_atom_normalize( SolverAtom { ty: OpType::DL, From 0fbf88aafabe57bc5f7fa382a973fd1b2a4d5072 Mon Sep 17 00:00:00 2001 From: brock elmore Date: Thu, 11 Jul 2024 14:00:50 -0700 Subject: [PATCH 08/10] import fix --- crates/graph/src/nodes/context/node.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/graph/src/nodes/context/node.rs b/crates/graph/src/nodes/context/node.rs index c8feab5b..7640649d 100644 --- a/crates/graph/src/nodes/context/node.rs +++ b/crates/graph/src/nodes/context/node.rs @@ -1,5 +1,5 @@ use crate::{ - nodes::{Concrete, Context, ContextVarNode, KilledKind}, + nodes::{Concrete, Context, ContextVarNode, FunctionNode, KilledKind}, range::elem::Elem, AnalyzerBackend, AsDotStr, GraphBackend, Node, }; From b4855ed4c39bfd9a16f4ac4fe73a69c610303a04 Mon Sep 17 00:00:00 2001 From: brock elmore Date: Thu, 11 Jul 2024 17:03:10 -0700 Subject: [PATCH 09/10] comptroller runs in ~9s --- crates/graph/src/graph_elements.rs | 11 +- crates/graph/src/nodes/context/node.rs | 1 + crates/graph/src/nodes/context/solving.rs | 8 +- crates/graph/src/nodes/context/var/node.rs | 1 + crates/graph/src/nodes/context/var/ranging.rs | 4 +- .../graph/src/nodes/context/var/underlying.rs | 4 +- crates/graph/src/nodes/context/variables.rs | 29 ++- .../graph/src/nodes/debug_reconstruction.rs | 4 + crates/graph/src/nodes/struct_ty.rs | 4 + .../src/range/elem/elem_enum/range_elem.rs | 13 +- crates/graph/src/range/range_trait.rs | 4 +- crates/graph/src/range/solc_range.rs | 75 ++++--- crates/graph/src/solvers/dl.rs | 2 +- crates/pyrometer/src/analyzer.rs | 4 + crates/pyrometer/src/analyzer_backend.rs | 60 +++--- crates/pyrometer/src/graph_backend.rs | 34 ++-- crates/shared/src/analyzer_like.rs | 2 +- crates/shared/src/graph_like.rs | 16 +- crates/solc-expressions/src/assign.rs | 85 ++++++-- .../src/context_builder/expr.rs | 5 +- .../src/context_builder/stmt.rs | 14 +- .../solc-expressions/src/func_call/apply.rs | 183 ++++++++++++------ .../solc-expressions/src/func_call/helper.rs | 57 +++--- .../src/member_access/struct_access.rs | 1 + crates/solc-expressions/src/require.rs | 20 +- crates/solc-expressions/src/variable.rs | 60 ++++++ mini.sol | 44 +++-- 27 files changed, 507 insertions(+), 238 deletions(-) diff --git a/crates/graph/src/graph_elements.rs b/crates/graph/src/graph_elements.rs index cd0cb3b7..569befc7 100644 --- a/crates/graph/src/graph_elements.rs +++ b/crates/graph/src/graph_elements.rs @@ -10,7 +10,7 @@ use lazy_static::lazy_static; use petgraph::{Directed, Graph}; use solang_parser::pt::{Identifier, Loc}; -use std::collections::HashMap; +use std::collections::{BTreeSet, HashMap}; pub trait RepresentationInvariant { fn is_representation_ok( @@ -402,6 +402,15 @@ impl GraphLike for DummyGraph { panic!("Dummy Graph") } + fn mark_dirty(&mut self, _node: NodeIdx) {} + fn dirty_nodes(&self) -> &BTreeSet { + todo!() + } + + fn dirty_nodes_mut(&mut self) -> &mut BTreeSet { + todo!() + } + fn graph(&self) -> &Graph { panic!("Dummy Graph") } diff --git a/crates/graph/src/nodes/context/node.rs b/crates/graph/src/nodes/context/node.rs index 7640649d..314e19d2 100644 --- a/crates/graph/src/nodes/context/node.rs +++ b/crates/graph/src/nodes/context/node.rs @@ -52,6 +52,7 @@ impl ContextNode { &self, analyzer: &'a mut impl AnalyzerBackend, ) -> Result<&'a mut Context, GraphError> { + analyzer.mark_dirty(self.0.into()); match analyzer.node_mut(*self) { Node::Context(c) => Ok(c), Node::Unresolved(ident) => Err(GraphError::UnknownVariable(format!( diff --git a/crates/graph/src/nodes/context/solving.rs b/crates/graph/src/nodes/context/solving.rs index a67ba661..fc74dae9 100644 --- a/crates/graph/src/nodes/context/solving.rs +++ b/crates/graph/src/nodes/context/solving.rs @@ -54,10 +54,10 @@ impl ContextNode { Ok(ranges .iter() .filter_map(|(_dep, range)| { - if let Some(atom) = Elem::Arena(range.min).atomize(analyzer, arena) { + if let Some(atom) = range.min.atomize(analyzer, arena) { Some(atom) } else { - Elem::Arena(range.max).atomize(analyzer, arena) + range.max.atomize(analyzer, arena) } }) .collect::>()) @@ -131,7 +131,7 @@ impl ContextNode { let r = range.flattened_range(analyzer, arena)?.into_owned(); // add the atomic constraint - if let Some(atom) = Elem::Arena(r.min).atomize(analyzer, arena) { + if let Some(atom) = r.min.atomize(analyzer, arena) { let mut solver = std::mem::take(&mut self.underlying_mut(analyzer)?.dl_solver); let constraints = solver.add_constraints(vec![atom], analyzer, arena); constraints @@ -140,7 +140,7 @@ impl ContextNode { solver.add_constraint(constraint, normalized); }); self.underlying_mut(analyzer)?.dl_solver = solver; - } else if let Some(atom) = Elem::Arena(r.max).atomize(analyzer, arena) { + } else if let Some(atom) = r.max.atomize(analyzer, arena) { let mut solver = std::mem::take(&mut self.underlying_mut(analyzer)?.dl_solver); let constraints = solver.add_constraints(vec![atom], analyzer, arena); constraints diff --git a/crates/graph/src/nodes/context/var/node.rs b/crates/graph/src/nodes/context/var/node.rs index 9dc9dbc4..85adfa65 100644 --- a/crates/graph/src/nodes/context/var/node.rs +++ b/crates/graph/src/nodes/context/var/node.rs @@ -81,6 +81,7 @@ impl ContextVarNode { &self, analyzer: &'a mut impl GraphBackend, ) -> Result<&'a mut ContextVar, GraphError> { + analyzer.mark_dirty(self.0.into()); match analyzer.node_mut(*self) { Node::ContextVar(c) => Ok(c), Node::Unresolved(ident) => Err(GraphError::UnknownVariable(format!( diff --git a/crates/graph/src/nodes/context/var/ranging.rs b/crates/graph/src/nodes/context/var/ranging.rs index a4690704..eff44095 100644 --- a/crates/graph/src/nodes/context/var/ranging.rs +++ b/crates/graph/src/nodes/context/var/ranging.rs @@ -309,7 +309,7 @@ impl ContextVarNode { pub fn set_range_exclusions( &self, analyzer: &mut impl GraphBackend, - new_exclusions: Vec, + new_exclusions: Vec>, ) -> Result<(), GraphError> { tracing::trace!( "setting range exclusions for {}", @@ -405,7 +405,7 @@ impl ContextVarNode { pub fn try_set_range_exclusions( &self, analyzer: &mut impl GraphBackend, - new_exclusions: Vec, + new_exclusions: Vec>, ) -> Result { tracing::trace!( "setting range exclusions for: {}", diff --git a/crates/graph/src/nodes/context/var/underlying.rs b/crates/graph/src/nodes/context/var/underlying.rs index 0b1993c1..e8df5dba 100644 --- a/crates/graph/src/nodes/context/var/underlying.rs +++ b/crates/graph/src/nodes/context/var/underlying.rs @@ -400,7 +400,7 @@ impl ContextVar { pub fn set_range_exclusions( &mut self, - new_exclusions: Vec, + new_exclusions: Vec>, fallback_range: Option, ) -> Result<(), GraphError> { match &mut self.ty { @@ -457,7 +457,7 @@ impl ContextVar { pub fn try_set_range_exclusions( &mut self, - new_exclusions: Vec, + new_exclusions: Vec>, fallback_range: Option, ) -> bool { match &mut self.ty { diff --git a/crates/graph/src/nodes/context/variables.rs b/crates/graph/src/nodes/context/variables.rs index 03fdce21..a3573c69 100644 --- a/crates/graph/src/nodes/context/variables.rs +++ b/crates/graph/src/nodes/context/variables.rs @@ -402,6 +402,30 @@ impl ContextNode { } } + pub fn recursive_move_struct_field( + &self, + parent: ContextVarNode, + field: ContextVarNode, + loc: Loc, + analyzer: &mut impl AnalyzerBackend, + ) -> Result<(), GraphError> { + let mut new_cvar = field.latest_version(analyzer).underlying(analyzer)?.clone(); + new_cvar.loc = Some(loc); + + let new_cvarnode = ContextVarNode::from(analyzer.add_node(Node::ContextVar(new_cvar))); + + analyzer.add_edge( + new_cvarnode.0, + parent.0, + Edge::Context(ContextEdge::AttrAccess("field")), + ); + + let sub_fields = field.struct_to_fields(analyzer)?; + sub_fields.iter().try_for_each(|sub_field| { + self.recursive_move_struct_field(new_cvarnode, *sub_field, loc, analyzer) + }) + } + /// May move the variable from an old context to this context pub fn maybe_move_var( &self, @@ -442,10 +466,9 @@ impl ContextNode { Edge::Context(ContextEdge::InheritedVariable), ); - let fields = new_cvarnode.struct_to_fields(analyzer)?; + let fields = var.struct_to_fields(analyzer)?; fields.iter().try_for_each(|field| { - let _ = self.maybe_move_var(*field, loc, analyzer)?; - Ok(()) + self.recursive_move_struct_field(new_cvarnode, *field, loc, analyzer) })?; Ok(new_cvarnode.into()) } else { diff --git a/crates/graph/src/nodes/debug_reconstruction.rs b/crates/graph/src/nodes/debug_reconstruction.rs index f286397d..166b6c41 100644 --- a/crates/graph/src/nodes/debug_reconstruction.rs +++ b/crates/graph/src/nodes/debug_reconstruction.rs @@ -502,6 +502,10 @@ impl FunctionNode { &self, analyzer: &mut impl AnalyzerBackend, ) -> FuncReconstructionReqs { + println!( + "reconstruction requirements for: {}", + self.name(analyzer).unwrap() + ); FuncReconstructionReqs { storage: self.maybe_used_storage(analyzer).unwrap_or_default(), usertypes: self.maybe_used_usertypes(analyzer).unwrap_or_default(), diff --git a/crates/graph/src/nodes/struct_ty.rs b/crates/graph/src/nodes/struct_ty.rs index 5ce87170..e739a711 100644 --- a/crates/graph/src/nodes/struct_ty.rs +++ b/crates/graph/src/nodes/struct_ty.rs @@ -104,6 +104,10 @@ impl StructNode { cvar, Edge::Context(ContextEdge::AttrAccess("field")), ); + // do so recursively + if let Some(field_struct) = ContextVarNode::from(fc_node).ty(analyzer)?.maybe_struct() { + field_struct.add_fields_to_cvar(analyzer, loc, ContextVarNode::from(fc_node))?; + } Ok(()) }) } diff --git a/crates/graph/src/range/elem/elem_enum/range_elem.rs b/crates/graph/src/range/elem/elem_enum/range_elem.rs index c00d5e40..5e9ffcad 100644 --- a/crates/graph/src/range/elem/elem_enum/range_elem.rs +++ b/crates/graph/src/range/elem/elem_enum/range_elem.rs @@ -482,7 +482,6 @@ impl RangeElem for Elem { return Ok(*min.clone()); } } - Null => return Ok(Elem::Null), _ => {} } } @@ -513,15 +512,21 @@ impl RangeElem for Elem { match arena.ranges.get_mut(idx) { Some(Self::Reference(ref mut d)) => { - tracing::trace!("simplify minimize cache MISS: {self}"); + tracing::trace!( + "simplify minimize cache MISS: {self}, new simp min: {min}" + ); d.flattened_min = Some(Box::new(min.clone())); } Some(Self::Expr(ref mut expr)) => { - tracing::trace!("simplify minimize cache MISS: {self}"); + tracing::trace!( + "simplify minimize cache MISS: {self}, new simp min: {min}" + ); expr.flattened_min = Some(Box::new(min.clone())); } Some(Self::ConcreteDyn(ref mut d)) => { - tracing::trace!("simplify minimize cache MISS: {self}"); + tracing::trace!( + "simplify minimize cache MISS: {self}, new simp min: {min}" + ); d.flattened_min = Some(Box::new(min.clone())); } _ => {} diff --git a/crates/graph/src/range/range_trait.rs b/crates/graph/src/range/range_trait.rs index e5b1bd75..261e7336 100644 --- a/crates/graph/src/range/range_trait.rs +++ b/crates/graph/src/range/range_trait.rs @@ -61,11 +61,11 @@ pub trait Range { /// Set the range maximum fn set_range_max(&mut self, new: Self::ElemTy); /// Set the range exclusions - fn set_range_exclusions(&mut self, new: Vec) + fn set_range_exclusions(&mut self, new: Vec) where Self: std::marker::Sized; /// Add an exclusion value to the range - fn add_range_exclusion(&mut self, new: usize) + fn add_range_exclusion(&mut self, new: Self::ElemTy) where Self: std::marker::Sized; /// Replace a potential recursion causing node index with a new index diff --git a/crates/graph/src/range/solc_range.rs b/crates/graph/src/range/solc_range.rs index 151971fe..3ba41dcd 100644 --- a/crates/graph/src/range/solc_range.rs +++ b/crates/graph/src/range/solc_range.rs @@ -13,18 +13,14 @@ use std::{borrow::Cow, collections::BTreeMap}; #[derive(Clone, Debug, PartialEq, Eq, Ord, PartialOrd, Hash)] pub struct FlattenedRange { - pub min: usize, - pub max: usize, - pub exclusions: Vec, + pub min: Elem, + pub max: Elem, + pub exclusions: Vec>, } impl From for SolcRange { fn from(range: FlattenedRange) -> Self { - SolcRange::new( - Elem::Arena(range.min), - Elem::Arena(range.max), - range.exclusions, - ) + SolcRange::new(range.min, range.max, range.exclusions) } } @@ -34,7 +30,7 @@ pub struct SolcRange { pub min_cached: Option, pub max: Elem, pub max_cached: Option, - pub exclusions: Vec, + pub exclusions: Vec>, pub flattened: Option, } @@ -56,7 +52,7 @@ impl AsDotStr for SolcRange { .s, self.exclusions .iter() - .map(|excl| Elem::Arena(*excl).to_range_string(false, analyzer, arena).s) + .map(|excl| excl.to_range_string(false, analyzer, arena).s) .collect::>() .join(", ") ) @@ -102,7 +98,7 @@ impl SolcRange { Ok(deps) } - pub fn new(min: Elem, max: Elem, exclusions: Vec) -> Self { + pub fn new(min: Elem, max: Elem, exclusions: Vec>) -> Self { Self { min, min_cached: None, @@ -121,18 +117,12 @@ impl SolcRange { arena: &mut RangeArena>, ) { if let Some(ref mut flattened) = &mut self.flattened { - Elem::Arena(flattened.min).replace_dep( - to_replace, - replacement.clone(), - analyzer, - arena, - ); - Elem::Arena(flattened.max).replace_dep( - to_replace, - replacement.clone(), - analyzer, - arena, - ); + flattened + .min + .replace_dep(to_replace, replacement.clone(), analyzer, arena); + flattened + .max + .replace_dep(to_replace, replacement.clone(), analyzer, arena); } self.min .replace_dep(to_replace, replacement.clone(), analyzer, arena); @@ -538,22 +528,24 @@ impl SolcRange { return Ok(cached.clone()); } - let mut min = Elem::Arena(arena.idx_or_upsert(self.min.clone(), analyzer)); - let mut max = Elem::Arena(arena.idx_or_upsert(self.max.clone(), analyzer)); + let mut min = self.min.clone(); + min.arenaize(analyzer, arena)?; min.cache_flatten(analyzer, arena)?; + let mut max = self.max.clone(); + max.arenaize(analyzer, arena)?; max.cache_flatten(analyzer, arena)?; self.min = min.clone(); self.max = max.clone(); - let simp_min = min.simplify_minimize(analyzer, arena)?; - let simp_max = max.simplify_maximize(analyzer, arena)?; - let min = arena.idx_or_upsert(simp_min, analyzer); - let max = arena.idx_or_upsert(simp_max, analyzer); + let mut simp_min = min.simplify_minimize(analyzer, arena)?; + simp_min.arenaize(analyzer, arena)?; + let mut simp_max = max.simplify_maximize(analyzer, arena)?; + simp_max.arenaize(analyzer, arena)?; let flat_range = FlattenedRange { - min, - max, + min: simp_min, + max: simp_max, exclusions: self.exclusions.clone(), }; self.flattened = Some(flat_range.clone()); @@ -583,10 +575,12 @@ impl Range for SolcRange { analyzer: &mut impl GraphBackend, arena: &mut RangeArena>, ) -> Result<(), GraphError> { - let min = std::mem::take(&mut self.min); - let max = std::mem::take(&mut self.max); - self.min = Elem::Arena(arena.idx_or_upsert(min, analyzer)); - self.max = Elem::Arena(arena.idx_or_upsert(max, analyzer)); + let mut min = std::mem::take(&mut self.min); + let mut max = std::mem::take(&mut self.max); + min.arenaize(analyzer, arena)?; + max.arenaize(analyzer, arena)?; + self.min = min; + self.max = max; if self.max_cached.is_none() { let max = self.range_max_mut(); max.cache_maximize(analyzer, arena)?; @@ -646,11 +640,7 @@ impl Range for SolcRange { } fn range_exclusions(&self) -> Vec { - self.exclusions - .clone() - .into_iter() - .map(Elem::Arena) - .collect() + self.exclusions.clone().into_iter().collect() } fn set_range_min(&mut self, new: Self::ElemTy) { self.min_cached = None; @@ -663,14 +653,15 @@ impl Range for SolcRange { self.max = new; } - fn add_range_exclusion(&mut self, new: usize) { + fn add_range_exclusion(&mut self, new: Elem) { if !self.exclusions.contains(&new) { self.exclusions.push(new); } } - fn set_range_exclusions(&mut self, new: Vec) { + fn set_range_exclusions(&mut self, new: Vec>) { self.exclusions = new; } + fn filter_min_recursion( &mut self, self_idx: NodeIdx, diff --git a/crates/graph/src/solvers/dl.rs b/crates/graph/src/solvers/dl.rs index c7f13c91..b413e840 100644 --- a/crates/graph/src/solvers/dl.rs +++ b/crates/graph/src/solvers/dl.rs @@ -1059,7 +1059,7 @@ impl DLSolver { }; Some(res) } - other => panic!("other op: {}, {constraint:#?}", other.to_string()), + _other => return None, } } else if constraint.rhs.is_part() { let new_rhs = AtomOrPart::Atom(SolverAtom { diff --git a/crates/pyrometer/src/analyzer.rs b/crates/pyrometer/src/analyzer.rs index 11675376..a1017491 100644 --- a/crates/pyrometer/src/analyzer.rs +++ b/crates/pyrometer/src/analyzer.rs @@ -24,6 +24,7 @@ use solang_parser::{ }, }; +use std::collections::BTreeSet; use std::{ collections::BTreeMap, fs, @@ -111,6 +112,8 @@ pub struct Analyzer { pub block: BlockNode, /// The underlying graph holding all of the elements of the contracts pub graph: Graph, + /// Nodes that may have been mutated in some way + pub dirty_nodes: BTreeSet, /// The entry node - this is the root of the dag, all relevant things should eventually point back to this (otherwise can be discarded) pub entry: NodeIdx, /// A mapping of a solidity builtin to the index in the graph @@ -157,6 +160,7 @@ impl Default for Analyzer { tmp_msg: None, block: BlockNode(0), graph: Default::default(), + dirty_nodes: Default::default(), entry: NodeIndex::from(0), builtins: Default::default(), user_types: Default::default(), diff --git a/crates/pyrometer/src/analyzer_backend.rs b/crates/pyrometer/src/analyzer_backend.rs index 2d0892c3..1562477c 100644 --- a/crates/pyrometer/src/analyzer_backend.rs +++ b/crates/pyrometer/src/analyzer_backend.rs @@ -2,9 +2,9 @@ use crate::Analyzer; use graph::{ elem::Elem, nodes::{ - BlockNode, Builtin, Concrete, ConcreteNode, ContextNode, ContextVar, ContractNode, - FuncReconstructionReqs, Function, FunctionNode, FunctionParam, FunctionParamNode, - FunctionReturn, KilledKind, MsgNode, + BlockNode, Builtin, Concrete, ConcreteNode, ContextNode, ContextVar, ContextVarNode, + ContractNode, FuncReconstructionReqs, Function, FunctionNode, FunctionParam, + FunctionParamNode, FunctionReturn, KilledKind, MsgNode, }, AnalyzerBackend, Edge, GraphBackend, Node, RepresentationInvariant, TypeNode, VarType, }; @@ -82,6 +82,14 @@ impl AnalyzerLike for Analyzer { needed_functions.sort_by(|a, b| a.0.cmp(&b.0)); needed_functions.dedup(); + println!( + "needed functions: {:#?}", + needed_functions + .iter() + .map(|i| i.name(self).unwrap()) + .collect::>() + ); + fn recurse_find( contract: ContractNode, target_contract: ContractNode, @@ -110,13 +118,12 @@ impl AnalyzerLike for Analyzer { Vec<(FunctionNode, FuncReconstructionReqs)>, > = Default::default(); - println!("contract_to_funcs: {contract_to_funcs:#?}"); - let mut tys = vec![]; let mut enums = vec![]; let mut structs = vec![]; let mut errs = vec![]; needed_functions.into_iter().for_each(|func| { + println!("iterating with func: {}", func.name(self).unwrap()); let maybe_func_contract = func.maybe_associated_contract(self); let reqs = func.reconstruction_requirements(self); reqs.usertypes.iter().for_each(|var| { @@ -260,11 +267,7 @@ impl AnalyzerLike for Analyzer { Some(reconstruction_edge) } - e => { - println!("here4"); - println!("{e:?}"); - None - } + e => None, } } _ => None, @@ -527,21 +530,30 @@ impl AnalyzerLike for Analyzer { } fn is_representation_ok( - &self, + &mut self, arena: &RangeArena<::RangeElem>, ) -> Result, GraphError> { - let t: Vec<_> = self - .graph() - .node_indices() - .map(|idx| match self.node(idx) { - Node::Context(..) => ContextNode::from(idx).is_representation_ok(self, arena), - _ => Ok(None), - }) - .collect::>, GraphError>>()? - .into_iter() - .flatten() - .collect(); - - Ok(t) + let mut res = vec![]; + let dirty = self.take_dirty_nodes(); + for node in dirty { + match self.node(node) { + Node::Context(..) => { + if let Some(err) = ContextNode::from(node).is_representation_ok(self, arena)? { + res.push(err); + } + } + Node::ContextVar(..) => { + if ContextVarNode::from(node).maybe_ctx(self).is_some() { + if let Some(err) = + ContextVarNode::from(node).is_representation_ok(self, arena)? + { + res.push(err); + } + } + } + _ => {} + } + } + Ok(res) } } diff --git a/crates/pyrometer/src/graph_backend.rs b/crates/pyrometer/src/graph_backend.rs index 062e8bdc..55d65308 100644 --- a/crates/pyrometer/src/graph_backend.rs +++ b/crates/pyrometer/src/graph_backend.rs @@ -35,26 +35,15 @@ impl GraphLike for Analyzer { &self.graph } - fn add_node(&mut self, node: impl Into) -> NodeIdx - where - Self: std::marker::Sized, - Self: GraphLike, - { - let res = self.graph_mut().add_node(node.into()); - res + fn mark_dirty(&mut self, node: NodeIdx) { + self.dirty_nodes.insert(node); } - fn add_edge( - &mut self, - from_node: impl Into, - to_node: impl Into, - edge: impl Into, - ) where - Self: std::marker::Sized, - Self: GraphLike, - { - self.graph_mut() - .add_edge(from_node.into(), to_node.into(), edge.into()); + fn dirty_nodes(&self) -> &BTreeSet { + &self.dirty_nodes + } + fn dirty_nodes_mut(&mut self) -> &mut BTreeSet { + &mut self.dirty_nodes } } @@ -1211,6 +1200,15 @@ impl GraphLike for G<'_> { panic!("Should not call this") } + fn mark_dirty(&mut self, _node: NodeIdx) {} + fn dirty_nodes(&self) -> &BTreeSet { + panic!("Should not call this") + } + + fn dirty_nodes_mut(&mut self) -> &mut BTreeSet { + panic!("Should not call this") + } + fn graph(&self) -> &Graph { self.graph } diff --git a/crates/shared/src/analyzer_like.rs b/crates/shared/src/analyzer_like.rs index 44006c98..e9d68b8b 100644 --- a/crates/shared/src/analyzer_like.rs +++ b/crates/shared/src/analyzer_like.rs @@ -190,7 +190,7 @@ pub trait AnalyzerLike: GraphLike { fn minimize_debug(&self) -> &Option; fn minimize_err(&mut self, ctx: Self::ContextNode) -> String; fn is_representation_ok( - &self, + &mut self, arena: &RangeArena<::RangeElem>, ) -> Result, GraphError>; } diff --git a/crates/shared/src/graph_like.rs b/crates/shared/src/graph_like.rs index bceee4f6..7cb36f22 100644 --- a/crates/shared/src/graph_like.rs +++ b/crates/shared/src/graph_like.rs @@ -42,6 +42,7 @@ pub trait GraphLike { /// Add a node to the graph fn add_node(&mut self, node: impl Into) -> NodeIdx { let res = self.graph_mut().add_node(node.into()); + self.mark_dirty(res); res } /// Get a reference to a node in the graph @@ -56,6 +57,14 @@ pub trait GraphLike { .node_weight_mut(node.into()) .expect("Index not in graph") } + + fn mark_dirty(&mut self, node: NodeIdx); + fn dirty_nodes(&self) -> &BTreeSet; + fn dirty_nodes_mut(&mut self) -> &mut BTreeSet; + fn take_dirty_nodes(&mut self) -> BTreeSet { + std::mem::take(self.dirty_nodes_mut()) + } + /// Add an edge to the graph fn add_edge( &mut self, @@ -63,8 +72,11 @@ pub trait GraphLike { to_node: impl Into, edge: impl Into, ) { - self.graph_mut() - .add_edge(from_node.into(), to_node.into(), edge.into()); + let from = from_node.into(); + let to = to_node.into(); + self.mark_dirty(from); + self.mark_dirty(to); + self.graph_mut().add_edge(from, to, edge.into()); } } diff --git a/crates/solc-expressions/src/assign.rs b/crates/solc-expressions/src/assign.rs index f11f18d9..5c9142e7 100644 --- a/crates/solc-expressions/src/assign.rs +++ b/crates/solc-expressions/src/assign.rs @@ -262,26 +262,71 @@ pub trait Assign: AnalyzerBackend + Sized self.update_array_if_index_access(arena, ctx, loc, lhs_cvar, rhs_cvar)?; // handle struct assignment - if let Ok(fields) = rhs_cvar - .latest_version_or_inherited_in_ctx(ctx, self) - .struct_to_fields(self) - { - if !fields.is_empty() { - fields.into_iter().for_each(|field| { - let mut new_var = field.underlying(self).unwrap().clone(); - let field_name = field.name(self).unwrap(); - let field_name = field_name.split('.').collect::>()[1]; - let new_name = format!("{}.{field_name}", lhs_cvar.name(self).unwrap()); - new_var.name.clone_from(&new_name); - new_var.display_name = new_name; - let new_field = ContextVarNode::from(self.add_node(Node::ContextVar(new_var))); - self.add_edge( - new_field, - lhs_cvar.first_version(self), - Edge::Context(ContextEdge::AttrAccess("field")), - ); - }) - } + if let (Ok(lhs_fields), Ok(rhs_fields)) = ( + lhs_cvar + .latest_version_or_inherited_in_ctx(ctx, self) + .struct_to_fields(self), + rhs_cvar + .latest_version_or_inherited_in_ctx(ctx, self) + .struct_to_fields(self), + ) { + // assert_eq!(lhs_fields.len(), rhs_fields.len()); + lhs_fields.iter().try_for_each(|field| { + let full_name = field.name(self).unwrap(); + let field_name = full_name + .split('.') + .collect::>() + .last() + .cloned() + .unwrap(); + if let Some(matching_field) = rhs_fields.iter().find(|r_field| { + let r_full_name = r_field.name(self).unwrap(); + let r_field_name = r_full_name + .split('.') + .collect::>() + .last() + .cloned() + .unwrap(); + field_name == r_field_name + }) { + let _ = self.assign( + arena, + loc, + field.latest_version_or_inherited_in_ctx(ctx, self), + matching_field.latest_version_or_inherited_in_ctx(ctx, self), + ctx, + )?; + Ok(()) + } else { + Err(ExprErr::ParseError( + loc, + "Missing fields for struct assignment".to_string(), + )) + } + })?; + + // if !fields.is_empty() { + // fields.into_iter().for_each(|field| { + // lhs_cvar.struc + // let mut new_var = field.underlying(self).unwrap().clone(); + // let field_name = field.name(self).unwrap(); + // let field_name = field_name + // .split('.') + // .collect::>() + // .last() + // .cloned() + // .unwrap(); + // let new_name = format!("{}.{field_name}", lhs_cvar.name(self).unwrap()); + // new_var.name.clone_from(&new_name); + // new_var.display_name = new_name; + // let new_field = ContextVarNode::from(self.add_node(Node::ContextVar(new_var))); + // self.add_edge( + // new_field, + // lhs_cvar.first_version(self), + // Edge::Context(ContextEdge::AttrAccess("field")), + // ); + // }) + // } } // advance the rhs variable to avoid recursion issues diff --git a/crates/solc-expressions/src/context_builder/expr.rs b/crates/solc-expressions/src/context_builder/expr.rs index 5f1d7c89..e86e5cdf 100644 --- a/crates/solc-expressions/src/context_builder/expr.rs +++ b/crates/solc-expressions/src/context_builder/expr.rs @@ -57,9 +57,8 @@ pub trait ExpressionParser: .expr_ret_stack .is_empty() { - if let Some(errs) = - self.add_if_err(self.is_representation_ok(arena).into_expr_err(expr.loc())) - { + let res = self.is_representation_ok(arena).into_expr_err(expr.loc()); + if let Some(errs) = self.add_if_err(res) { if !errs.is_empty() { ctx.kill(self, expr.loc(), KilledKind::ParseError).unwrap(); errs.into_iter().for_each(|err| { diff --git a/crates/solc-expressions/src/context_builder/stmt.rs b/crates/solc-expressions/src/context_builder/stmt.rs index 7f1a4321..143e31c2 100644 --- a/crates/solc-expressions/src/context_builder/stmt.rs +++ b/crates/solc-expressions/src/context_builder/stmt.rs @@ -97,9 +97,8 @@ pub trait StatementParser: if let Some(ctx) = parent_ctx { if let Node::Context(_) = self.node(ctx) { let c = ContextNode::from(ctx.into()); - if let Some(errs) = - self.add_if_err(self.is_representation_ok(arena).into_expr_err(stmt.loc())) - { + let res = self.is_representation_ok(arena).into_expr_err(stmt.loc()); + if let Some(errs) = self.add_if_err(res) { if !errs.is_empty() { let Some(is_killed) = self.add_if_err(c.is_killed(self).into_expr_err(stmt.loc())) @@ -142,6 +141,7 @@ pub trait StatementParser: Node::Function(fn_node) => { mods_set = fn_node.modifiers_set; entry_loc = Some(fn_node.loc); + tracing::trace!("creating genesis context for function"); let ctx = Context::new( FunctionNode::from(parent.into()), self.add_if_err( @@ -207,6 +207,13 @@ pub trait StatementParser: Edge::Context(ContextEdge::CalldataVariable), ); + let ty = ContextVarNode::from(cvar_node).ty(self).unwrap(); + if let Some(strukt) = ty.maybe_struct() { + strukt + .add_fields_to_cvar(self, *loc, ContextVarNode::from(cvar_node)) + .unwrap(); + } + Some((param_node, ContextVarNode::from(cvar_node))) } else { None @@ -423,6 +430,7 @@ pub trait StatementParser: "Variable definition had no left hand side".to_string(), )); }; + if matches!(lhs_paths, ExprRet::CtxKilled(_)) { ctx.push_expr(lhs_paths, analyzer).into_expr_err(loc)?; return Ok(()); diff --git a/crates/solc-expressions/src/func_call/apply.rs b/crates/solc-expressions/src/func_call/apply.rs index d7cf18ce..a191e609 100644 --- a/crates/solc-expressions/src/func_call/apply.rs +++ b/crates/solc-expressions/src/func_call/apply.rs @@ -41,8 +41,9 @@ pub trait FuncApplier: seen: &mut Vec, ) -> Result { tracing::trace!( - "Trying to apply function: {}", - func.loc_specified_name(self).into_expr_err(loc)? + "Trying to apply function: {} onto context {}", + func.loc_specified_name(self).into_expr_err(loc)?, + ctx.path(self) ); // ensure no modifiers (for now) // if pure function: @@ -55,8 +56,8 @@ pub trait FuncApplier: FuncVis::Pure => { // pure functions are guaranteed to not require the use of state, so // the only things we care about are function inputs and function outputs - if let Some(body_ctx) = func.maybe_body_ctx(self) { - if body_ctx + if let Some(apply_ctx) = func.maybe_body_ctx(self) { + if apply_ctx .underlying(self) .into_expr_err(loc)? .child @@ -66,21 +67,23 @@ pub trait FuncApplier: "Applying function: {}", func.name(self).into_expr_err(loc)? ); - let edges = body_ctx.successful_edges(self).into_expr_err(loc)?; + let edges = apply_ctx.successful_edges(self).into_expr_err(loc)?; match edges.len() { 0 => {} 1 => { - self.apply_pure( + if !self.apply_pure( arena, loc, func, params, func_inputs, - body_ctx, + apply_ctx, edges[0], ctx, false, - )?; + )? { + ctx.kill(self, loc, KilledKind::Revert).into_expr_err(loc)?; + } return Ok(true); } 2.. => { @@ -99,7 +102,7 @@ pub trait FuncApplier: func, params, func_inputs, - body_ctx, + apply_ctx, edge, *new_fork, true, @@ -122,17 +125,20 @@ pub trait FuncApplier: "Childless pure apply: {}", func.name(self).into_expr_err(loc)? ); - self.apply_pure( + let res = self.apply_pure( arena, loc, func, params, func_inputs, - body_ctx, - body_ctx, + apply_ctx, + apply_ctx, ctx, false, )?; + if !res { + ctx.kill(self, loc, KilledKind::Revert).into_expr_err(loc)?; + } return Ok(true); } } else { @@ -185,6 +191,20 @@ pub trait FuncApplier: } } else { tracing::trace!("View function not processed"); + if ctx.associated_fn(self) == Ok(func) { + return Ok(false); + } + + if seen.contains(&func) { + return Ok(false); + } + + self.handled_funcs_mut().push(func); + if let Some(body) = &func.underlying(self).unwrap().body.clone() { + self.parse_ctx_statement(arena, body, false, Some(func)); + } + + seen.push(func); } } FuncVis::Mut => { @@ -218,6 +238,20 @@ pub trait FuncApplier: } } else { tracing::trace!("Mut function not processed"); + if ctx.associated_fn(self) == Ok(func) { + return Ok(false); + } + + if seen.contains(&func) { + return Ok(false); + } + + self.handled_funcs_mut().push(func); + if let Some(body) = &func.underlying(self).unwrap().body.clone() { + self.parse_ctx_statement(arena, body, false, Some(func)); + } + + seen.push(func); } } } @@ -232,13 +266,14 @@ pub trait FuncApplier: func: FunctionNode, params: &[FunctionParamNode], func_inputs: &[ContextVarNode], - body_ctx: ContextNode, + apply_ctx: ContextNode, resulting_edge: ContextNode, target_ctx: ContextNode, forks: bool, ) -> Result { let replacement_map = - self.basic_inputs_replacement_map(arena, body_ctx, loc, params, func_inputs)?; + self.basic_inputs_replacement_map(arena, apply_ctx, loc, params, func_inputs)?; + tracing::trace!("applying pure function - replacement map: {replacement_map:#?}"); let mut rets: Vec<_> = resulting_edge .return_nodes(self) .into_expr_err(loc)? @@ -400,7 +435,16 @@ pub trait FuncApplier: ); let underlying_mut = target_ctx.underlying_mut(self).into_expr_err(loc)?; underlying_mut.path = new_path; - underlying_mut.applies.push(func); + + target_ctx + .propogate_applied(func, self) + .into_expr_err(loc)?; + if let Some(body) = func.maybe_body_ctx(self) { + for app in body.underlying(self).into_expr_err(loc)?.applies.clone() { + target_ctx.propogate_applied(app, self).into_expr_err(loc)?; + } + } + target_ctx .push_expr(ExprRet::Multi(rets), self) .into_expr_err(loc)?; @@ -411,12 +455,12 @@ pub trait FuncApplier: fn basic_inputs_replacement_map( &mut self, arena: &mut RangeArena>, - ctx: ContextNode, + apply_ctx: ContextNode, loc: Loc, params: &[FunctionParamNode], func_inputs: &[ContextVarNode], ) -> Result, ContextVarNode)>, ExprErr> { - let inputs = ctx.input_variables(self); + let inputs = apply_ctx.input_variables(self); let mut replacement_map: BTreeMap, ContextVarNode)> = BTreeMap::default(); params @@ -425,13 +469,11 @@ pub trait FuncApplier: .try_for_each(|(param, func_input)| { if let Some(name) = param.maybe_name(self).into_expr_err(loc)? { let mut new_cvar = func_input - .latest_version_or_inherited_in_ctx(ctx, self) + .latest_version_or_inherited_in_ctx(apply_ctx, self) .underlying(self) .into_expr_err(loc)? .clone(); new_cvar.loc = Some(param.loc(self).unwrap()); - // new_cvar.name = name.clone(); - // new_cvar.display_name = name.clone(); new_cvar.is_tmp = false; new_cvar.storage = if let Some(StorageLocation::Storage(_)) = param.underlying(self).unwrap().storage @@ -444,12 +486,6 @@ pub trait FuncApplier: let replacement = ContextVarNode::from(self.add_node(Node::ContextVar(new_cvar))); - self.add_edge( - replacement, - *func_input, - Edge::Context(ContextEdge::InputVariable), - ); - if let Some(param_ty) = VarType::try_from_idx(self, param.ty(self).unwrap()) { if !replacement.ty_eq_ty(¶m_ty, self).into_expr_err(loc)? { replacement @@ -460,7 +496,7 @@ pub trait FuncApplier: if let Some(_len_var) = replacement.array_to_len_var(self) { // bring the length variable along as well - self.get_length(arena, ctx, loc, *func_input, false) + self.get_length(arena, apply_ctx, loc, *func_input, false) .unwrap(); } @@ -470,22 +506,19 @@ pub trait FuncApplier: let new_min = r.range_min().into_owned().cast(r2.range_min().into_owned()); let new_max = r.range_max().into_owned().cast(r2.range_max().into_owned()); replacement - .latest_version_or_inherited_in_ctx(ctx, self) + .latest_version_or_inherited_in_ctx(apply_ctx, self) .try_set_range_min(self, arena, new_min) .into_expr_err(loc)?; replacement - .latest_version_or_inherited_in_ctx(ctx, self) + .latest_version_or_inherited_in_ctx(apply_ctx, self) .try_set_range_max(self, arena, new_max) .into_expr_err(loc)?; replacement - .latest_version_or_inherited_in_ctx(ctx, self) + .latest_version_or_inherited_in_ctx(apply_ctx, self) .try_set_range_exclusions(self, r.exclusions) .into_expr_err(loc)?; } - ctx.add_var(replacement, self).unwrap(); - self.add_edge(replacement, ctx, Edge::Context(ContextEdge::Variable)); - let Some(correct_input) = inputs .iter() .find(|input| input.name(self).unwrap() == name) @@ -496,37 +529,59 @@ pub trait FuncApplier: )); }; - if let Ok(fields) = correct_input.struct_to_fields(self) { - if !fields.is_empty() { - let replacement_fields = func_input.struct_to_fields(self).unwrap(); - fields.iter().for_each(|field| { - let field_name = field.name(self).unwrap(); - let to_replace_field_name = - field_name.split('.').collect::>()[1]; - if let Some(replacement_field) = - replacement_fields.iter().find(|replacement_field| { - let name = replacement_field.name(self).unwrap(); - let replacement_name = - name.split('.').collect::>()[1]; - to_replace_field_name == replacement_name - }) - { - let mut replacement_field_as_elem = - Elem::from(*replacement_field); - replacement_field_as_elem.arenaize(self, arena).unwrap(); - if let Some(next) = field.next_version(self) { - replacement_map.insert( - next.0.into(), - (replacement_field_as_elem.clone(), *replacement_field), - ); - } - replacement_map.insert( - field.0.into(), - (replacement_field_as_elem, *replacement_field), - ); - } - }); - } + let target_fields = correct_input.struct_to_fields(self).into_expr_err(loc)?; + let replacement_fields = + func_input.struct_to_fields(self).into_expr_err(loc)?; + let match_field = + |this: &Self, + target_field: ContextVarNode, + replacement_fields: &[ContextVarNode]| + -> Option<(ContextVarNode, ContextVarNode)> { + let target_full_name = target_field.name(this).clone().unwrap(); + let target_field_name = target_full_name + .split('.') + .collect::>() + .last() + .cloned() + .unwrap(); + let replacement_field = + replacement_fields.iter().find(|rep_field| { + let replacement_full_name = rep_field.name(this).unwrap(); + let replacement_field_name = replacement_full_name + .split('.') + .collect::>() + .last() + .cloned() + .unwrap(); + replacement_field_name == target_field_name + })?; + Some((target_field, *replacement_field)) + }; + + let mut struct_stack = target_fields + .into_iter() + .filter_map(|i| match_field(self, i, &replacement_fields[..])) + .collect::>(); + + while let Some((target_field, replacement_field)) = struct_stack.pop() { + let mut replacement_field_as_elem = Elem::from(replacement_field); + replacement_field_as_elem.arenaize(self, arena).unwrap(); + let to_replace = target_field.next_version(self).unwrap_or(target_field); + replacement_map.insert( + to_replace.0.into(), + (replacement_field_as_elem.clone(), replacement_field), + ); + + let target_sub_fields = + target_field.struct_to_fields(self).into_expr_err(loc)?; + let replacement_sub_fields = replacement_field + .struct_to_fields(self) + .into_expr_err(loc)?; + let subs = target_sub_fields + .into_iter() + .filter_map(|i| match_field(self, i, &replacement_sub_fields[..])) + .collect::>(); + struct_stack.extend(subs); } let mut replacement_as_elem = Elem::from(replacement); diff --git a/crates/solc-expressions/src/func_call/helper.rs b/crates/solc-expressions/src/func_call/helper.rs index aa25d418..ea6bc3f0 100644 --- a/crates/solc-expressions/src/func_call/helper.rs +++ b/crates/solc-expressions/src/func_call/helper.rs @@ -617,7 +617,7 @@ pub trait CallerHelper: AnalyzerBackend + inheritor_ctx, loc, arena, - &|analyzer, arena, inheritor_ctx, loc| { + &|analyzer, _arena, inheritor_ctx, loc| { let vars = grantor_ctx.local_vars(analyzer).clone(); vars.iter().try_for_each(|(name, old_var)| { let var = old_var.latest_version(analyzer); @@ -625,27 +625,13 @@ pub trait CallerHelper: AnalyzerBackend + if var.is_storage(analyzer).into_expr_err(loc)? { if let Some(inheritor_var) = inheritor_ctx.var_by_name(analyzer, name) { let inheritor_var = inheritor_var.latest_version(analyzer); - if let Some(r) = underlying.ty.range(analyzer).into_expr_err(loc)? { - let new_inheritor_var = analyzer - .advance_var_in_ctx( - inheritor_var, - underlying.loc.expect("No loc for val change"), - inheritor_ctx, - ) - .unwrap(); - let _ = new_inheritor_var.set_range_min( - analyzer, - arena, - r.range_min().into_owned(), - ); - let _ = new_inheritor_var.set_range_max( - analyzer, - arena, - r.range_max().into_owned(), - ); - let _ = new_inheritor_var - .set_range_exclusions(analyzer, r.exclusions.clone()); - } + analyzer + .advance_var_in_ctx( + inheritor_var, + underlying.loc.expect("No loc for val change"), + inheritor_ctx, + ) + .unwrap(); } else { let new_in_inheritor = analyzer.add_node(Node::ContextVar(underlying.clone())); @@ -662,6 +648,33 @@ pub trait CallerHelper: AnalyzerBackend + var, Edge::Context(ContextEdge::InheritedVariable), ); + let from_fields = + var.struct_to_fields(analyzer).into_expr_err(loc)?; + let mut struct_stack = from_fields + .into_iter() + .map(|i| (i, new_in_inheritor)) + .collect::>(); + while !struct_stack.is_empty() { + let (field, parent) = struct_stack.pop().unwrap(); + let underlying = + field.underlying(analyzer).into_expr_err(loc)?; + let new_field_in_inheritor = + analyzer.add_node(Node::ContextVar(underlying.clone())); + analyzer.add_edge( + new_field_in_inheritor, + parent, + Edge::Context(ContextEdge::AttrAccess("field")), + ); + + let sub_fields = + field.struct_to_fields(analyzer).into_expr_err(loc)?; + struct_stack.extend( + sub_fields + .into_iter() + .map(|i| (i, new_field_in_inheritor)) + .collect::>(), + ); + } } } Ok(()) diff --git a/crates/solc-expressions/src/member_access/struct_access.rs b/crates/solc-expressions/src/member_access/struct_access.rs index b8833f0f..f0187d01 100644 --- a/crates/solc-expressions/src/member_access/struct_access.rs +++ b/crates/solc-expressions/src/member_access/struct_access.rs @@ -44,6 +44,7 @@ pub trait StructAccess: field.latest_version_or_inherited_in_ctx(ctx, self).into(), )) } else if let Some(field) = struct_node.find_field(self, ident) { + println!("here1234"); let cvar = if let Some(parent) = maybe_parent { parent } else { diff --git a/crates/solc-expressions/src/require.rs b/crates/solc-expressions/src/require.rs index 6a953ab8..f8117774 100644 --- a/crates/solc-expressions/src/require.rs +++ b/crates/solc-expressions/src/require.rs @@ -1057,7 +1057,7 @@ pub trait Require: AnalyzerBackend + Variable + BinOp + Sized { } RangeOp::Neq => { // check if contains - let elem = Elem::from(const_var.latest_version_or_inherited_in_ctx(ctx, self)); + let mut elem = Elem::from(const_var.latest_version_or_inherited_in_ctx(ctx, self)); // potentially add the const var as a range exclusion if let Some(Ordering::Equal) = nonconst_range @@ -1097,8 +1097,8 @@ pub trait Require: AnalyzerBackend + Variable + BinOp + Sized { .into_expr_err(loc)?; } else { // just add as an exclusion - let idx = arena.idx_or_upsert(elem, self); - nonconst_range.add_range_exclusion(idx); + elem.arenaize(self, arena).into_expr_err(loc)?; + nonconst_range.add_range_exclusion(elem); nonconst_var .set_range_exclusions(self, nonconst_range.exclusions) .into_expr_err(loc)?; @@ -1304,18 +1304,20 @@ pub trait Require: AnalyzerBackend + Variable + BinOp + Sized { return Ok(true); } - let rhs_elem = Elem::from(new_rhs.latest_version_or_inherited_in_ctx(ctx, self)); + let mut rhs_elem = + Elem::from(new_rhs.latest_version_or_inherited_in_ctx(ctx, self)); // just add as an exclusion - let idx = arena.idx_or_upsert(rhs_elem, self); - lhs_range.add_range_exclusion(idx); + rhs_elem.arenaize(self, arena).into_expr_err(loc)?; + lhs_range.add_range_exclusion(rhs_elem); new_lhs .set_range_exclusions(self, lhs_range.exclusions) .into_expr_err(loc)?; - let lhs_elem = Elem::from(new_lhs.latest_version_or_inherited_in_ctx(ctx, self)); + let mut lhs_elem = + Elem::from(new_lhs.latest_version_or_inherited_in_ctx(ctx, self)); // just add as an exclusion - let idx = arena.idx_or_upsert(lhs_elem, self); - rhs_range.add_range_exclusion(idx); + lhs_elem.arenaize(self, arena).into_expr_err(loc)?; + rhs_range.add_range_exclusion(lhs_elem); new_rhs .set_range_exclusions(self, rhs_range.exclusions) .into_expr_err(loc)?; diff --git a/crates/solc-expressions/src/variable.rs b/crates/solc-expressions/src/variable.rs index 31e8e9f7..618574ee 100644 --- a/crates/solc-expressions/src/variable.rs +++ b/crates/solc-expressions/src/variable.rs @@ -357,6 +357,12 @@ pub trait Variable: AnalyzerBackend + Size let lhs = ContextVarNode::from(self.add_node(Node::ContextVar(var))); ctx.add_var(lhs, self).into_expr_err(loc)?; self.add_edge(lhs, ctx, Edge::Context(ContextEdge::Variable)); + + if let Some(strukt) = lhs.ty(self).into_expr_err(loc)?.maybe_struct() { + strukt + .add_fields_to_cvar(self, loc, lhs) + .into_expr_err(loc)?; + } let rhs = ContextVarNode::from(*rhs); self.apply_to_edges(ctx, loc, arena, &|analyzer, arena, ctx, loc| { @@ -555,6 +561,31 @@ pub trait Variable: AnalyzerBackend + Size cvar_node.0, Edge::Context(ContextEdge::InheritedVariable), ); + + let from_fields = cvar_node.struct_to_fields(self).into_expr_err(loc)?; + let mut struct_stack = from_fields + .into_iter() + .map(|i| (i, new_cvarnode)) + .collect::>(); + while !struct_stack.is_empty() { + let (field, parent) = struct_stack.pop().unwrap(); + let underlying = field.underlying(self).into_expr_err(loc)?; + let new_field_in_inheritor = + self.add_node(Node::ContextVar(underlying.clone())); + self.add_edge( + new_field_in_inheritor, + parent, + Edge::Context(ContextEdge::AttrAccess("field")), + ); + + let sub_fields = field.struct_to_fields(self).into_expr_err(loc)?; + struct_stack.extend( + sub_fields + .into_iter() + .map(|i| (i, new_field_in_inheritor)) + .collect::>(), + ); + } } else { self.add_edge(new_cvarnode, cvar_node.0, Edge::Context(ContextEdge::Prev)); } @@ -565,6 +596,8 @@ pub trait Variable: AnalyzerBackend + Size } } + self.mark_dirty(new_cvarnode); + Ok(ContextVarNode::from(new_cvarnode)) } @@ -626,6 +659,31 @@ pub trait Variable: AnalyzerBackend + Size cvar_node.0, Edge::Context(ContextEdge::InheritedVariable), ); + + let from_fields = cvar_node.struct_to_fields(self).into_expr_err(loc)?; + let mut struct_stack = from_fields + .into_iter() + .map(|i| (i, new_cvarnode)) + .collect::>(); + while !struct_stack.is_empty() { + let (field, parent) = struct_stack.pop().unwrap(); + let underlying = field.underlying(self).into_expr_err(loc)?; + let new_field_in_inheritor = + self.add_node(Node::ContextVar(underlying.clone())); + self.add_edge( + new_field_in_inheritor, + parent, + Edge::Context(ContextEdge::AttrAccess("field")), + ); + + let sub_fields = field.struct_to_fields(self).into_expr_err(loc)?; + struct_stack.extend( + sub_fields + .into_iter() + .map(|i| (i, new_field_in_inheritor)) + .collect::>(), + ); + } } else { self.add_edge(new_cvarnode, cvar_node.0, Edge::Context(ContextEdge::Prev)); } @@ -636,6 +694,8 @@ pub trait Variable: AnalyzerBackend + Size } } + self.mark_dirty(new_cvarnode); + Ok(ContextVarNode::from(new_cvarnode)) } diff --git a/mini.sol b/mini.sol index 2d301fb6..048670c1 100644 --- a/mini.sol +++ b/mini.sol @@ -1,14 +1,36 @@ +contract Comp { + struct Checkpoint { + uint32 fromBlock; + uint96 votes; + } -contract StruktTester { - struct A { - uint256 b; - uint256 c; - } + mapping(address => mapping(uint32 => Checkpoint)) public checkpoints; + mapping(address => uint32) public numCheckpoints; - function constructStruct() public { - A memory b = A({b: 100, c: 100}); - require(b.b == 100); - require(b.c == 100); - } + function _moveDelegates( + address srcRep, + address dstRep, + uint96 amount + ) internal { + if (amount > 0) { + uint32 srcRepNum = numCheckpoints[srcRep]; + uint96 srcRepOld = srcRepNum > 0 + ? checkpoints[srcRep][srcRepNum - 1].votes + : 0; + uint96 srcRepNew = sub96( + srcRepOld, + amount, + "Comp::_moveVotes: vote amount underflows" + ); + } + } -} \ No newline at end of file + function sub96( + uint96 a, + uint96 b, + string memory errorMessage + ) internal pure returns (uint96) { + require(b <= a, errorMessage); + return a - b; + } +} From e31cd0bbf3650157bc3434d849e773bcf479ab8b Mon Sep 17 00:00:00 2001 From: brock elmore Date: Thu, 11 Jul 2024 17:03:38 -0700 Subject: [PATCH 10/10] lint --- crates/graph/src/nodes/context/variables.rs | 2 +- crates/graph/src/solvers/atoms.rs | 12 ++++++------ crates/solc-expressions/src/assign.rs | 2 +- crates/solc-expressions/src/func_call/helper.rs | 3 +-- crates/solc-expressions/src/variable.rs | 6 ++---- 5 files changed, 11 insertions(+), 14 deletions(-) diff --git a/crates/graph/src/nodes/context/variables.rs b/crates/graph/src/nodes/context/variables.rs index a3573c69..e9ba4098 100644 --- a/crates/graph/src/nodes/context/variables.rs +++ b/crates/graph/src/nodes/context/variables.rs @@ -470,7 +470,7 @@ impl ContextNode { fields.iter().try_for_each(|field| { self.recursive_move_struct_field(new_cvarnode, *field, loc, analyzer) })?; - Ok(new_cvarnode.into()) + Ok(new_cvarnode) } else { Ok(var) } diff --git a/crates/graph/src/solvers/atoms.rs b/crates/graph/src/solvers/atoms.rs index 50d0e02d..c414f885 100644 --- a/crates/graph/src/solvers/atoms.rs +++ b/crates/graph/src/solvers/atoms.rs @@ -302,7 +302,7 @@ impl Atomize for Elem { match self { Elem::Arena(_) => { self.dearenaize_clone(arena) - .atoms_or_part(Some(&self), analyzer, arena) + .atoms_or_part(Some(self), analyzer, arena) } Elem::Concrete(_) | Elem::Reference(_) => AtomOrPart::Part(self.clone()), Elem::ConcreteDyn(_) => AtomOrPart::Part(self.clone()), @@ -311,17 +311,17 @@ impl Atomize for Elem { match collapse(*expr.lhs.clone(), expr.op, *expr.rhs.clone(), arena) { MaybeCollapsed::Concretes(_l, _r) => { let exec_res = expr.exec_op(true, analyzer, arena).unwrap(); - return exec_res.atoms_or_part(Some(&self), analyzer, arena); + return exec_res.atoms_or_part(Some(self), analyzer, arena); } MaybeCollapsed::Collapsed(elem) => { - return elem.atoms_or_part(Some(&self), analyzer, arena); + return elem.atoms_or_part(Some(self), analyzer, arena); } MaybeCollapsed::Not(..) => {} } match ( - expr.lhs.atoms_or_part(Some(&self), analyzer, arena), - expr.rhs.atoms_or_part(Some(&self), analyzer, arena), + expr.lhs.atoms_or_part(Some(self), analyzer, arena), + expr.rhs.atoms_or_part(Some(self), analyzer, arena), ) { (ref lp @ AtomOrPart::Part(ref l), ref rp @ AtomOrPart::Part(ref r)) => { // println!("part part"); @@ -373,7 +373,7 @@ impl Atomize for Elem { if res == Elem::Expr(expr.clone()) { AtomOrPart::Part(res) } else { - res.atoms_or_part(Some(&self), analyzer, arena) + res.atoms_or_part(Some(self), analyzer, arena) } } (Elem::ConcreteDyn(_), _) => AtomOrPart::Part(Elem::Null), diff --git a/crates/solc-expressions/src/assign.rs b/crates/solc-expressions/src/assign.rs index 5c9142e7..bf6787fc 100644 --- a/crates/solc-expressions/src/assign.rs +++ b/crates/solc-expressions/src/assign.rs @@ -3,7 +3,7 @@ use crate::{array::Array, variable::Variable, ContextBuilder, ExpressionParser, use graph::{ elem::{Elem, RangeElem}, nodes::{Concrete, ContextNode, ContextVarNode, ExprRet}, - AnalyzerBackend, ContextEdge, Edge, Node, + AnalyzerBackend, ContextEdge, Edge, }; use shared::{ExprErr, GraphError, IntoExprErr, RangeArena}; diff --git a/crates/solc-expressions/src/func_call/helper.rs b/crates/solc-expressions/src/func_call/helper.rs index ea6bc3f0..c3d4b554 100644 --- a/crates/solc-expressions/src/func_call/helper.rs +++ b/crates/solc-expressions/src/func_call/helper.rs @@ -654,8 +654,7 @@ pub trait CallerHelper: AnalyzerBackend + .into_iter() .map(|i| (i, new_in_inheritor)) .collect::>(); - while !struct_stack.is_empty() { - let (field, parent) = struct_stack.pop().unwrap(); + while let Some((field, parent)) = struct_stack.pop() { let underlying = field.underlying(analyzer).into_expr_err(loc)?; let new_field_in_inheritor = diff --git a/crates/solc-expressions/src/variable.rs b/crates/solc-expressions/src/variable.rs index 618574ee..d27a5e1f 100644 --- a/crates/solc-expressions/src/variable.rs +++ b/crates/solc-expressions/src/variable.rs @@ -567,8 +567,7 @@ pub trait Variable: AnalyzerBackend + Size .into_iter() .map(|i| (i, new_cvarnode)) .collect::>(); - while !struct_stack.is_empty() { - let (field, parent) = struct_stack.pop().unwrap(); + while let Some((field, parent)) = struct_stack.pop() { let underlying = field.underlying(self).into_expr_err(loc)?; let new_field_in_inheritor = self.add_node(Node::ContextVar(underlying.clone())); @@ -665,8 +664,7 @@ pub trait Variable: AnalyzerBackend + Size .into_iter() .map(|i| (i, new_cvarnode)) .collect::>(); - while !struct_stack.is_empty() { - let (field, parent) = struct_stack.pop().unwrap(); + while let Some((field, parent)) = struct_stack.pop() { let underlying = field.underlying(self).into_expr_err(loc)?; let new_field_in_inheritor = self.add_node(Node::ContextVar(underlying.clone()));