diff --git a/.gitignore b/.gitignore index 82367927b..44e3f17da 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ Cargo.lock /target **/*.rs.bk + diff --git a/Cargo.toml b/Cargo.toml index cd7e9800c..cb5312e15 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,7 +30,7 @@ serde = "1.0" serde_derive = { version = "1.0" } inkwell = { version = "0.1.0-beta.2", features = ["target-webassembly", "target-bpf", "llvm11-0"] } blake2-rfc = "0.2.18" -phf = { version = "0.8", features = ["macros"] } +phf = { version = "0.9", features = ["macros"] } unicode-xid = "0.2.0" handlebars = "3.4" contract-metadata = "0.2.0" @@ -54,7 +54,7 @@ ethereum-types = "0.10" wasmi = "0.9" rand = "0.8" sha2 = "0.9" -solana_rbpf = "0.2" +solana_rbpf = "^0.2.11" byteorder = "1.3" assert_cmd = "1.0" bincode = "1.3" diff --git a/build.rs b/build.rs index 7b9b83b05..bf7d56ab1 100644 --- a/build.rs +++ b/build.rs @@ -11,7 +11,7 @@ fn main() { let cxxflags = Command::new("llvm-config") .args(&["--cxxflags"]) .output() - .unwrap(); + .expect("could not execute llvm-config"); let cxxflags = String::from_utf8(cxxflags.stdout).unwrap(); diff --git a/src/codegen/cfg.rs b/src/codegen/cfg.rs index 0b047de45..0d0f5ec45 100644 --- a/src/codegen/cfg.rs +++ b/src/codegen/cfg.rs @@ -616,6 +616,12 @@ impl ControlFlowGraph { ty.to_string(ns), self.expr_to_string(contract, ns, e) ), + Expression::BytesCast(_, ty, from, e) => format!( + "{} from:{} ({})", + ty.to_string(ns), + from.to_string(ns), + self.expr_to_string(contract, ns, e) + ), Expression::Builtin(_, _, builtin, args) => format!( "(builtin {:?} ({}))", builtin, diff --git a/src/codegen/constant_folding.rs b/src/codegen/constant_folding.rs index f3de3e5fc..8285f5654 100644 --- a/src/codegen/constant_folding.rs +++ b/src/codegen/constant_folding.rs @@ -1062,7 +1062,7 @@ fn expression( | Expression::FormatString { .. } | Expression::InternalFunctionCfg(_) => (expr.clone(), false), // nothing else is permitted in cfg - _ => unreachable!(), + _ => panic!("expr should not be in cfg: {:?}", expr), } } diff --git a/src/codegen/expression.rs b/src/codegen/expression.rs index 880f762e3..9a5f83b8a 100644 --- a/src/codegen/expression.rs +++ b/src/codegen/expression.rs @@ -708,6 +708,12 @@ pub fn expression( ) } } + Expression::BytesCast(loc, ty, from, e) => Expression::BytesCast( + *loc, + ty.clone(), + from.clone(), + Box::new(expression(e, cfg, contract_no, ns, vartab)), + ), Expression::Load(loc, ty, e) => Expression::Load( *loc, ty.clone(), @@ -1784,24 +1790,25 @@ fn array_subscript( Type::Bytes(array_length) => { let res_ty = Type::Bytes(1); let from_ty = Type::Bytes(*array_length); + let index_ty = Type::Uint(*array_length as u16 * 8); Expression::Trunc( *loc, res_ty, Box::new(Expression::ShiftRight( *loc, - from_ty.clone(), + from_ty, Box::new(array), // shift by (array_length - 1 - index) * 8 Box::new(Expression::ShiftLeft( *loc, - from_ty.clone(), + index_ty.clone(), Box::new(Expression::Subtract( *loc, - from_ty.clone(), + index_ty.clone(), Box::new(Expression::NumberLiteral( *loc, - from_ty.clone(), + index_ty.clone(), BigInt::from_u8(array_length - 1).unwrap(), )), Box::new(cast_shift_arg( @@ -1814,7 +1821,7 @@ fn array_subscript( )), Box::new(Expression::NumberLiteral( *loc, - from_ty, + index_ty, BigInt::from_u8(3).unwrap(), )), )), diff --git a/src/sema/ast.rs b/src/sema/ast.rs index 8fc0f098f..4b3a8d5e7 100644 --- a/src/sema/ast.rs +++ b/src/sema/ast.rs @@ -62,6 +62,7 @@ pub struct EventDecl { pub fields: Vec, pub signature: String, pub anonymous: bool, + pub used: bool, } impl fmt::Display for EventDecl { @@ -284,6 +285,8 @@ pub struct Variable { pub visibility: pt::Visibility, pub constant: bool, pub initializer: Option, + pub assigned: bool, + pub read: bool, } #[derive(Clone, PartialEq)] diff --git a/src/sema/builtin.rs b/src/sema/builtin.rs index 45de3e7af..e6190ceaa 100644 --- a/src/sema/builtin.rs +++ b/src/sema/builtin.rs @@ -467,7 +467,7 @@ pub fn resolve_call( args: &[pt::Expression], contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, is_constant: bool, diagnostics: &mut Vec, ) -> Result { @@ -590,7 +590,7 @@ pub fn resolve_method_call( args: &[pt::Expression], contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, diagnostics: &mut Vec, ) -> Result { // The abi.* functions need special handling, others do not diff --git a/src/sema/contracts.rs b/src/sema/contracts.rs index eb2c9d546..e64916946 100644 --- a/src/sema/contracts.rs +++ b/src/sema/contracts.rs @@ -13,6 +13,7 @@ use super::statements; use super::symtable::Symtable; use super::variables; use super::{ast, SOLANA_FIRST_OFFSET}; +use crate::sema::unused_variable::emit_warning_local_variable; use crate::{emit, Target}; impl ast::Contract { @@ -249,7 +250,7 @@ fn resolve_base_args( .position(|e| e.contract_no == base_no) { if let Some(args) = &base.args { - let symtable = Symtable::new(); + let mut symtable = Symtable::new(); // find constructor which matches this if let Ok((Some(constructor_no), args)) = match_constructor_to_args( @@ -259,7 +260,7 @@ fn resolve_base_args( base_no, *contract_no, ns, - &symtable, + &mut symtable, &mut diagnostics, ) { ns.contracts[*contract_no].bases[pos].constructor = @@ -868,6 +869,12 @@ fn resolve_bodies( .is_err() { broken = true; + } else { + for variable in ns.functions[function_no].symtable.vars.values() { + if let Some(warning) = emit_warning_local_variable(&variable) { + ns.diagnostics.push(warning); + } + } } } diff --git a/src/sema/expression.rs b/src/sema/expression.rs index 3afa482a1..3fa6b1436 100644 --- a/src/sema/expression.rs +++ b/src/sema/expression.rs @@ -22,6 +22,9 @@ use super::eval::eval_const_number; use super::format::string_format; use super::symtable::Symtable; use crate::parser::pt; +use crate::sema::unused_variable::{ + assigned_variable, check_function_call, check_var_usage_expression, used_variable, +}; use crate::Target; use base58::{FromBase58, FromBase58Error}; @@ -1312,23 +1315,31 @@ pub fn expression( file_no: usize, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, is_constant: bool, diagnostics: &mut Vec, resolve_to: Option<&Type>, ) -> Result { match expr { - pt::Expression::ArrayLiteral(loc, exprs) => resolve_array_literal( - loc, - exprs, - file_no, - contract_no, - ns, - symtable, - is_constant, - diagnostics, - resolve_to, - ), + pt::Expression::ArrayLiteral(loc, exprs) => { + let res = resolve_array_literal( + loc, + exprs, + file_no, + contract_no, + ns, + symtable, + is_constant, + diagnostics, + resolve_to, + ); + + if let Ok(exp) = &res { + used_variable(ns, exp, symtable); + } + + res + } pt::Expression::BoolLiteral(loc, v) => Ok(Expression::BoolLiteral(*loc, *v)), pt::Expression::StringLiteral(v) => { // Concatenate the strings @@ -1704,6 +1715,8 @@ pub fn expression( resolve_to, )?; + check_var_usage_expression(ns, &left, &right, symtable); + let ty = coerce_int( &left.ty(), &l.loc(), @@ -1744,6 +1757,8 @@ pub fn expression( resolve_to, )?; + check_var_usage_expression(ns, &left, &right, symtable); + let ty = coerce_int( &left.ty(), &l.loc(), @@ -1784,6 +1799,8 @@ pub fn expression( resolve_to, )?; + check_var_usage_expression(ns, &left, &right, symtable); + let ty = coerce_int( &left.ty(), &l.loc(), @@ -1824,6 +1841,8 @@ pub fn expression( resolve_to, )?; + check_var_usage_expression(ns, &left, &right, symtable); + let ty = coerce_int( &left.ty(), &l.loc(), @@ -1864,6 +1883,7 @@ pub fn expression( None, )?; + check_var_usage_expression(ns, &left, &right, symtable); // left hand side may be bytes/int/uint // right hand size may be int/uint let _ = get_int_length(&left.ty(), &l.loc(), true, ns, diagnostics)?; @@ -1900,6 +1920,8 @@ pub fn expression( None, )?; + check_var_usage_expression(ns, &left, &right, symtable); + let left_type = left.ty(); // left hand side may be bytes/int/uint // right hand size may be int/uint @@ -1936,6 +1958,8 @@ pub fn expression( resolve_to, )?; + check_var_usage_expression(ns, &left, &right, symtable); + let ty = coerce_int( &left.ty(), &l.loc(), @@ -2005,6 +2029,8 @@ pub fn expression( resolve_to, )?; + check_var_usage_expression(ns, &left, &right, symtable); + let ty = coerce_int( &left.ty(), &l.loc(), @@ -2045,6 +2071,8 @@ pub fn expression( resolve_to, )?; + check_var_usage_expression(ns, &left, &right, symtable); + let ty = coerce_int( &left.ty(), &l.loc(), @@ -2114,6 +2142,8 @@ pub fn expression( resolve_to, )?; + check_var_usage_expression(ns, &base, &exp, symtable); + let base_type = base.ty(); let exp_type = exp.ty(); @@ -2168,6 +2198,7 @@ pub fn expression( None, )?; + check_var_usage_expression(ns, &left, &right, symtable); let ty = coerce_int( &left.ty(), &l.loc(), @@ -2207,6 +2238,7 @@ pub fn expression( None, )?; + check_var_usage_expression(ns, &left, &right, symtable); let ty = coerce_int( &left.ty(), &l.loc(), @@ -2245,6 +2277,7 @@ pub fn expression( diagnostics, None, )?; + check_var_usage_expression(ns, &left, &right, symtable); let ty = coerce_int( &left.ty(), @@ -2284,6 +2317,7 @@ pub fn expression( diagnostics, None, )?; + check_var_usage_expression(ns, &left, &right, symtable); let ty = coerce_int( &left.ty(), @@ -2341,6 +2375,7 @@ pub fn expression( resolve_to, )?; + used_variable(ns, &expr, symtable); Ok(Expression::Not( *loc, Box::new(cast(&loc, expr, &Type::Bool, true, ns, diagnostics)?), @@ -2358,6 +2393,7 @@ pub fn expression( resolve_to, )?; + used_variable(ns, &expr, symtable); let expr_ty = expr.ty(); get_int_length(&expr_ty, loc, true, ns, diagnostics)?; @@ -2388,6 +2424,7 @@ pub fn expression( resolve_to, )?; + used_variable(ns, &expr, symtable); let expr_type = expr.ty(); if let Expression::NumberLiteral(_, _, n) = expr { @@ -2410,6 +2447,7 @@ pub fn expression( diagnostics, resolve_to, )?; + used_variable(ns, &expr, symtable); let expr_type = expr.ty(); get_int_length(&expr_type, loc, false, ns, diagnostics)?; @@ -2438,6 +2476,7 @@ pub fn expression( diagnostics, resolve_to, )?; + check_var_usage_expression(ns, &left, &right, symtable); let cond = expression( c, file_no, @@ -2448,6 +2487,7 @@ pub fn expression( diagnostics, resolve_to, )?; + used_variable(ns, &cond, symtable); let cond = cast(&c.loc(), cond, &Type::Bool, true, ns, diagnostics)?; @@ -2569,6 +2609,7 @@ pub fn expression( diagnostics, )?; + check_function_call(ns, &expr, symtable); if expr.tys().len() > 1 { diagnostics.push(Diagnostic::error( *loc, @@ -2590,26 +2631,41 @@ pub fn expression( } match call.as_ref() { - pt::Expression::FunctionCall(_, ty, args) => new( - loc, - ty, - args, - file_no, - contract_no, - ns, - symtable, - diagnostics, - ), - pt::Expression::NamedFunctionCall(_, ty, args) => constructor_named_args( - loc, - ty, - args, - file_no, - contract_no, - ns, - symtable, - diagnostics, - ), + pt::Expression::FunctionCall(_, ty, args) => { + let res = new( + loc, + ty, + args, + file_no, + contract_no, + ns, + symtable, + diagnostics, + ); + + if let Ok(exp) = &res { + check_function_call(ns, &exp, symtable); + } + res + } + pt::Expression::NamedFunctionCall(_, ty, args) => { + let res = constructor_named_args( + loc, + ty, + args, + file_no, + contract_no, + ns, + symtable, + diagnostics, + ); + + if let Ok(exp) = &res { + check_function_call(ns, &exp, symtable); + } + + res + } _ => unreachable!(), } } @@ -2681,6 +2737,7 @@ pub fn expression( diagnostics, )?; + check_function_call(ns, &expr, symtable); if expr.tys().len() > 1 { diagnostics.push(Diagnostic::error( *loc, @@ -2760,6 +2817,8 @@ pub fn expression( diagnostics, )?; + check_var_usage_expression(ns, &l, &r, symtable); + Ok(Expression::Or(*loc, Box::new(l), Box::new(r))) } pt::Expression::And(loc, left, right) => { @@ -2798,6 +2857,7 @@ pub fn expression( ns, diagnostics, )?; + check_var_usage_expression(ns, &l, &r, symtable); Ok(Expression::And(*loc, Box::new(l), Box::new(r))) } @@ -2898,7 +2958,7 @@ fn constructor( file_no: usize, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, diagnostics: &mut Vec, ) -> Result { // The current contract cannot be constructed with new. In order to create @@ -2985,7 +3045,7 @@ pub fn match_constructor_to_args( contract_no: usize, args_contact_no: usize, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, diagnostics: &mut Vec, ) -> Result<(Option, Vec), ()> { let marker = diagnostics.len(); @@ -3093,7 +3153,7 @@ pub fn constructor_named_args( file_no: usize, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, diagnostics: &mut Vec, ) -> Result { let (ty, call_args, _) = collect_call_args(ty, diagnostics)?; @@ -3417,7 +3477,7 @@ pub fn new( file_no: usize, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, diagnostics: &mut Vec, ) -> Result { let (ty, call_args, call_args_loc) = collect_call_args(ty, diagnostics)?; @@ -3506,6 +3566,8 @@ pub fn new( Some(&Type::Uint(32)), )?; + used_variable(ns, &size_expr, symtable); + let size = cast(&size_loc, size_expr, &Type::Uint(32), true, ns, diagnostics)?; Ok(Expression::AllocDynamicArray( @@ -3524,7 +3586,7 @@ fn equal( file_no: usize, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, is_constant: bool, diagnostics: &mut Vec, ) -> Result { @@ -3549,6 +3611,8 @@ fn equal( None, )?; + check_var_usage_expression(ns, &left, &right, symtable); + // Comparing stringliteral against stringliteral if let (Expression::BytesLiteral(_, _, l), Expression::BytesLiteral(_, _, r)) = (&left, &right) { @@ -3640,7 +3704,7 @@ fn addition( file_no: usize, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, is_constant: bool, diagnostics: &mut Vec, resolve_to: Option<&Type>, @@ -3665,6 +3729,7 @@ fn addition( diagnostics, resolve_to, )?; + check_var_usage_expression(ns, &left, &right, symtable); // Concatenate stringliteral with stringliteral if let (Expression::BytesLiteral(_, _, l), Expression::BytesLiteral(_, _, r)) = (&left, &right) @@ -3777,7 +3842,7 @@ pub fn assign_single( file_no: usize, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, diagnostics: &mut Vec, ) -> Result { let var = expression( @@ -3790,6 +3855,7 @@ pub fn assign_single( diagnostics, None, )?; + assigned_variable(ns, &var, symtable); let var_ty = var.ty(); let val = expression( right, @@ -3801,6 +3867,7 @@ pub fn assign_single( diagnostics, Some(var_ty.deref_any()), )?; + used_variable(ns, &val, symtable); match &var { Expression::ConstantVariable(loc, _, Some(contract_no), var_no) => { diagnostics.push(Diagnostic::error( @@ -3865,7 +3932,7 @@ fn assign_expr( file_no: usize, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, diagnostics: &mut Vec, ) -> Result { let var = expression( @@ -3878,6 +3945,7 @@ fn assign_expr( diagnostics, None, )?; + assigned_variable(ns, &var, symtable); let var_ty = var.ty(); let resolve_to = if matches!( @@ -3899,6 +3967,7 @@ fn assign_expr( diagnostics, resolve_to, )?; + used_variable(ns, &set, symtable); let set_type = set.ty(); let op = |assign: Expression, @@ -4044,7 +4113,7 @@ fn incr_decr( file_no: usize, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, diagnostics: &mut Vec, ) -> Result { let op = |e: Expression, ty: Type| -> Expression { @@ -4071,6 +4140,7 @@ fn incr_decr( diagnostics, None, )?; + used_variable(ns, &var, symtable); let var_ty = var.ty(); match &var { @@ -4217,7 +4287,7 @@ fn member_access( file_no: usize, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, is_constant: bool, diagnostics: &mut Vec, resolve_to: Option<&Type>, @@ -4345,6 +4415,7 @@ fn member_access( match expr_ty { Type::Bytes(n) => { if id.name == "length" { + used_variable(ns, &expr, symtable); return Ok(Expression::NumberLiteral( *loc, Type::Uint(8), @@ -4356,7 +4427,10 @@ fn member_access( if id.name == "length" { return match dim.last().unwrap() { None => Ok(Expression::DynamicArrayLength(*loc, Box::new(expr))), - Some(d) => bigint_to_expression(loc, d, ns, diagnostics, Some(&Type::Uint(32))), + Some(d) => { + used_variable(ns, &expr, symtable); + bigint_to_expression(loc, d, ns, diagnostics, Some(&Type::Uint(32))) + } }; } } @@ -4465,9 +4539,9 @@ fn member_access( if !is_this { diagnostics.push(Diagnostic::error( - expr.loc(), - "substrate can only retrieve balance of this, like ‘address(this).balance’".to_string(), - )); + expr.loc(), + "substrate can only retrieve balance of this, like ‘address(this).balance’".to_string(), + )); return Err(()); } } @@ -4564,7 +4638,7 @@ fn array_subscript( file_no: usize, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, is_constant: bool, diagnostics: &mut Vec, ) -> Result { @@ -4692,7 +4766,7 @@ fn struct_literal( file_no: usize, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, is_constant: bool, diagnostics: &mut Vec, ) -> Result { @@ -4723,7 +4797,7 @@ fn struct_literal( diagnostics, Some(&struct_def.fields[i].ty), )?; - + used_variable(ns, &expr, symtable); fields.push(cast( loc, expr, @@ -4751,7 +4825,7 @@ fn call_function_type( file_no: usize, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, diagnostics: &mut Vec, ) -> Result { let mut function = expression( @@ -5004,7 +5078,7 @@ pub fn call_position_args( virtual_call: bool, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, diagnostics: &mut Vec, ) -> Result { let mut name_matches = 0; @@ -5141,7 +5215,7 @@ fn function_call_with_named_args( virtual_call: bool, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, diagnostics: &mut Vec, ) -> Result { let mut arguments = HashMap::new(); @@ -5300,7 +5374,7 @@ fn named_struct_literal( file_no: usize, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, is_constant: bool, diagnostics: &mut Vec, ) -> Result { @@ -5338,7 +5412,7 @@ fn named_struct_literal( diagnostics, Some(&f.ty), )?; - + used_variable(ns, &expr, symtable); fields[i] = cast(loc, expr, &f.ty, true, ns, diagnostics)?; } None => { @@ -5369,7 +5443,7 @@ fn method_call_pos_args( file_no: usize, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, diagnostics: &mut Vec, ) -> Result { if let pt::Expression::Variable(namespace) = var { @@ -5670,7 +5744,6 @@ fn method_call_pos_args( return Err(()); } }; - return Ok(Expression::Builtin( func.loc, vec![ret_ty], @@ -6188,7 +6261,7 @@ fn method_call_named_args( file_no: usize, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, diagnostics: &mut Vec, ) -> Result { if let pt::Expression::Variable(namespace) = var { @@ -6530,7 +6603,7 @@ fn resolve_array_literal( file_no: usize, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, is_constant: bool, diagnostics: &mut Vec, resolve_to: Option<&Type>, @@ -6571,6 +6644,7 @@ fn resolve_array_literal( first.ty() }; + used_variable(ns, &first, symtable); let mut exprs = vec![first]; for e in flattened { @@ -6584,6 +6658,7 @@ fn resolve_array_literal( diagnostics, Some(&ty), )?; + used_variable(ns, &other, symtable); if other.ty() != ty { other = cast(&e.loc(), other, &ty, true, ns, diagnostics)?; @@ -6727,7 +6802,7 @@ fn parse_call_args( file_no: usize, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, diagnostics: &mut Vec, ) -> Result { let mut args: HashMap<&String, &pt::NamedArgument> = HashMap::new(); @@ -6918,7 +6993,7 @@ pub fn function_call_expr( file_no: usize, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, is_constant: bool, diagnostics: &mut Vec, ) -> Result { @@ -7051,7 +7126,7 @@ pub fn named_function_call_expr( file_no: usize, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, diagnostics: &mut Vec, ) -> Result { let (ty, call_args, call_args_loc) = collect_call_args(ty, diagnostics)?; @@ -7147,7 +7222,7 @@ fn mapping_subscript( file_no: usize, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, is_constant: bool, diagnostics: &mut Vec, ) -> Result { diff --git a/src/sema/format.rs b/src/sema/format.rs index 03feeadb3..6026b706b 100644 --- a/src/sema/format.rs +++ b/src/sema/format.rs @@ -18,7 +18,7 @@ pub fn string_format( file_no: usize, contract_no: Option, ns: &mut Namespace, - symtable: &Symtable, + symtable: &mut Symtable, diagnostics: &mut Vec, ) -> Result { // first resolve the arguments. We can't say anything about the format string if the args are broken diff --git a/src/sema/mod.rs b/src/sema/mod.rs index 90cddd337..46fd79b0c 100644 --- a/src/sema/mod.rs +++ b/src/sema/mod.rs @@ -21,6 +21,7 @@ mod statements; pub mod symtable; pub mod tags; mod types; +mod unused_variable; mod variables; use self::contracts::visit_bases; @@ -30,6 +31,7 @@ use self::functions::{resolve_params, resolve_returns}; use self::symtable::Symtable; use self::variables::var_decl; use crate::file_cache::{FileCache, ResolvedFile}; +use crate::sema::unused_variable::{check_unused_events, check_unused_namespace_variables}; pub type ArrayDimension = Option<(pt::Loc, BigInt)>; @@ -40,8 +42,17 @@ pub const SOLANA_FIRST_OFFSET: u64 = 16; pub const SOLANA_SPARSE_ARRAY_SIZE: u64 = 1024; /// Load a file file from the cache, parse and resolve it. The file must be present in -/// the cache. This function is recursive for imports. +/// the cache. pub fn sema(file: ResolvedFile, cache: &mut FileCache, ns: &mut ast::Namespace) { + sema_file(file, cache, ns); + + // Checks for unused variables + check_unused_namespace_variables(ns); + check_unused_events(ns); +} + +/// Parse and resolve a file and its imports in a recursive manner. +fn sema_file(file: ResolvedFile, cache: &mut FileCache, ns: &mut ast::Namespace) { let file_no = ns.files.len(); let source_code = cache.get_file_contents(&file.full_path); @@ -168,7 +179,7 @@ fn resolve_import( } Ok(file) => { if !ns.files.iter().any(|f| *f == file.full_path) { - sema(file.clone(), cache, ns); + sema_file(file.clone(), cache, ns); // give up if we failed if diagnostics::any_errors(&ns.diagnostics) { @@ -1469,14 +1480,14 @@ impl ast::Namespace { expr: &pt::Expression, diagnostics: &mut Vec, ) -> Result { - let symtable = Symtable::new(); + let mut symtable = Symtable::new(); let size_expr = expression( &expr, file_no, contract_no, self, - &symtable, + &mut symtable, true, diagnostics, Some(&ast::Type::Uint(256)), diff --git a/src/sema/statements.rs b/src/sema/statements.rs index 0e046e6f7..7f849cbe1 100644 --- a/src/sema/statements.rs +++ b/src/sema/statements.rs @@ -6,6 +6,8 @@ use super::expression::{ }; use super::symtable::{LoopScopes, Symtable}; use crate::parser::pt; +use crate::sema::symtable::VariableUsage; +use crate::sema::unused_variable::{check_function_call, used_variable}; use std::collections::HashMap; pub fn resolve_function_body( @@ -23,9 +25,13 @@ pub fn resolve_function_body( for (i, p) in def.params.iter().enumerate() { let p = p.1.as_ref().unwrap(); if let Some(ref name) = p.name { - if let Some(pos) = - symtable.add(name, ns.functions[function_no].params[i].ty.clone(), ns) - { + if let Some(pos) = symtable.add( + name, + ns.functions[function_no].params[i].ty.clone(), + ns, + true, + VariableUsage::Parameter, + ) { ns.check_shadowing(file_no, contract_no, name); symtable.arguments.push(Some(pos)); @@ -74,9 +80,12 @@ pub fn resolve_function_body( base_no, contract_no, ns, - &symtable, + &mut symtable, &mut diagnostics, ) { + for arg in &args { + used_variable(ns, arg, &mut symtable); + } ns.functions[function_no] .bases .insert(base_no, (base.loc, constructor_no, args)); @@ -152,7 +161,7 @@ pub fn resolve_function_body( true, contract_no, ns, - &symtable, + &mut symtable, &mut diagnostics, ) { modifiers.push(e); @@ -176,9 +185,14 @@ pub fn resolve_function_body( if let Some(ref name) = p.1.as_ref().unwrap().name { return_required = false; - if let Some(pos) = symtable.add(name, ret.ty.clone(), ns) { + if let Some(pos) = symtable.add( + name, + ret.ty.clone(), + ns, + false, + VariableUsage::ReturnVariable, + ) { ns.check_shadowing(file_no, contract_no, name); - symtable.returns.push(pos); } } else { @@ -188,7 +202,15 @@ pub fn resolve_function_body( name: "".to_owned(), }; - let pos = symtable.add(&id, ret.ty.clone(), ns).unwrap(); + let pos = symtable + .add( + &id, + ret.ty.clone(), + ns, + true, + VariableUsage::AnonymousReturnVariable, + ) + .unwrap(); symtable.returns.push(pos); } @@ -309,12 +331,20 @@ fn statement( Some(&var_ty), )?; + used_variable(ns, &expr, symtable); + Some(cast(&expr.loc(), expr, &var_ty, true, ns, diagnostics)?) } else { None }; - if let Some(pos) = symtable.add(&decl.name, var_ty.clone(), ns) { + if let Some(pos) = symtable.add( + &decl.name, + var_ty.clone(), + ns, + initializer.is_some(), + VariableUsage::LocalVariable, + ) { ns.check_shadowing(file_no, contract_no, &decl.name); res.push(Statement::VariableDecl( @@ -398,7 +428,7 @@ fn statement( diagnostics, Some(&Type::Bool), )?; - + used_variable(ns, &expr, symtable); let cond = cast(&expr.loc(), expr, &Type::Bool, true, ns, diagnostics)?; symtable.new_scope(); @@ -433,7 +463,7 @@ fn statement( diagnostics, Some(&Type::Bool), )?; - + used_variable(ns, &expr, symtable); let cond = cast(&expr.loc(), expr, &Type::Bool, true, ns, diagnostics)?; symtable.new_scope(); @@ -467,6 +497,7 @@ fn statement( diagnostics, Some(&Type::Bool), )?; + used_variable(ns, &expr, symtable); let cond = cast(&expr.loc(), expr, &Type::Bool, true, ns, diagnostics)?; @@ -710,6 +741,15 @@ fn statement( diagnostics, )?; + for offset in symtable.returns.iter() { + let elem = symtable.vars.get_mut(offset).unwrap(); + (*elem).assigned = true; + } + + for item in &vals { + used_variable(ns, item, symtable); + } + res.push(Statement::Return(*loc, vals)); Ok(false) @@ -861,7 +901,8 @@ fn emit_event( let mut temp_diagnostics = Vec::new(); for event_no in &event_nos { - let event = &ns.events[*event_no]; + let event = &mut ns.events[*event_no]; + event.used = true; if args.len() != event.fields.len() { temp_diagnostics.push(Diagnostic::error( *loc, @@ -897,6 +938,7 @@ fn emit_event( break; } }; + used_variable(ns, &arg, symtable); match cast( &arg.loc(), @@ -956,7 +998,8 @@ fn emit_event( let mut temp_diagnostics = Vec::new(); for event_no in &event_nos { - let event = &ns.events[*event_no]; + let event = &mut ns.events[*event_no]; + event.used = true; let params_len = event.fields.len(); if params_len != arguments.len() { @@ -1022,6 +1065,8 @@ fn emit_event( } }; + used_variable(ns, &arg, symtable); + match cast(&arg.loc(), arg, ¶m.ty, true, ns, &mut temp_diagnostics) { Ok(expr) => cast_args.push(expr), Err(_) => { @@ -1156,7 +1201,13 @@ fn destructure( let (ty, ty_loc) = resolve_var_decl_ty(&ty, &storage, file_no, contract_no, ns, diagnostics)?; - if let Some(pos) = symtable.add(&name, ty.clone(), ns) { + if let Some(pos) = symtable.add( + &name, + ty.clone(), + ns, + true, + VariableUsage::DestructureVariable, + ) { ns.check_shadowing(file_no, contract_no, &name); left_tys.push(Some(ty.clone())); @@ -1204,27 +1255,35 @@ fn destructure_values( diagnostics: &mut Vec, ) -> Result { let expr = match expr { - pt::Expression::FunctionCall(loc, ty, args) => function_call_expr( - loc, - ty, - args, - file_no, - contract_no, - ns, - symtable, - false, - diagnostics, - )?, - pt::Expression::NamedFunctionCall(loc, ty, args) => named_function_call_expr( - loc, - ty, - args, - file_no, - contract_no, - ns, - symtable, - diagnostics, - )?, + pt::Expression::FunctionCall(loc, ty, args) => { + let res = function_call_expr( + loc, + ty, + args, + file_no, + contract_no, + ns, + symtable, + false, + diagnostics, + )?; + check_function_call(ns, &res, symtable); + res + } + pt::Expression::NamedFunctionCall(loc, ty, args) => { + let res = named_function_call_expr( + loc, + ty, + args, + file_no, + contract_no, + ns, + symtable, + diagnostics, + )?; + check_function_call(ns, &res, symtable); + res + } pt::Expression::Ternary(loc, cond, left, right) => { let cond = expression( cond, @@ -1237,6 +1296,7 @@ fn destructure_values( Some(&Type::Bool), )?; + used_variable(ns, &cond, symtable); let left = destructure_values( &left.loc(), left, @@ -1248,7 +1308,7 @@ fn destructure_values( ns, diagnostics, )?; - + used_variable(ns, &left, symtable); let right = destructure_values( &right.loc(), right, @@ -1260,6 +1320,7 @@ fn destructure_values( ns, diagnostics, )?; + used_variable(ns, &right, symtable); return Ok(Expression::Ternary( *loc, @@ -1306,7 +1367,9 @@ fn destructure_values( )); return Err(()); } - _ => (), + _ => { + used_variable(ns, &e, symtable); + } } list.push(e); @@ -1559,27 +1622,37 @@ fn try_catch( } let fcall = match expr { - pt::Expression::FunctionCall(loc, ty, args) => function_call_expr( - loc, - ty, - args, - file_no, - contract_no, - ns, - symtable, - false, - diagnostics, - )?, - pt::Expression::NamedFunctionCall(loc, ty, args) => named_function_call_expr( - loc, - ty, - args, - file_no, - contract_no, - ns, - symtable, - diagnostics, - )?, + pt::Expression::FunctionCall(loc, ty, args) => { + let res = function_call_expr( + loc, + ty, + args, + file_no, + contract_no, + ns, + symtable, + false, + diagnostics, + )?; + check_function_call(ns, &res, symtable); + res + } + pt::Expression::NamedFunctionCall(loc, ty, args) => { + let res = named_function_call_expr( + loc, + ty, + args, + file_no, + contract_no, + ns, + symtable, + diagnostics, + )?; + + check_function_call(ns, &res, symtable); + + res + } pt::Expression::New(loc, call) => { let mut call = call.as_ref(); @@ -1598,26 +1671,36 @@ fn try_catch( } match call { - pt::Expression::FunctionCall(_, ty, args) => new( - loc, - ty, - args, - file_no, - contract_no, - ns, - symtable, - diagnostics, - )?, - pt::Expression::NamedFunctionCall(_, ty, args) => constructor_named_args( - loc, - ty, - args, - file_no, - contract_no, - ns, - symtable, - diagnostics, - )?, + pt::Expression::FunctionCall(_, ty, args) => { + let res = new( + loc, + ty, + args, + file_no, + contract_no, + ns, + symtable, + diagnostics, + )?; + check_function_call(ns, &res, symtable); + + res + } + pt::Expression::NamedFunctionCall(_, ty, args) => { + let res = constructor_named_args( + loc, + ty, + args, + file_no, + contract_no, + ns, + symtable, + diagnostics, + )?; + check_function_call(ns, &res, symtable); + + res + } _ => unreachable!(), } } @@ -1737,7 +1820,13 @@ fn try_catch( } if let Some(name) = name { - if let Some(pos) = symtable.add(&name, ret_ty.clone(), ns) { + if let Some(pos) = symtable.add( + &name, + ret_ty.clone(), + ns, + false, + VariableUsage::TryCatchReturns, + ) { ns.check_shadowing(file_no, contract_no, &name); params.push(( Some(pos), @@ -1840,7 +1929,13 @@ fn try_catch( }; if let Some(name) = &error_stmt.1.name { - if let Some(pos) = symtable.add(&name, Type::String, ns) { + if let Some(pos) = symtable.add( + &name, + Type::String, + ns, + true, + VariableUsage::TryCatchErrorString, + ) { ns.check_shadowing(file_no, contract_no, &name); error_pos = Some(pos); @@ -1904,7 +1999,9 @@ fn try_catch( let mut catch_stmt_resolved = Vec::new(); if let Some(name) = &catch_stmt.0.name { - if let Some(pos) = symtable.add(&name, catch_ty, ns) { + if let Some(pos) = + symtable.add(&name, catch_ty, ns, true, VariableUsage::TryCatchErrorBytes) + { ns.check_shadowing(file_no, contract_no, &name); catch_param_pos = Some(pos); catch_param.name = name.name.to_string(); diff --git a/src/sema/symtable.rs b/src/sema/symtable.rs index 379131a3f..f495b888b 100644 --- a/src/sema/symtable.rs +++ b/src/sema/symtable.rs @@ -12,6 +12,21 @@ pub struct Variable { pub ty: Type, pub pos: usize, pub slice: bool, + pub assigned: bool, + pub read: bool, + pub usage_type: VariableUsage, +} + +#[derive(Clone)] +pub enum VariableUsage { + Parameter, + ReturnVariable, + AnonymousReturnVariable, + LocalVariable, + DestructureVariable, + TryCatchReturns, + TryCatchErrorString, + TryCatchErrorBytes, } struct VarScope(HashMap, Option>); @@ -36,7 +51,14 @@ impl Symtable { } } - pub fn add(&mut self, id: &pt::Identifier, ty: Type, ns: &mut Namespace) -> Option { + pub fn add( + &mut self, + id: &pt::Identifier, + ty: Type, + ns: &mut Namespace, + assigned: bool, + usage_type: VariableUsage, + ) -> Option { let pos = ns.next_id; ns.next_id += 1; @@ -47,6 +69,9 @@ impl Symtable { ty, pos, slice: false, + assigned, + usage_type, + read: false, }, ); diff --git a/src/sema/types.rs b/src/sema/types.rs index a341c2c54..a25a0f895 100644 --- a/src/sema/types.rs +++ b/src/sema/types.rs @@ -85,6 +85,7 @@ pub fn resolve_typenames<'a>( fields: Vec::new(), anonymous: def.anonymous, signature: String::new(), + used: false, }); delay.events.push((pos, def, None)); @@ -279,6 +280,7 @@ fn resolve_contract<'a>( fields: Vec::new(), anonymous: s.anonymous, signature: String::new(), + used: false, }); delay.events.push((pos, s, Some(contract_no))); diff --git a/src/sema/unused_variable.rs b/src/sema/unused_variable.rs new file mode 100644 index 000000000..e45f11d9f --- /dev/null +++ b/src/sema/unused_variable.rs @@ -0,0 +1,392 @@ +use crate::parser::pt::Loc; +use crate::sema::ast::{ + Builtin, Diagnostic, ErrorType, Expression, Level, Namespace, Note, Statement, +}; +use crate::sema::symtable::{Symtable, VariableUsage}; +use crate::sema::{ast, symtable}; + +/// Mark variables as assigned, either in the symbol table (for local variables) or in the +/// Namespace (for storage variables) +pub fn assigned_variable(ns: &mut Namespace, exp: &Expression, symtable: &mut Symtable) { + match &exp { + Expression::StorageVariable(_, _, contract_no, offset) => { + ns.contracts[*contract_no].variables[*offset].assigned = true; + } + + Expression::Variable(_, _, offset) => { + let var = symtable.vars.get_mut(offset).unwrap(); + (*var).assigned = true; + } + + Expression::StructMember(_, _, str, _) => { + assigned_variable(ns, str, symtable); + } + + Expression::Subscript(_, _, array, index) + | Expression::DynamicArraySubscript(_, _, array, index) + | Expression::StorageBytesSubscript(_, array, index) => { + assigned_variable(ns, array, symtable); + used_variable(ns, index, symtable); + } + + Expression::Trunc(_, _, var) + | Expression::Cast(_, _, var) + | Expression::BytesCast(_, _, _, var) => { + assigned_variable(ns, var, symtable); + } + + _ => {} + } +} + +/// Mark variables as used, either in the symbol table (for local variables) or in the +/// Namespace (for global constants and storage variables) +/// The functions handles complex expressions in a recursive fashion, such as array length call, +/// assign expressions and array subscripts. +pub fn used_variable(ns: &mut Namespace, exp: &Expression, symtable: &mut Symtable) { + match &exp { + Expression::StorageVariable(_, _, contract_no, offset) => { + ns.contracts[*contract_no].variables[*offset].read = true; + } + + Expression::Variable(_, _, offset) => { + let var = symtable.vars.get_mut(offset).unwrap(); + (*var).read = true; + } + + Expression::ConstantVariable(_, _, Some(contract_no), offset) => { + ns.contracts[*contract_no].variables[*offset].read = true; + } + + Expression::ConstantVariable(_, _, None, offset) => { + ns.constants[*offset].read = true; + } + + Expression::StructMember(_, _, str, _) => { + used_variable(ns, str, symtable); + } + + Expression::Subscript(_, _, array, index) + | Expression::DynamicArraySubscript(_, _, array, index) + | Expression::StorageBytesSubscript(_, array, index) => { + used_variable(ns, array, symtable); + used_variable(ns, index, symtable); + } + + Expression::DynamicArrayLength(_, array) + | Expression::StorageArrayLength { + loc: _, + ty: _, + array, + .. + } => { + used_variable(ns, array, symtable); + } + + Expression::StorageLoad(_, _, expr) + | Expression::SignExt(_, _, expr) + | Expression::ZeroExt(_, _, expr) + | Expression::Trunc(_, _, expr) + | Expression::Cast(_, _, expr) + | Expression::BytesCast(_, _, _, expr) => { + used_variable(ns, expr, symtable); + } + + _ => {} + } +} + +/// Mark function arguments as used. If the function is an attribute of another variable, mark the +/// usage of the latter as well +pub fn check_function_call(ns: &mut Namespace, exp: &Expression, symtable: &mut Symtable) { + match &exp { + Expression::InternalFunctionCall { + loc: _, + returns: _, + function, + args, + } + | Expression::ExternalFunctionCall { + loc: _, + returns: _, + function, + args, + .. + } => { + for arg in args { + used_variable(ns, arg, symtable); + } + check_function_call(ns, function, symtable); + } + + Expression::Constructor { + loc: _, + contract_no: _, + constructor_no: _, + args, + .. + } => { + for arg in args { + used_variable(ns, arg, symtable); + } + } + + Expression::ExternalFunctionCallRaw { + loc: _, + ty: _, + address, + args, + .. + } => { + used_variable(ns, args, symtable); + used_variable(ns, address, symtable); + } + + Expression::ExternalFunction { + loc: _, + ty: _, + address, + function_no, + } => { + used_variable(ns, address, symtable); + if ns.functions[*function_no].is_accessor { + let body = ns.functions[*function_no].body[0].clone(); + if let Statement::Return(_, exprs) = &body { + used_variable(ns, &exprs[0], symtable); + } + } + } + + Expression::Builtin(_, _, expr_type, args) => match expr_type { + Builtin::ArrayPush => { + assigned_variable(ns, &args[0], symtable); + if args.len() > 1 { + used_variable(ns, &args[1], symtable); + } + } + + Builtin::ArrayPop => { + used_variable(ns, &args[0], symtable); + } + _ => {} + }, + + Expression::DynamicArrayPush(_, array, _, arg) => { + assigned_variable(ns, array, symtable); + used_variable(ns, arg, symtable); + } + + Expression::DynamicArrayPop(_, array, _) => { + used_variable(ns, array, symtable); + } + + _ => {} + } +} + +/// Marks as used variables that appear in an expression with right and left hand side. +pub fn check_var_usage_expression( + ns: &mut Namespace, + left: &Expression, + right: &Expression, + symtable: &mut Symtable, +) { + used_variable(ns, left, symtable); + used_variable(ns, right, symtable); +} + +/// Generate warnings for unused varibles +fn generate_unused_warning(loc: Loc, text: &str, notes: Vec) -> Diagnostic { + Diagnostic { + level: Level::Warning, + ty: ErrorType::Warning, + pos: Some(loc), + message: text.parse().unwrap(), + notes, + } +} + +/// Emit different warning types according to the function variable usage +pub fn emit_warning_local_variable(variable: &symtable::Variable) -> Option { + match &variable.usage_type { + VariableUsage::Parameter => { + if !variable.read { + return Some(generate_unused_warning( + variable.id.loc, + &format!( + "function parameter '{}' has never been read", + variable.id.name + ), + vec![], + )); + } + None + } + + VariableUsage::ReturnVariable => { + if !variable.assigned { + return Some(generate_unused_warning( + variable.id.loc, + &format!( + "return variable '{}' has never been assigned", + variable.id.name + ), + vec![], + )); + } + None + } + + VariableUsage::LocalVariable => { + if !variable.assigned && !variable.read { + return Some(generate_unused_warning( + variable.id.loc, + &format!( + "local variable '{}' has never been read nor assigned", + variable.id.name + ), + vec![], + )); + } else if variable.assigned && !variable.read { + return Some(generate_unused_warning( + variable.id.loc, + &format!( + "local variable '{}' has been assigned, but never read", + variable.id.name + ), + vec![], + )); + } else if !variable.assigned && variable.read { + return Some(generate_unused_warning( + variable.id.loc, + &format!( + "local variable '{}' has never been assigned a value, but has been read", + variable.id.name + ), + vec![], + )); + } + None + } + + VariableUsage::DestructureVariable => { + if !variable.read { + return Some(generate_unused_warning( + variable.id.loc, + &format!( + "destructure variable '{}' has never been used", + variable.id.name + ), + vec![], + )); + } + + None + } + + VariableUsage::TryCatchReturns => { + if !variable.read { + return Some(generate_unused_warning( + variable.id.loc, + &format!( + "try-catch returns variable '{}' has never been read", + variable.id.name + ), + vec![], + )); + } + + None + } + + VariableUsage::TryCatchErrorBytes => { + if !variable.read { + return Some(generate_unused_warning( + variable.id.loc, + &format!( + "try-catch error bytes '{}' has never been used", + variable.id.name + ), + vec![], + )); + } + + None + } + + VariableUsage::TryCatchErrorString => { + if !variable.read { + return Some(generate_unused_warning( + variable.id.loc, + &format!( + "try-catch error string '{}' has never been used", + variable.id.name + ), + vec![], + )); + } + + None + } + VariableUsage::AnonymousReturnVariable => None, + } +} + +/// Emit warnings depending on the storage variable usage +fn emit_warning_contract_variables(variable: &ast::Variable) -> Option { + if variable.assigned && !variable.read { + return Some(generate_unused_warning( + variable.loc, + &format!( + "storage variable '{}' has been assigned, but never read", + variable.name + ), + vec![], + )); + } else if !variable.assigned && !variable.read { + return Some(generate_unused_warning( + variable.loc, + &format!("storage variable '{}' has never been used", variable.name), + vec![], + )); + } + //Solidity attributes zero value to contract values that have never been assigned + //There is no need to raise warning if we use them, as they have a valid value. + + None +} + +/// Check for unused constants and storage variables +pub fn check_unused_namespace_variables(ns: &mut Namespace) { + for contract in &ns.contracts { + for variable in &contract.variables { + if let Some(warning) = emit_warning_contract_variables(variable) { + ns.diagnostics.push(warning); + } + } + } + + //Global constants should have been initialized during declaration + for constant in &ns.constants { + if !constant.read { + ns.diagnostics.push(generate_unused_warning( + constant.loc, + &format!("global constant '{}' has never been used", constant.name), + vec![], + )) + } + } +} + +/// Check for unused events +pub fn check_unused_events(ns: &mut Namespace) { + for event in &ns.events { + if !event.used { + ns.diagnostics.push(generate_unused_warning( + event.loc, + &format!("event '{}' has never been emitted", event.name), + vec![], + )) + } + } +} diff --git a/src/sema/variables.rs b/src/sema/variables.rs index 68cf98268..6a3b9a57c 100644 --- a/src/sema/variables.rs +++ b/src/sema/variables.rs @@ -184,7 +184,7 @@ pub fn var_decl( file_no, contract_no, ns, - &symtable, + symtable, is_constant, &mut diagnostics, Some(&ty), @@ -249,7 +249,9 @@ pub fn var_decl( visibility: visibility.clone(), ty: ty.clone(), constant: is_constant, + assigned: initializer.is_some(), initializer, + read: false, }; let pos = if let Some(contract_no) = contract_no { diff --git a/tests/solana.rs b/tests/solana.rs index 43d130718..35a9caafd 100644 --- a/tests/solana.rs +++ b/tests/solana.rs @@ -894,13 +894,6 @@ impl VirtualMachine { let program = &self.stack[0]; - let mut executable = >::from_elf( - &self.account_data[&program.program].data, - None, - Config::default(), - ) - .expect("should work"); - let mut syscall_registry = SyscallRegistry::default(); syscall_registry .register_syscall_by_name(b"sol_log_", SolLog::call) @@ -926,7 +919,13 @@ impl VirtualMachine { .register_syscall_by_name(b"sol_alloc_free_", SyscallAllocFree::call) .unwrap(); - executable.set_syscall_registry(syscall_registry); + let executable = >::from_elf( + &self.account_data[&program.program].data, + None, + Config::default(), + syscall_registry, + ) + .expect("should work"); let mut vm = EbpfVm::::new( executable.as_ref(), diff --git a/tests/substrate_tests/arrays.rs b/tests/substrate_tests/arrays.rs index e7862fd90..dec128f01 100644 --- a/tests/substrate_tests/arrays.rs +++ b/tests/substrate_tests/arrays.rs @@ -2,7 +2,7 @@ use parity_scale_codec::Encode; use parity_scale_codec_derive::{Decode, Encode}; use rand::Rng; -use crate::{build_solidity, first_error, parse_and_resolve}; +use crate::{build_solidity, first_error, no_errors, parse_and_resolve}; use solang::Target; #[derive(Debug, PartialEq, Encode, Decode)] @@ -1916,3 +1916,34 @@ fn alloc_size_from_storage() { runtime.function("contfunc", Vec::new()); assert_eq!(runtime.vm.output, vec![0u64].encode()); } + +#[test] +fn lucas() { + let ns = parse_and_resolve( + r#"contract Test { + bytes byteArr; + bytes32 baRR; + + function get() public { + string memory s = "Test"; + byteArr = bytes(s); + uint16 a = 1; + uint8 b; + b = uint8(a); + + uint256 c; + c = b; + bytes32 b32; + b32 = bytes32(byteArr); + baRR = bytes32(c); + uint i1 = 1; + uint i2 = 1; + assert(b32[(i1*i2)-i1] == bytes1(baRR)); + } + } + "#, + Target::Substrate, + ); + + no_errors(ns.diagnostics); +} diff --git a/tests/substrate_tests/tags.rs b/tests/substrate_tests/tags.rs index 7fc8d66a3..5350d29c8 100644 --- a/tests/substrate_tests/tags.rs +++ b/tests/substrate_tests/tags.rs @@ -184,7 +184,8 @@ fn event_tag() { Target::Substrate, ); - assert_eq!(ns.diagnostics.len(), 0); + //Event never emitted generates a warning + assert_eq!(ns.diagnostics.len(), 1); assert_eq!(ns.events[0].tags[0].tag, "param"); assert_eq!(ns.events[0].tags[0].value, "asdad"); @@ -208,7 +209,8 @@ fn event_tag() { Target::Substrate, ); - assert_eq!(ns.diagnostics.len(), 1); + //Event never emitted generates a warning + assert_eq!(ns.diagnostics.len(), 2); assert_eq!(ns.events[0].tags[0].tag, "title"); assert_eq!(ns.events[0].tags[0].value, "foo bar"); @@ -399,7 +401,7 @@ fn functions() { Target::Substrate, ); - assert_eq!(ns.diagnostics.len(), 2); + assert_eq!(ns.diagnostics.len(), 5); assert_eq!(ns.functions[0].tags[0].tag, "param"); assert_eq!(ns.functions[0].tags[0].value, "sadad"); @@ -441,7 +443,8 @@ fn variables() { Target::Substrate, ); - assert_eq!(ns.diagnostics.len(), 2); + //Variable 'y' has never been used (one item error in diagnostic) + assert_eq!(ns.diagnostics.len(), 3); assert_eq!(ns.contracts[0].variables[0].tags[0].tag, "notice"); assert_eq!(ns.contracts[0].variables[0].tags[0].value, "so here we are"); diff --git a/tests/unused_variable_detection.rs b/tests/unused_variable_detection.rs new file mode 100644 index 000000000..4e27ced1d --- /dev/null +++ b/tests/unused_variable_detection.rs @@ -0,0 +1,901 @@ +use itertools::Itertools; +use solang::file_cache::FileCache; +use solang::sema::ast; +use solang::sema::ast::{Diagnostic, Level}; +use solang::{parse_and_resolve, Target}; + +fn generic_target_parse(src: &'static str) -> ast::Namespace { + let mut cache = FileCache::new(); + cache.set_file_contents("test.sol", src.to_string()); + + parse_and_resolve("test.sol", &mut cache, Target::Generic) +} + +fn generic_parse_two_files(src1: &'static str, src2: &'static str) -> ast::Namespace { + let mut cache = FileCache::new(); + cache.set_file_contents("test.sol", src1.to_string()); + cache.set_file_contents("test2.sol", src2.to_string()); + + parse_and_resolve("test.sol", &mut cache, Target::Generic) +} + +fn count_warnings(diagnostics: &[Diagnostic]) -> usize { + diagnostics + .iter() + .filter(|&x| x.level == Level::Warning) + .count() +} + +fn get_first_warning(diagnostics: &[Diagnostic]) -> &Diagnostic { + diagnostics + .iter() + .find_or_first(|&x| x.level == Level::Warning) + .unwrap() +} + +fn get_warnings(diagnostics: &[Diagnostic]) -> Vec<&Diagnostic> { + let mut res = Vec::new(); + for elem in diagnostics { + if elem.level == Level::Warning { + res.push(elem); + } + } + + res +} + +fn assert_message_in_warnings(diagnostics: &[Diagnostic], message: &str) -> bool { + let warnings = get_warnings(diagnostics); + for warning in warnings { + if warning.message == message { + return true; + } + } + + false +} + +#[test] +fn emit_event() { + //Used event + let case_1 = r#" + contract usedEvent { + event Hey(uint8 n); + function emitEvent(uint8 n) public { + emit Hey(n); + } + } + "#; + + let ns = generic_target_parse(case_1); + assert_eq!(count_warnings(&ns.diagnostics), 0); + + // Unused event + let case_2 = r#" + contract usedEvent { + event Hey(uint8 n); + event Hello(uint8 n); + function emitEvent(uint8 n) public { + emit Hey(n); + } + } + "#; + + let ns = generic_target_parse(case_2); + assert_eq!(count_warnings(&ns.diagnostics), 1); + assert_eq!( + get_first_warning(&ns.diagnostics).message, + "event 'Hello' has never been emitted" + ); +} + +#[test] +fn constant_variable() { + let file_2 = r#" + uint32 constant outside = 2; + "#; + + let file_1 = r#" + import "test2.sol"; + contract Testing { + uint32 test; + uint32 constant cte = 5; + constructor() { + test = outside; + test = cte; + } + + function get() public view returns (uint32) { + return test; + } + } + "#; + + //Constant properly read + let ns = generic_parse_two_files(file_1, file_2); + assert_eq!(count_warnings(&ns.diagnostics), 0); + + let file_1 = r#" + import "test2.sol"; + contract Testing { + uint32 test; + uint32 constant cte = 5; + constructor() { + test = 45; + } + + function get() public view returns (uint32) { + return test; + } + } + "#; + + let ns = generic_parse_two_files(file_1, file_2); + assert_eq!(count_warnings(&ns.diagnostics), 2); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "storage variable 'cte' has been assigned, but never read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "global constant 'outside' has never been used" + )); +} + +#[test] +fn storage_variable() { + let file = r#" + contract Test { + string str = "This is a test"; + string str2; + constructor() { + str = "This is another test"; + } + } + "#; + + let ns = generic_target_parse(file); + let warnings = get_warnings(&ns.diagnostics); + assert_eq!(warnings.len(), 2); + assert_eq!( + warnings[0].message, + "storage variable 'str' has been assigned, but never read" + ); + assert_eq!( + warnings[1].message, + "storage variable 'str2' has never been used" + ); + + let file = r#" + contract Test { + string str = "This is a test"; + string str2; + constructor() { + str = "This is another test"; + str2 = str; + } + } + "#; + + let ns = generic_target_parse(file); + assert_eq!(count_warnings(&ns.diagnostics), 1); + assert_eq!( + get_first_warning(&ns.diagnostics).message, + "storage variable 'str2' has been assigned, but never read" + ); + + let file = r#" + contract Test { + string str = "This is a test"; + constructor() { + str = "This is another test"; + } + } + + contract Test2 is Test { + function get() public view returns (string) { + return str; + } + } + "#; + + let ns = generic_target_parse(file); + assert_eq!(count_warnings(&ns.diagnostics), 0); +} + +#[test] +fn state_variable() { + let file = r#" + contract Test { + function get() public pure { + uint32 a = 1; + uint32 b; + b = 1; + uint32 c; + + uint32 d; + d = 1; + uint32 e; + e = d*5; + d = e/5; + } + } + "#; + + let ns = generic_target_parse(file); + assert_eq!(count_warnings(&ns.diagnostics), 3); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 'b' has been assigned, but never read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 'a' has been assigned, but never read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 'c' has never been read nor assigned" + )); +} + +#[test] +fn struct_usage() { + let file = r#" + struct testing { + uint8 it; + bool tf; + } + + contract Test { + testing t1; + testing t4; + testing t6; + constructor() { + t1 = testing(8, false); + } + + function modify() public returns (uint8) { + testing memory t2; + t2.it = 4; + + + t4 = testing(1, true); + testing storage t3 = t4; + uint8 k = 2*4/t3.it; + testing t5; + + return k; + } + } + "#; + + let ns = generic_target_parse(file); + assert_eq!(count_warnings(&ns.diagnostics), 4); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 't2' has been assigned, but never read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "storage variable 't1' has been assigned, but never read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 't5' has never been read nor assigned" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "storage variable 't6' has never been used" + )); +} + +#[test] +fn subscript() { + let file = r#" + contract Test { + int[] arr1; + int[4] arr2; + int[4] arr3; + bytes byteArr; + + uint constant e = 1; + + function get() public { + uint8 a = 1; + uint8 b = 2; + + arr1[a] = 1; + arr2[a+b] = 2; + + uint8 c = 1; + uint8 d = 1; + int[] memory arr4; + arr4[0] = 1; + int[4] storage arr5 = arr3; + arr5[c*d] = 1; + + byteArr[e] = 0x05; + } + } + "#; + + let ns = generic_target_parse(file); + assert_eq!(count_warnings(&ns.diagnostics), 5); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 'arr4' has been assigned, but never read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 'arr5' has been assigned, but never read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "storage variable 'arr1' has been assigned, but never read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "storage variable 'arr2' has been assigned, but never read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "storage variable 'byteArr' has been assigned, but never read" + )); + + let file = r#" + contract Test { + int[] arr1; + int[4] arr2; + int[4] arr3; + bytes byteArr; + + uint constant e = 1; + + function get() public { + uint8 a = 1; + uint8 b = 2; + + arr1[a] = 1; + arr2[a+b] = 2; + assert(arr1[a] == arr2[b]); + + uint8 c = 1; + uint8 d = 1; + int[] memory arr4; + arr4[0] = 1; + int[4] storage arr5 = arr3; + arr5[c*d] = 1; + assert(arr4[c] == arr5[d]); + assert(arr3[c] == arr5[d]); + + byteArr[e] = 0x05; + assert(byteArr[e] == byteArr[e]); + } + } + "#; + + let ns = generic_target_parse(file); + assert_eq!(count_warnings(&ns.diagnostics), 0); +} + +#[test] +fn assign_trunc_cast() { + //This covers ZeroExt as well + let file = r#" + contract Test { + bytes byteArr; + bytes32 baRR; + + function get() public { + string memory s = "Test"; + byteArr = bytes(s); + uint16 a = 1; + uint8 b; + b = uint8(a); + + uint256 c; + c = b; + bytes32 b32; + bytes memory char = bytes(bytes32(uint(a) * 2 ** (8 * b))); + baRR = bytes32(c); + bytes32 cdr = bytes32(char); + assert(b32 == baRR); + if(b32 != cdr) { + + } + } + } +"#; + + let ns = generic_target_parse(file); + assert_eq!(count_warnings(&ns.diagnostics), 2); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 'b32' has never been assigned a value, but has been read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "storage variable 'byteArr' has been assigned, but never read" + )); +} + +#[test] +fn array_length() { + let file = r#" + contract Test { + int[5] arr1; + int[] arr2; + + function get() public view returns (bool) { + int[5] memory arr3; + int[] memory arr4; + + bool test = false; + if(arr1.length == arr2.length) { + test = true; + } + else if(arr3.length != arr4.length) { + test = false; + } + + return test; + } + } + "#; + + let ns = generic_target_parse(file); + assert_eq!(count_warnings(&ns.diagnostics), 2); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 'arr3' has never been assigned a value, but has been read", + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 'arr4' has never been assigned a value, but has been read" + )); +} + +#[test] +fn sign_ext_storage_load() { + let file = r#" + contract Test { + bytes a; + + function use(bytes memory b) pure public { + assert(b[0] == b[1]); + } + + function get() public pure returns (int16 ret) { + use(a); + + int8 b = 1; + int16 c = 1; + int16 d; + d = c << b; + ret = d; + } + } + "#; + + let ns = generic_target_parse(file); + assert_eq!(count_warnings(&ns.diagnostics), 0); +} + +#[test] +fn statements() { + let file = r#" + contract AddNumbers { function add(uint256 a, uint256 b) external pure returns (uint256 c) {c = b;} } + contract Example { + AddNumbers addContract; + event StringFailure(string stringFailure); + event BytesFailure(bytes bytesFailure); + + function exampleFunction(uint256 _a, uint256 _b) public returns (uint256 _c) { + + try addContract.add(_a, _b) returns (uint256 _value) { + return (_value); + } catch Error(string memory _err) { + // This may occur if there is an overflow with the two numbers and the `AddNumbers` contract explicitly fails with a `revert()` + emit StringFailure(_err); + } catch (bytes memory _err) { + emit BytesFailure(_err); + } + } + + function testFunction() pure public { + int three = 3; + { + int test = 2; + int c = test*3; + + while(c != test) { + c -= three; + } + } + + int four = 4; + int test = 3; + do { + int ct = 2; + } while(four > test); + + } + + function bytesToUInt(uint v) public pure returns (uint ret) { + if (v == 0) { + ret = 0; + } + else { + while (v > 0) { + ret = uint(uint(ret) / (2 ** 8)); + ret |= uint(((v % 10) + 48) * 2 ** (8 * 31)); + v /= 10; + } + } + return ret; + } + + function stringToUint(string s) public pure returns (uint result) { + bytes memory b = bytes(s); + uint i; + result = 0; + for (i = 0; i < b.length; i++) { + uint c = uint(b[i]); + if (c >= 48 && c <= 57) { + result = result * 10 + (c - 48); + } + } + } + } + "#; + let ns = generic_target_parse(file); + assert_eq!(count_warnings(&ns.diagnostics), 2); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "function parameter 'a' has never been read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 'ct' has been assigned, but never read", + )); +} + +#[test] +fn function_call() { + let file = r#" + contract Test1 { + uint32 public a; + + constructor(uint32 b) { + a = b; + } + + } + + contract Test2{ + function test(uint32 v1, uint32 v2) private returns (uint32) { + uint32 v = 1; + Test1 t = new Test1(v); + uint32[2] memory vec = [v2, v1]; + + return vec[0] + t.a(); + } + + function callTest() public { + uint32 ta = 1; + uint32 tb = 2; + + ta = test(ta, tb); + } + } + + contract C { + uint public data; + function x() public returns (uint) { + data = 3; + return this.data(); + } + } + "#; + + let ns = generic_target_parse(file); + assert_eq!(count_warnings(&ns.diagnostics), 0); + + let file = r#" + contract Test1 { + uint32 public a; + + constructor(uint32 b) { + a = b; + } + + } + + contract Test2 is Test1{ + + constructor(uint32 val) Test1(val) {} + + function test(uint32 v1, uint32 v2) private returns (uint32) { + uint32 v = 1; + Test1 t = new Test1(v); + uint32[2] memory vec = [v2, v1]; + + return vec[0] + t.a(); + } + + function callTest() public { + uint32 ta = 1; + uint32 tb = 2; + + ta = test(ta, tb); + } + } + "#; + + let ns = generic_target_parse(file); + assert_eq!(count_warnings(&ns.diagnostics), 0); +} + +#[test] +fn array_push_pop() { + let file = r#" + contract Test1 { + uint32[] vec1; + + function testVec() public { + uint32 a = 1; + uint32 b = 2; + uint32[] memory vec2; + + vec1.push(a); + vec2.push(b); + } + } + "#; + + let ns = generic_target_parse(file); + assert_eq!(count_warnings(&ns.diagnostics), 2); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 'vec2' has been assigned, but never read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "storage variable 'vec1' has been assigned, but never read" + )); + + let file = r#" + contract Test1 { + uint32[] vec1; + + function testVec() public { + uint32 a = 1; + uint32 b = 2; + uint32[] memory vec2; + + vec1.push(a); + vec2.push(b); + vec1.pop(); + vec2.pop(); + } + } + "#; + + let ns = generic_target_parse(file); + assert_eq!(count_warnings(&ns.diagnostics), 0); +} + +#[test] +fn return_variable() { + let file = r#" + contract Test1 { + string testing; + + function test1() public pure returns (uint32 ret, string memory ret2) { + return (2, "Testing is fun"); + } + + function test2() public returns (uint32 hey) { + + (uint32 a, string memory t) = test1(); + testing = t; + + } + } + "#; + + let ns = generic_target_parse(file); + assert_eq!(count_warnings(&ns.diagnostics), 3); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "destructure variable 'a' has never been used" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "return variable 'hey' has never been assigned" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "storage variable 'testing' has been assigned, but never read" + )); +} + +#[test] +fn try_catch() { + let file = r#" + contract CalledContract {} + + contract TryCatcher { + + event SuccessEvent(bool t); + event CatchEvent(bool t); + + function execute() public { + + try new CalledContract() returns(CalledContract returnedInstance) { + emit SuccessEvent(true); + } catch Error(string memory revertReason) { + emit CatchEvent(true); + } catch (bytes memory returnData) { + emit CatchEvent(false); + } + } + } + "#; + + let ns = generic_target_parse(file); + assert_eq!(count_warnings(&ns.diagnostics), 3); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "try-catch error bytes 'returnData' has never been used" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "try-catch returns variable 'returnedInstance' has never been read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "try-catch error string 'revertReason' has never been used" + )); + + let file = r#" + contract CalledContract { + bool public ok = true; + bool public notOk = false; + } + + contract TryCatcher { + + event SuccessEvent(bool t); + event CatchEvent(string t); + event CatchBytes(bytes t); + + function execute() public { + + try new CalledContract() returns(CalledContract returnedInstance) { + // returnedInstance can be used to obtain the address of the newly deployed contract + emit SuccessEvent(returnedInstance.ok()); + } catch Error(string memory revertReason) { + emit CatchEvent(revertReason); + } catch (bytes memory returnData) { + emit CatchBytes(returnData); + } + } + } + "#; + + let ns = generic_target_parse(file); + assert_eq!(count_warnings(&ns.diagnostics), 1); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "storage variable 'notOk' has been assigned, but never read" + )); +} + +#[test] +fn destructure() { + let file = r#" + contract Test2{ + + function callTest() public view returns (uint32 ret) { + uint32 ta = 1; + uint32 tb = 2; + uint32 te = 3; + + string memory tc = "hey"; + bytes memory td = bytes(tc); + address nameReg = address(this); + (bool tf,) = nameReg.call(td); + + + ta = tf? tb : te; + uint8 tg = 1; + uint8 th = 2; + (tg, th) = (th, tg); + return ta; + } + } + "#; + + let ns = generic_target_parse(file); + assert_eq!(count_warnings(&ns.diagnostics), 0); +} + +#[test] +fn struct_initialization() { + let file = r#" + contract Test1{ + struct Test2{ + uint8 a; + uint8 b; + } + + function callTest() public pure returns (uint32 ret) { + uint8 tg = 1; + uint8 th = 2; + + Test2 memory t2; + t2 = Test2(tg, th); + ret = t2.a; + } + } + "#; + + let ns = generic_target_parse(file); + assert_eq!(count_warnings(&ns.diagnostics), 0); +} + +#[test] +fn subarray_mapping_struct_literal() { + let file = r#" + contract T { + int p; + constructor(int b) { + p = b; + } + + function sum(int a, int b) virtual public returns (int){ + uint8 v1 = 1; + uint8 v2 = 2; + uint8 v3 = 3; + uint8 v4 = 4; + uint8[2][2] memory v = [[v1, v2], [v3, v4]]; + return a + b * p/v[0][1]; + } + } + + contract Test is T(2){ + + struct fooStruct { + int foo; + int figther; + } + mapping(string => int) public mp; + enum FreshJuiceSize{ SMALL, MEDIUM, LARGE } + FreshJuiceSize choice; + + function sum(int a, int b) override public returns (int) { + choice = FreshJuiceSize.LARGE; + return a*b; + } + + function test() public returns (int){ + int a = 1; + int b = 2; + int c = super.sum(a, b); + int d = 3; + fooStruct memory myStruct = fooStruct({foo: c, figther: d}); + string memory t = "Do some tests"; + mp[t] = myStruct.figther; + return mp[t]; + } + } + "#; + + let ns = generic_target_parse(file); + assert_eq!(count_warnings(&ns.diagnostics), 1); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "storage variable 'choice' has been assigned, but never read" + )); +}