From 38c69f789caa9116c3678f0144ab6d40ff580ee0 Mon Sep 17 00:00:00 2001 From: brock elmore Date: Mon, 22 Jul 2024 13:27:50 -0700 Subject: [PATCH] fixes --- crates/graph/src/nodes/context/variables.rs | 38 +++++++------- crates/graph/src/range/elem/expr/collapse.rs | 1 - crates/pyrometer/tests/test_data/variable.sol | 16 ++++++ crates/solc-expressions/src/assign.rs | 50 +++++++++++++++++++ .../src/context_builder/flattened.rs | 48 ++++++++++-------- .../src/context_builder/mod.rs | 6 +-- .../src/func_call/intrinsic_call/address.rs | 2 +- .../func_call/intrinsic_call/constructors.rs | 2 +- .../func_call/intrinsic_call/dyn_builtin.rs | 3 +- crates/solc-expressions/src/variable.rs | 18 +++++-- crates/solc-expressions/src/yul/yul_funcs.rs | 10 ++-- 11 files changed, 136 insertions(+), 58 deletions(-) diff --git a/crates/graph/src/nodes/context/variables.rs b/crates/graph/src/nodes/context/variables.rs index c34c054c..afa6ac9b 100644 --- a/crates/graph/src/nodes/context/variables.rs +++ b/crates/graph/src/nodes/context/variables.rs @@ -433,25 +433,25 @@ impl ContextNode { } } - /// Pop the latest expression return off the stack - #[tracing::instrument(level = "trace", skip_all)] - pub fn pop_expr_latest( - &self, - loc: Loc, - analyzer: &mut impl AnalyzerBackend, - ) -> Result, GraphError> { - let underlying_mut = self.underlying_mut(analyzer)?; - if let Some(elem) = underlying_mut.expr_ret_stack.pop() { - tracing::trace!( - "popping var {} from: {}", - elem.debug_str(analyzer), - self.path(analyzer) - ); - Ok(Some(self.maybe_move_expr(elem, loc, analyzer)?)) - } else { - Ok(None) - } - } + // /// Pop the latest expression return off the stack + // #[tracing::instrument(level = "trace", skip_all)] + // pub fn pop_expr_latest( + // &self, + // loc: Loc, + // analyzer: &mut impl AnalyzerBackend, + // ) -> Result, GraphError> { + // let underlying_mut = self.underlying_mut(analyzer)?; + // if let Some(elem) = underlying_mut.expr_ret_stack.pop() { + // tracing::trace!( + // "popping var {} from: {}", + // elem.debug_str(analyzer), + // self.path(analyzer) + // ); + // Ok(Some(self.maybe_move_expr(elem, loc, analyzer)?)) + // } else { + // Ok(None) + // } + // } pub fn pop_n_latest_exprs( &self, diff --git a/crates/graph/src/range/elem/expr/collapse.rs b/crates/graph/src/range/elem/expr/collapse.rs index 2f3730d7..c3195cc7 100644 --- a/crates/graph/src/range/elem/expr/collapse.rs +++ b/crates/graph/src/range/elem/expr/collapse.rs @@ -522,7 +522,6 @@ pub fn collapse( (_, RangeOp::Eq) => { // (x _ y) == z ==> (x _ y if z == true) if z.range_eq(&Elem::from(Concrete::from(true)), arena) { - println!("true collapse"); MaybeCollapsed::Collapsed(Elem::Expr(expr)) } else if z.range_eq(&Elem::from(Concrete::from(false)), arena) { // (!x && !y) diff --git a/crates/pyrometer/tests/test_data/variable.sol b/crates/pyrometer/tests/test_data/variable.sol index c70dc46e..d24980cb 100644 --- a/crates/pyrometer/tests/test_data/variable.sol +++ b/crates/pyrometer/tests/test_data/variable.sol @@ -23,6 +23,22 @@ contract Variable { } } +contract B { + struct A { + address a; + } +} + +contract A is B { + A a; // contract A + + function return_struct() external { + // a is of type B.A, *not* Contract::A + a = A(address(this)); + // return a; + } +} + contract C { C c; diff --git a/crates/solc-expressions/src/assign.rs b/crates/solc-expressions/src/assign.rs index 98e07452..f3537300 100644 --- a/crates/solc-expressions/src/assign.rs +++ b/crates/solc-expressions/src/assign.rs @@ -27,6 +27,7 @@ pub trait Assign: AnalyzerBackend + Sized ctx.kill(self, loc, *kind).into_expr_err(loc)?; Ok(()) } + (ExprRet::Single(lhs), ExprRet::SingleLiteral(rhs)) => { // ie: uint x = 5; let lhs_cvar = @@ -97,6 +98,55 @@ pub trait Assign: AnalyzerBackend + Sized lhs_cvar.display_name(self).unwrap(), ); + println!( + "{:?} = {:?}", + lhs_cvar.ty(self).unwrap(), + rhs_cvar.ty(self).unwrap() + ); + + if lhs_cvar.is_struct(self).into_expr_err(loc)? + && rhs_cvar.is_struct(self).into_expr_err(loc)? + { + let lhs_fields = lhs_cvar.struct_to_fields(self).into_expr_err(loc)?; + let rhs_fields = rhs_cvar.struct_to_fields(self).into_expr_err(loc)?; + lhs_fields.iter().try_for_each(|lhs_field| { + let lhs_full_name = lhs_field.name(self).into_expr_err(loc)?; + let split = lhs_full_name.split('.').collect::>(); + let Some(lhs_field_name) = split.last() else { + return Err(ExprErr::ParseError( + lhs_field.loc(self).unwrap(), + format!("Incorrectly named field: {lhs_full_name} - no '.' delimiter"), + )); + }; + + let mut found = false; + for rhs_field in rhs_fields.iter() { + let rhs_full_name = rhs_field.name(self).into_expr_err(loc)?; + let split = rhs_full_name.split('.').collect::>(); + let Some(rhs_field_name) = split.last() else { + return Err(ExprErr::ParseError( + rhs_field.loc(self).unwrap(), + format!("Incorrectly named field: {rhs_full_name} - no '.' delimiter"), + )); + }; + if lhs_field_name == rhs_field_name { + found = true; + let _ = self.assign(arena, loc, *lhs_field, *rhs_field, ctx)?; + break; + } + } + if found { + Ok(()) + } else { + Err(ExprErr::ParseError( + loc, + format!("Struct types mismatched - could not find field: {lhs_field_name}"), + )) + } + })?; + return Ok(ExprRet::Single(lhs_cvar.0.into())); + } + rhs_cvar .cast_from(&lhs_cvar, self, arena) .into_expr_err(loc)?; diff --git a/crates/solc-expressions/src/context_builder/flattened.rs b/crates/solc-expressions/src/context_builder/flattened.rs index 4ef603f7..3347ecb9 100644 --- a/crates/solc-expressions/src/context_builder/flattened.rs +++ b/crates/solc-expressions/src/context_builder/flattened.rs @@ -670,6 +670,12 @@ pub trait Flatten: self.push_expr(parent); } + Assign(_, lhs, rhs) => { + self.traverse_expression(lhs, unchecked); + self.traverse_expression(rhs, unchecked); + self.push_expr(FlatExpr::try_from(parent_expr).unwrap()); + } + Modulo(_, lhs, rhs) | AssignModulo(_, lhs, rhs) | ShiftLeft(_, lhs, rhs) @@ -682,7 +688,6 @@ pub trait Flatten: | AssignXor(_, lhs, rhs) | BitwiseOr(_, lhs, rhs) | AssignOr(_, lhs, rhs) - | Assign(_, lhs, rhs) | Equal(_, lhs, rhs) | NotEqual(_, lhs, rhs) | Less(_, lhs, rhs) @@ -1217,12 +1222,14 @@ pub trait Flatten: unreachable!() }; - let var = ContextVar::new_from_contract( + let mut var = ContextVar::new_from_contract( loc, ctx.associated_contract(self).into_expr_err(loc)?, self, ) .into_expr_err(loc)?; + var.name = "this".to_string(); + var.display_name = "this".to_string(); let cvar = self.add_node(Node::ContextVar(var)); ctx.add_var(cvar.into(), self).into_expr_err(loc)?; self.add_edge(cvar, ctx, Edge::Context(ContextEdge::Variable)); @@ -1962,7 +1969,7 @@ pub trait Flatten: }; let res = ctx.pop_n_latest_exprs(2, loc, self).into_expr_err(loc)?; - let [lhs, rhs] = into_sized(res); + let [rhs, lhs] = into_sized(res); self.match_assign_sides(arena, ctx, loc, &lhs, &rhs) } @@ -2238,26 +2245,23 @@ pub trait Flatten: None, )?; - if let Some(ret) = ctx.pop_expr_latest(loc, self).into_expr_err(loc)? { - if ContextVarNode::from(ret.expect_single().into_expr_err(loc)?) - .is_memory(self) - .into_expr_err(loc)? - { - // its a memory based variable, push a uint instead - let b = Builtin::Uint(256); - let var = ContextVar::new_from_builtin(loc, self.builtin_or_add(b).into(), self) - .into_expr_err(loc)?; - let node = self.add_node(var); - ctx.push_expr(ExprRet::Single(node), self) - .into_expr_err(loc) - } else { - ctx.push_expr(ret, self).into_expr_err(loc) - } + let ret = ctx + .pop_n_latest_exprs(1, loc, self) + .into_expr_err(loc)? + .swap_remove(0); + if ContextVarNode::from(ret.expect_single().into_expr_err(loc)?) + .is_memory(self) + .into_expr_err(loc)? + { + // its a memory based variable, push a uint instead + let b = Builtin::Uint(256); + let var = ContextVar::new_from_builtin(loc, self.builtin_or_add(b).into(), self) + .into_expr_err(loc)?; + let node = self.add_node(var); + ctx.push_expr(ExprRet::Single(node), self) + .into_expr_err(loc) } else { - Err(ExprErr::Unresolved( - loc, - format!("Could not find yul variable with name: {name}"), - )) + ctx.push_expr(ret, self).into_expr_err(loc) } } diff --git a/crates/solc-expressions/src/context_builder/mod.rs b/crates/solc-expressions/src/context_builder/mod.rs index fd4fa9af..5912b47d 100644 --- a/crates/solc-expressions/src/context_builder/mod.rs +++ b/crates/solc-expressions/src/context_builder/mod.rs @@ -89,10 +89,10 @@ pub trait ContextBuilder: AnalyzerBackend Ok(Some(target_var)) => { // perform a cast tracing::trace!( - "{}: casting {:?} to {:?}", + "casting {} to {} in {}", + latest.ty(self).unwrap().as_string(self).unwrap(), + target_var.ty(self).unwrap().as_string(self).unwrap(), ctx.path(self), - latest.ty(self).unwrap(), - target_var.ty(self).unwrap(), ); let next = self .advance_var_in_ctx_forcible(latest, loc, ctx, true) diff --git a/crates/solc-expressions/src/func_call/intrinsic_call/address.rs b/crates/solc-expressions/src/func_call/intrinsic_call/address.rs index cc656d23..7c4492d3 100644 --- a/crates/solc-expressions/src/func_call/intrinsic_call/address.rs +++ b/crates/solc-expressions/src/func_call/intrinsic_call/address.rs @@ -38,10 +38,10 @@ pub trait AddressCaller: AnalyzerBackend + } } + #[tracing::instrument(level = "trace", skip_all)] fn external_call(&mut self, ctx: ContextNode, _ty: &str, loc: Loc) -> Result<(), ExprErr> { // TODO: Check if we have the code for the address // if we dont, model it as a unrestricted call that can make other calls - ctx.pop_expr_latest(loc, self).into_expr_err(loc)?; // TODO: try to be smarter based on the address input let booln = self.builtin_or_add(Builtin::Bool); let bool_cvar = ContextVar::new_from_builtin(loc, booln.into(), self).into_expr_err(loc)?; diff --git a/crates/solc-expressions/src/func_call/intrinsic_call/constructors.rs b/crates/solc-expressions/src/func_call/intrinsic_call/constructors.rs index 16a21e39..0f115bd6 100644 --- a/crates/solc-expressions/src/func_call/intrinsic_call/constructors.rs +++ b/crates/solc-expressions/src/func_call/intrinsic_call/constructors.rs @@ -162,7 +162,7 @@ pub trait ConstructorCaller: ctx.add_var(fc_node.into(), self).into_expr_err(loc)?; let field_as_ret = ExprRet::Single(fc_node); self.match_assign_sides(arena, ctx, loc, &field_as_ret, &input)?; - let _ = ctx.pop_expr_latest(loc, self).into_expr_err(loc)?; + let _ = ctx.pop_n_latest_exprs(1, loc, self).into_expr_err(loc)?; Ok(()) })?; diff --git a/crates/solc-expressions/src/func_call/intrinsic_call/dyn_builtin.rs b/crates/solc-expressions/src/func_call/intrinsic_call/dyn_builtin.rs index f7f09049..1606eb63 100644 --- a/crates/solc-expressions/src/func_call/intrinsic_call/dyn_builtin.rs +++ b/crates/solc-expressions/src/func_call/intrinsic_call/dyn_builtin.rs @@ -74,8 +74,9 @@ pub trait DynBuiltinCaller: AnalyzerBackend { // pop the accumulation node off the stack let accum_node = ctx - .pop_expr_latest(loc, self) + .pop_n_latest_exprs(1, loc, self) .into_expr_err(loc)? + .get(0) .unwrap() .expect_single() .unwrap(); diff --git a/crates/solc-expressions/src/variable.rs b/crates/solc-expressions/src/variable.rs index 7a5dd171..f1aff890 100644 --- a/crates/solc-expressions/src/variable.rs +++ b/crates/solc-expressions/src/variable.rs @@ -76,10 +76,12 @@ pub trait Variable: AnalyzerBackend + Size } else if (self.env_variable(arena, ident, target_ctx)?).is_some() { Ok(()) } else if let Some(idxs) = self.user_types().get(&ident.name).cloned() { + tracing::trace!("Getting variable via user_types"); let idx = if idxs.len() == 1 { idxs[0] } else { // disambiguate by scope + tracing::trace!("disambiguating by scope"); let in_scope = if let Some(contract) = ctx .maybe_associated_contract(self) .into_expr_err(ident.loc)? @@ -159,6 +161,15 @@ pub trait Variable: AnalyzerBackend + Size ); } + if let Some(strukt) = ContextVarNode::from(new_cvarnode) + .maybe_struct(self) + .into_expr_err(ident.loc)? + { + strukt + .add_fields_to_cvar(self, ident.loc, ContextVarNode::from(new_cvarnode)) + .into_expr_err(ident.loc)?; + } + target_ctx .push_expr(ExprRet::Single(new_cvarnode), self) .into_expr_err(ident.loc)?; @@ -198,16 +209,15 @@ pub trait Variable: AnalyzerBackend + Size location: Option, ) -> Option { // disambiguate based on left hand side if it exists - if let Some(maybe_lhs) = ctx.pop_expr_latest(loc, self).ok()? { + if let Some(maybe_lhs) = ctx.underlying(self).ok()?.expr_ret_stack.get(0) { + tracing::trace!("Disambiguate based on lhs: {}", maybe_lhs.debug_str(self)); if let ExprRet::Single(lhs_idx) = maybe_lhs { - if let Some(var_ty) = VarType::try_from_idx(self, lhs_idx) { + if let Some(var_ty) = VarType::try_from_idx(self, *lhs_idx) { if idxs.contains(&var_ty.ty_idx()) { - ctx.push_expr(maybe_lhs, self).ok()?; return Some(var_ty.ty_idx()); } } } - ctx.push_expr(maybe_lhs, self).ok()?; } // disambiguate based on storage location diff --git a/crates/solc-expressions/src/yul/yul_funcs.rs b/crates/solc-expressions/src/yul/yul_funcs.rs index 4b5436d3..217008df 100644 --- a/crates/solc-expressions/src/yul/yul_funcs.rs +++ b/crates/solc-expressions/src/yul/yul_funcs.rs @@ -163,12 +163,10 @@ pub trait YulFuncCaller: let [lhs_paths, rhs_paths] = inputs.into_sized(); self.cmp_inner(arena, ctx, loc, &lhs_paths, op, &rhs_paths)?; - let Some(result) = ctx.pop_expr_latest(loc, self).into_expr_err(loc)? else { - return Err(ExprErr::NoLhs( - loc, - "Yul Binary operation had no return".to_string(), - )); - }; + let result = ctx + .pop_n_latest_exprs(1, loc, self) + .into_expr_err(loc)? + .swap_remove(0); let res = ContextVarNode::from(result.expect_single().into_expr_err(loc)?); let next = self.advance_var_in_ctx(res, loc, ctx)?;