Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Checks for the Length of Arguments List #533

Merged
merged 23 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
9ca3149
feat: added checks for the length of arguments list
BowTiedWoo Oct 14, 2024
43ecb90
feat: add error variant for invalid argument length
BowTiedWoo Oct 14, 2024
117e02b
feat: add tests for length of args list
BowTiedWoo Oct 14, 2024
2cdaa75
Merge branch 'main' into feat/check-args-length
BowTiedWoo Oct 15, 2024
4c8655d
feat: add runtime error for arg length and update tests
BowTiedWoo Oct 18, 2024
c0e0fef
Merge branch 'main' into feat/check-args-length
BowTiedWoo Oct 18, 2024
552f79f
feat: modify arg checks and arg count errors
BowTiedWoo Oct 18, 2024
4866a0e
fix: remove code duplications
BowTiedWoo Oct 18, 2024
3f893c1
fix: add functions for checking arg length
BowTiedWoo Oct 18, 2024
fe2af84
fix: refactor to remove code duplication
BowTiedWoo Oct 21, 2024
db6ae8f
fix: update tests
BowTiedWoo Oct 21, 2024
ea22f47
Merge branch 'main' into feat/check-args-length
BowTiedWoo Oct 21, 2024
40dacf7
chore: remove prints
BowTiedWoo Oct 22, 2024
a1c8206
chore: add comments on tests where typechecker fails
BowTiedWoo Oct 22, 2024
01bd418
Merge branch 'main' into feat/check-args-length
BowTiedWoo Oct 22, 2024
8be2284
Merge branch 'main' into feat/check-args-length
BowTiedWoo Oct 22, 2024
e0c678e
fix: update tests to use crosscheck
BowTiedWoo Oct 22, 2024
6bf5b05
fix: format code
BowTiedWoo Oct 23, 2024
90bfb36
fix: remove prints
BowTiedWoo Oct 23, 2024
b415867
feat: short-cut traverse function
BowTiedWoo Oct 23, 2024
5c2afe2
feat: add arg count check for user defined fns
BowTiedWoo Oct 24, 2024
e8936cc
Merge branch 'main' into feat/check-args-length
BowTiedWoo Oct 24, 2024
341ec46
feat: read/write arg lengths from/to memory as bytes
BowTiedWoo Oct 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 76 additions & 1 deletion clar2wasm/src/error_mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use clarity::vm::{ClarityVersion, Value};
use wasmtime::{AsContextMut, Instance, Trap};

use crate::wasm_utils::{
read_from_wasm_indirect, read_identifier_from_wasm, signature_from_string,
read_bytes_from_wasm, read_from_wasm_indirect, read_identifier_from_wasm, signature_from_string,
};

const LOG2_ERROR_MESSAGE: &str = "log2 must be passed a positive integer";
Expand Down Expand Up @@ -69,6 +69,15 @@ pub enum ErrorMap {
/// usually triggered by `(unwrap!...)` and `(unwrap-err!...)`.
ShortReturnExpectedValue = 12,

/// Indicates an attempt to use a function with the wrong amount of arguments
ArgumentCountMismatch = 13,

/// Indicates an attempt to use a function with too few arguments
ArgumentCountAtLeast = 14,

/// Indicates an attempt to use a function with too many arguments
ArgumentCountAtMost = 15,

/// A catch-all for errors that are not mapped to specific error codes.
/// This might be used for unexpected or unclassified errors.
NotMapped = 99,
Expand All @@ -91,6 +100,9 @@ impl From<i32> for ErrorMap {
10 => ErrorMap::ShortReturnExpectedValueResponse,
11 => ErrorMap::ShortReturnExpectedValueOptional,
12 => ErrorMap::ShortReturnExpectedValue,
13 => ErrorMap::ArgumentCountMismatch,
14 => ErrorMap::ArgumentCountAtLeast,
15 => ErrorMap::ArgumentCountAtMost,
_ => ErrorMap::NotMapped,
}
}
Expand Down Expand Up @@ -258,6 +270,18 @@ fn from_runtime_error_code(
let clarity_val = short_return_value(&instance, &mut store, epoch_id, clarity_version);
Error::ShortReturn(ShortReturnType::ExpectedValue(clarity_val))
}
ErrorMap::ArgumentCountMismatch => {
let (expected, got) = get_runtime_error_arg_lengths(&instance, &mut store);
Error::Unchecked(CheckErrors::IncorrectArgumentCount(expected, got))
}
ErrorMap::ArgumentCountAtLeast => {
let (expected, got) = get_runtime_error_arg_lengths(&instance, &mut store);
Error::Unchecked(CheckErrors::RequiresAtLeastArguments(expected, got))
}
ErrorMap::ArgumentCountAtMost => {
let (expected, got) = get_runtime_error_arg_lengths(&instance, &mut store);
Error::Unchecked(CheckErrors::RequiresAtMostArguments(expected, got))
}
_ => panic!("Runtime error code {} not supported", runtime_error_code),
}
}
Expand All @@ -279,6 +303,28 @@ fn get_global_i32(instance: &Instance, store: &mut impl AsContextMut, name: &str
.unwrap_or_else(|| panic!("Could not find ${} global with i32 value", name))
}

/// Retrieves the expected and actual argument counts from a byte-encoded string.
///
/// This function interprets a string as a sequence of bytes, where the first 4 bytes
/// represent the expected number of arguments, and the bytes at positions 16 to 19
/// represent the actual number of arguments received. It converts these byte sequences
/// into `usize` values and returns them as a tuple.
///
/// # Returns
///
/// A tuple `(expected, got)` where:
/// - `expected` is the number of arguments expected.
/// - `got` is the number of arguments actually received.
fn extract_expected_and_got(bytes: &[u8]) -> (usize, usize) {
// Assuming the first 4 bytes represent the expected value
let expected = u32::from_le_bytes([bytes[0], bytes[1], bytes[2], bytes[3]]) as usize;

// Assuming the next 4 bytes represent the got value
let got = u32::from_le_bytes([bytes[4], bytes[5], bytes[6], bytes[7]]) as usize;

(expected, got)
}

/// Retrieves and deserializes a Clarity value from WebAssembly memory in the context of a short return.
///
/// This function is used to extract a Clarity value that has been stored in WebAssembly memory
Expand Down Expand Up @@ -312,3 +358,32 @@ fn short_return_value(
read_from_wasm_indirect(memory, store, &value_ty, val_offset, *epoch_id)
.unwrap_or_else(|e| panic!("Could not read thrown value from memory: {}", e))
}

/// Retrieves the argument lengths from the runtime error global variables.
///
/// This function reads the global variables `runtime-error-arg-offset` and `runtime-error-arg-len`
/// from the WebAssembly instance and constructs a string representing the argument lengths.
///
/// # Returns
///
/// A string representing the argument lengths.
fn get_runtime_error_arg_lengths(
instance: &Instance,
store: &mut impl AsContextMut,
) -> (usize, usize) {
let runtime_error_arg_offset = get_global_i32(instance, store, "runtime-error-arg-offset");
let runtime_error_arg_len = get_global_i32(instance, store, "runtime-error-arg-len");

let memory = instance
.get_memory(&mut *store, "memory")
.unwrap_or_else(|| panic!("Could not find wasm instance memory"));
let arg_lengths = read_bytes_from_wasm(
memory,
store,
runtime_error_arg_offset,
runtime_error_arg_len,
)
.unwrap_or_else(|e| panic!("Could not recover arg_lengths: {e}"));

extract_expected_and_got(&arg_lengths)
}
40 changes: 36 additions & 4 deletions clar2wasm/src/wasm_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ use walrus::{

use crate::error_mapping::ErrorMap;
use crate::wasm_utils::{
get_type_in_memory_size, get_type_size, is_in_memory_type, signature_from_string,
check_argument_count, get_type_in_memory_size, get_type_size, is_in_memory_type,
signature_from_string, ArgumentCountCheck,
};
use crate::{debug_msg, words};
use crate::{check_args, debug_msg, words};

// First free position after data directly defined in standard.wat
pub const END_OF_STANDARD_DATA: u32 = 1352;
Expand Down Expand Up @@ -115,6 +116,7 @@ pub enum GeneratorError {
NotImplemented,
InternalError(String),
TypeError(String),
ArgumentCountMismatch,
}

pub enum FunctionKind {
Expand All @@ -129,6 +131,7 @@ impl DiagnosableError for GeneratorError {
GeneratorError::NotImplemented => "Not implemented".to_string(),
GeneratorError::InternalError(msg) => format!("Internal error: {}", msg),
GeneratorError::TypeError(msg) => format!("Type error: {}", msg),
GeneratorError::ArgumentCountMismatch => "Argument count mismatch".to_string(),
}
}

Expand Down Expand Up @@ -1669,11 +1672,18 @@ impl WasmGenerator {
if let Some(FunctionType::Fixed(FixedFunction {
args: function_args,
..
})) = self.get_function_type(name)
})) = self.get_function_type(name).cloned()
{
check_args!(
self,
builder,
function_args.len(),
args.len(),
ArgumentCountCheck::Exact
);
for (arg, signature) in args
.iter()
.zip(function_args.clone().into_iter().map(|a| a.signature))
.zip(function_args.into_iter().map(|a| a.signature))
{
self.set_expr_type(arg, signature)?;
}
Expand Down Expand Up @@ -1980,6 +1990,7 @@ mod tests {
use clarity::vm::analysis::AnalysisDatabase;
use clarity::vm::costs::LimitedCostTracker;
use clarity::vm::database::MemoryBackingStore;
use clarity::vm::errors::{CheckErrors, Error};
use clarity::vm::types::{QualifiedContractIdentifier, StandardPrincipalData};
use clarity::vm::ClarityVersion;
use walrus::Module;
Expand Down Expand Up @@ -2123,6 +2134,27 @@ mod tests {
);
}

#[test]
fn function_has_correct_argument_count() {
// TODO: see issue #488
// The inconsistency in function arguments should have been caught by the typechecker.
// The runtime error below is being used as a workaround for a typechecker issue
// where certain errors are not properly handled.
// This test should be re-worked once the typechecker is fixed
// and can correctly detect all argument inconsistencies.
crosscheck(
"
(define-public (foo (arg int))
(ok true))
(foo 1 2)
(define-public (bar (arg int))
(ok true))
(bar)
",
Err(Error::Unchecked(CheckErrors::IncorrectArgumentCount(1, 2))),
);
}

//
// Module with tests that should only be executed
// when running Clarity::V2 or Clarity::v3.
Expand Down
83 changes: 82 additions & 1 deletion clar2wasm/src/wasm_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ use clarity::vm::types::{
};
use clarity::vm::{CallStack, ClarityVersion, ContractContext, ContractName, Value};
use stacks_common::types::StacksEpochId;
use walrus::{GlobalId, InstrSeqBuilder};
use wasmtime::{AsContextMut, Engine, Linker, Memory, Module, Store, Val, ValType};

use crate::error_mapping;
use crate::error_mapping::{self, ErrorMap};
use crate::initialize::ClarityWasmContext;
use crate::linker::link_host_functions;
use crate::wasm_generator::{GeneratorError, WasmGenerator};

#[allow(non_snake_case)]
pub enum MintAssetErrorCodes {
Expand Down Expand Up @@ -1642,3 +1644,82 @@ pub fn signature_from_string(
&mut (),
)?)
}

pub fn get_global(module: &walrus::Module, name: &str) -> Result<GlobalId, GeneratorError> {
module
.globals
.iter()
.find(|global| {
global
.name
.as_ref()
.map_or(false, |other_name| name == other_name)
})
.map(|global| global.id())
.ok_or_else(|| {
GeneratorError::InternalError(format!("Expected to find a global named ${name}"))
})
}

pub enum ArgumentCountCheck {
Exact,
AtLeast,
AtMost,
}

pub fn check_argument_count(
generator: &mut WasmGenerator,
builder: &mut InstrSeqBuilder,
expected: usize,
actual: usize,
check: ArgumentCountCheck,
) -> Result<(), GeneratorError> {
let expected = expected as u32;
let actual = actual as u32;
let mut handle_mismatch = |error_map: ErrorMap| -> Result<(), GeneratorError> {
let (arg_name_offset_start, arg_name_len_expected) =
generator.add_bytes_literal(&expected.to_le_bytes())?;
let (_, arg_name_len_got) = generator.add_bytes_literal(&actual.to_le_bytes())?;
builder
.i32_const(arg_name_offset_start as i32)
.global_set(get_global(&generator.module, "runtime-error-arg-offset")?)
.i32_const((arg_name_len_expected + arg_name_len_got) as i32)
.global_set(get_global(&generator.module, "runtime-error-arg-len")?)
.i32_const(error_map as i32)
.call(generator.func_by_name("stdlib.runtime-error"));
Ok(())
};

match check {
ArgumentCountCheck::Exact => {
if expected != actual {
handle_mismatch(ErrorMap::ArgumentCountMismatch)?;
return Err(GeneratorError::ArgumentCountMismatch);
}
}
ArgumentCountCheck::AtLeast => {
if expected > actual {
handle_mismatch(ErrorMap::ArgumentCountAtLeast)?;
return Err(GeneratorError::ArgumentCountMismatch);
}
}
ArgumentCountCheck::AtMost => {
if expected < actual {
handle_mismatch(ErrorMap::ArgumentCountAtMost)?;
return Err(GeneratorError::ArgumentCountMismatch);
}
}
}
Ok(())
}

#[macro_export]
macro_rules! check_args {
($generator:expr, $builder:expr, $expected:expr, $actual:expr, $check:expr) => {
if check_argument_count($generator, $builder, $expected, $actual, $check).is_err() {
// short cutting traverse functions
$builder.unreachable();
return Ok(());
}
};
}
22 changes: 21 additions & 1 deletion clar2wasm/src/words/bindings.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use clarity::vm::{ClarityName, SymbolicExpression};

use crate::check_args;
use crate::wasm_generator::{ArgumentsExt, GeneratorError, WasmGenerator};
use crate::wasm_utils::{check_argument_count, ArgumentCountCheck};
use crate::words::ComplexWord;

#[derive(Debug)]
Expand All @@ -18,6 +20,14 @@ impl ComplexWord for Let {
_expr: &SymbolicExpression,
args: &[SymbolicExpression],
) -> Result<(), GeneratorError> {
check_args!(
generator,
builder,
2,
args.len(),
ArgumentCountCheck::AtLeast
);

let bindings = args.get_list(0)?;

// Save the current named locals
Expand Down Expand Up @@ -80,7 +90,7 @@ impl ComplexWord for Let {
mod tests {
use clarity::vm::Value;

use crate::tools::{crosscheck, crosscheck_compare_only, crosscheck_expect_failure};
use crate::tools::{crosscheck, crosscheck_compare_only, crosscheck_expect_failure, evaluate};

#[cfg(feature = "test-clarity-v1")]
mod clarity_v1 {
Expand All @@ -100,6 +110,16 @@ mod tests {
}
}

#[test]
fn let_less_than_two_args() {
let result = evaluate("(let ((current-count (count u1))))");
assert!(result.is_err());
assert!(result
.unwrap_err()
.to_string()
.contains("expecting >= 2 arguments, got 1"));
}

#[test]
fn clar_let_disallow_builtin_names() {
// It's not allowed to use names of user-defined functions as bindings
Expand Down
Loading