Raise for already used arg name in function definition

clar2wasm/src/error_mapping.rs

@@ -3,6 +3,8 @@ use clarity::vm::types::ResponseData;
 use clarity::vm::Value;
 use wasmtime::{AsContextMut, Instance, Trap};
 
+use crate::wasm_utils::read_identifier_from_wasm;
+
 const LOG2_ERROR_MESSAGE: &str = "log2 must be passed a positive integer";
 const SQRTI_ERROR_MESSAGE: &str = "sqrti must be passed a positive integer";
 const POW_ERROR_MESSAGE: &str = "Power argument to (pow ...) must be a u32 integer";
@@ -18,6 +20,7 @@ pub enum ErrorMap {
     Panic = 6,
     ShortReturnAssertionFailure = 7,
     ArithmeticPowError = 8,
+    NameAlreadyUsed = 9,
     NotMapped = 99,
 }
 
@@ -34,6 +37,7 @@ impl From<i32> for ErrorMap {
             6 => ErrorMap::Panic,
             7 => ErrorMap::ShortReturnAssertionFailure,
             8 => ErrorMap::ArithmeticPowError,
+            9 => ErrorMap::NameAlreadyUsed,
             _ => ErrorMap::NotMapped,
         }
     }
@@ -156,6 +160,34 @@ fn from_runtime_error_code(
             RuntimeErrorType::Arithmetic(POW_ERROR_MESSAGE.into()),
             Some(Vec::new()),
         ),
+        ErrorMap::NameAlreadyUsed => {
+            let runtime_error_arg_offset = instance
+                .get_global(&mut store, "runtime-error-arg-offset")
+                .and_then(|glob| glob.get(&mut store).i32())
+                .unwrap_or_else(|| {
+                    panic!("Could not find $runtime-error-arg-offset global with i32 value")
+                });
+
+            let runtime_error_arg_len = instance
+                .get_global(&mut store, "runtime-error-arg-len")
+                .and_then(|glob| glob.get(&mut store).i32())
+                .unwrap_or_else(|| {
+                    panic!("Could not find $runtime-error-arg-len global with i32 value")
+                });
+
+            let memory = instance
+                .get_memory(&mut store, "memory")
+                .unwrap_or_else(|| panic!("Could not find wasm instance memory"));
+            let arg_name = read_identifier_from_wasm(
+                memory,
+                &mut store,
+                runtime_error_arg_offset,
+                runtime_error_arg_len,
+            )
+            .unwrap_or_else(|e| panic!("Could not recover arg_name: {e}"));
+
+            Error::Unchecked(CheckErrors::NameAlreadyUsed(arg_name))
+        }
         _ => panic!("Runtime error code {} not supported", runtime_error_code),
     }
 }

clar2wasm/src/linker.rs

@@ -3603,12 +3603,12 @@ fn link_print_fn(linker: &mut Linker<ClarityWasmContext>) -> Result<(), Error> {
                     .and_then(|export| export.into_memory())
                     .ok_or(Error::Wasm(WasmError::MemoryNotFound))?;
 
-                let serialized_ty = String::from_utf8(read_bytes_from_wasm(
+                let serialized_ty = read_identifier_from_wasm(
                     memory,
                     &mut caller,
                     serialized_ty_offset,
                     serialized_ty_length,
-                )?)?;
+                )?;
 
                 let epoch = caller.data().global_context.epoch_id;
                 let version = caller.data().contract_context().get_clarity_version();

clar2wasm/src/standard/standard.wat

@@ -256,6 +256,8 @@
     ;; Global definitions
     (global $stack-pointer (mut i32) (i32.const 0))
     (global $runtime-error-code (mut i32) (i32.const -1))
+    (global $runtime-error-arg-offset (mut i32) (i32.const -1))
+    (global $runtime-error-arg-len (mut i32) (i32.const -1))
 
     ;; (sha256) initial hash values: first 32 bits of the fractional parts of the square roots of the first 8 primes 2..19
     (data (i32.const 0) "\67\e6\09\6a\85\ae\67\bb\72\f3\6e\3c\3a\f5\4f\a5\7f\52\0e\51\8c\68\05\9b\ab\d9\83\1f\19\cd\e0\5b")
@@ -3880,6 +3882,8 @@
     ;; Globals
     (export "stack-pointer" (global $stack-pointer))
     (export "runtime-error-code" (global $runtime-error-code))
+    (export "runtime-error-arg-offset" (global $runtime-error-arg-offset))
+    (export "runtime-error-arg-len" (global $runtime-error-arg-len))
 
     ;; Functions
     (export "stdlib.add-uint" (func $stdlib.add-uint))

clar2wasm/src/tools.rs

@@ -564,13 +564,17 @@ pub fn crosscheck_expect_failure(snippet: &str) {
     let compiled = evaluate(snippet);
     let interpreted = interpret(snippet);
 
-    assert_eq!(
-        compiled.is_err(),
+    assert!(
         interpreted.is_err(),
-        "Compiled and interpreted results diverge! {}\ncompiled: {:?}\ninterpreted: {:?}",
+        "Interpreted didn't err: {}\ninterpreted: {:?}",
+        snippet,
+        &interpreted,
+    );
+    assert!(
+        compiled.is_err(),
+        "Compiled didn't err: {}\ncompiled: {:?}",
         snippet,
         &compiled,
-        &interpreted
     );
 }
 

clar2wasm/src/wasm_generator.rs

@@ -5,7 +5,7 @@ use clarity::vm::analysis::ContractAnalysis;
 use clarity::vm::diagnostic::DiagnosableError;
 use clarity::vm::types::signatures::{CallableSubtype, StringUTF8Length, BUFF_1};
 use clarity::vm::types::{
-    CharType, FixedFunction, FunctionType, PrincipalData, SequenceData, SequenceSubtype,
+    ASCIIData, CharType, FixedFunction, FunctionType, PrincipalData, SequenceData, SequenceSubtype,
     StringSubtype, TypeSignature,
 };
 use clarity::vm::variables::NativeVariables;
@@ -80,6 +80,10 @@ impl Bindings {
         self.0.insert(name, InnerBindings { locals, ty });
     }
 
+    pub(crate) fn contains(&mut self, name: &ClarityName) -> bool {
+        self.0.contains_key(name)
+    }
+
     pub(crate) fn get_locals(&self, name: &ClarityName) -> Option<&[LocalId]> {
         self.0.get(name).map(|b| b.locals.as_slice())
     }
@@ -248,6 +252,22 @@ pub fn type_from_sequence_element(se: &SequenceElementType) -> TypeSignature {
     }
 }
 
+fn get_global(module: &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}"))
+        })
+}
+
 impl WasmGenerator {
     pub fn new(contract_analysis: ContractAnalysis) -> Result<WasmGenerator, GeneratorError> {
         let standard_lib_wasm: &[u8] = include_bytes!("standard/standard.wasm");
@@ -256,22 +276,7 @@ impl WasmGenerator {
             GeneratorError::InternalError("failed to load standard library".to_owned())
         })?;
         // Get the stack-pointer global ID
-        let stack_pointer_name = "stack-pointer";
-        let global_id = module
-            .globals
-            .iter()
-            .find(|global| {
-                global
-                    .name
-                    .as_ref()
-                    .map_or(false, |name| name == stack_pointer_name)
-            })
-            .map(|global| global.id())
-            .ok_or_else(|| {
-                GeneratorError::InternalError(
-                    "Expected to find a global named $stack-pointer".to_owned(),
-                )
-            })?;
+        let global_id = get_global(&module, "stack-pointer")?;
 
         Ok(WasmGenerator {
             contract_analysis,
@@ -497,7 +502,13 @@ impl WasmGenerator {
         // Setup the parameters
         let mut param_locals = Vec::new();
         let mut params_types = Vec::new();
+        let mut reused_arg = None;
         for param in function_type.args.iter() {
+            // Interpreter returns the first reused arg as NameAlreadyUsed argument
+            if reused_arg.is_none() && bindings.contains(&param.name) {
+                reused_arg = Some(param.name.clone());
+            }
+
             let param_types = clar2wasm_ty(&param.signature);
             let mut plocals = Vec::with_capacity(param_types.len());
             for ty in param_types {
@@ -542,6 +553,28 @@ impl WasmGenerator {
         self.set_expr_type(body, function_type.returns.clone())?;
         self.traverse_expr(&mut block, body)?;
 
+        // If the same arg name is used multiple times, the interpreter throws an
+        // `Unchecked` error at runtime, so we do the same here
+        if let Some(arg_name) = reused_arg {
+            let (arg_name_offset, arg_name_len) =
+                self.add_clarity_string_literal(&CharType::ASCII(ASCIIData {
+                    data: arg_name.as_bytes().to_vec(),
+                }))?;
+
+            // Clear function body
+            block.instrs_mut().clear();
+
+            block
+                .i32_const(arg_name_offset as i32)
+                .global_set(get_global(&self.module, "runtime-error-arg-offset")?)
+                .i32_const(arg_name_len as i32)
+                .global_set(get_global(&self.module, "runtime-error-arg-len")?)
+                .i32_const(ErrorMap::NameAlreadyUsed as i32)
+                .call(self.func_by_name("stdlib.runtime-error"))
+                // To avoid having to generate correct return values
+                .unreachable();
+        }
+
         // Insert the function body block into the function
         func_body.instr(walrus::ir::Block { seq: block_id });
 

clar2wasm/src/words/functions.rs

@@ -112,9 +112,13 @@ impl ComplexWord for DefinePublicFunction {
 #[cfg(test)]
 mod tests {
     use clarity::types::StacksEpochId;
+    use clarity::vm::errors::{CheckErrors, Error};
     use clarity::vm::Value;
 
-    use crate::tools::{crosscheck, crosscheck_expect_failure, crosscheck_with_epoch, evaluate};
+    use crate::tools::{
+        crosscheck, crosscheck_expect_failure, crosscheck_multi_contract, crosscheck_with_epoch,
+        evaluate,
+    };
 
     #[test]
     fn top_level_define_first() {
@@ -340,4 +344,45 @@ mod tests {
         crosscheck_expect_failure("(define-read-only (element-at?) (ok u0))");
         crosscheck_expect_failure("(define-read-only (element-at) (ok u0))");
     }
+
+    #[test]
+    fn reuse_arg_name() {
+        let snippet = "
+(define-private (foo (a int) (a int) (b int) (b int)) 1)
+(define-private (bar (c int) (d int) (e int) (d int)) 1)
+";
+        crosscheck(
+            &format!("{snippet} (foo 1 2 3 4)"),
+            Err(Error::Unchecked(CheckErrors::NameAlreadyUsed(
+                "a".to_string(),
+            ))),
+        );
+        crosscheck(
+            &format!("{snippet} (bar 1 2 3 4)"),
+            Err(Error::Unchecked(CheckErrors::NameAlreadyUsed(
+                "d".to_string(),
+            ))),
+        );
+    }
+
+    #[test]
+    fn reuse_arg_name_contrac_call() {
+        let first_contract_name = "callee".into();
+        let first_snippet = "
+(define-public (foo (a int) (a int) (b int) (b int)) (ok 1))
+";
+
+        let second_contract_name = "caller".into();
+        let second_snippet = format!(r#"(contract-call? .{first_contract_name} foo 1 2 3 4)"#);
+
+        crosscheck_multi_contract(
+            &[
+                (first_contract_name, first_snippet),
+                (second_contract_name, &second_snippet),
+            ],
+            Err(Error::Unchecked(CheckErrors::NameAlreadyUsed(
+                "a".to_string(),
+            ))),
+        );
+    }
 }

clar2wasm/tests/wasm-generation/control_flow.rs

@@ -100,12 +100,10 @@ proptest! {
 
     #[test]
     fn begin((expr, expected_val, is_response_intermediary) in begin_strategy()) {
-        let _expected_val: Result<Option<Value>, ()> = if is_response_intermediary {
-            Err(())
-        } else{
-            Ok(Some(expected_val.into()))
+        if is_response_intermediary {
+            crosscheck_expect_failure(&expr);
+        } else {
+            crosscheck(&expr, Ok(Some(expected_val.into())));
         };
-
-        crosscheck_expect_failure(&expr);
     }
 }