diff --git a/crates/llvm-context/src/polkavm/context/mod.rs b/crates/llvm-context/src/polkavm/context/mod.rs index b5febe9..53a6b7b 100644 --- a/crates/llvm-context/src/polkavm/context/mod.rs +++ b/crates/llvm-context/src/polkavm/context/mod.rs @@ -1,6 +1,7 @@ //! The LLVM IR generator context. use std::cell::RefCell; +use std::collections::BTreeMap; use std::collections::HashMap; use std::rc::Rc; @@ -84,6 +85,10 @@ pub struct Context<'ctx> { current_function: Option>>>, /// The loop context stack. loop_stack: Vec>, + /// Monotonic counter for unique frontend function name mangling. + function_counter: usize, + /// Scope stack mapping Yul names to mangled LLVM names. + function_scope: Vec>, /// The PVM memory configuration. memory_config: SolcStandardJsonInputSettingsPolkaVMMemory, @@ -246,6 +251,8 @@ impl<'ctx> Context<'ctx> { functions: HashMap::with_capacity(Self::FUNCTIONS_HASHMAP_INITIAL_CAPACITY), current_function: None, loop_stack: Vec::with_capacity(Self::LOOP_STACK_INITIAL_CAPACITY), + function_counter: 0, + function_scope: Vec::new(), memory_config, debug_info, @@ -446,6 +453,12 @@ impl<'ctx> Context<'ctx> { &self.llvm_runtime } + pub fn get_current_scope(&mut self) -> &mut BTreeMap { + self.function_scope + .last_mut() + .expect("ICE: function scope must be pushed before declaring frontend functions") + } + /// Appends a function to the current module. pub fn add_function( &mut self, @@ -456,12 +469,20 @@ impl<'ctx> Context<'ctx> { location: Option<(u32, u32)>, is_frontend: bool, ) -> anyhow::Result>>> { - assert!( - self.get_function(name, is_frontend).is_none(), - "ICE: function '{name}' declared subsequentally" - ); - - let name = self.internal_function_name(name, is_frontend); + let name = if is_frontend { + assert!( + !self.get_current_scope().contains_key(name), + "ICE: function '{name}' declared subsequently in the same scope" + ); + let counter = self.function_counter; + self.function_counter += 1; + let mangled = format!("{name}_{}__{counter}", self.code_type().unwrap()); + self.get_current_scope() + .insert(name.to_string(), mangled.clone()); + mangled + } else { + name.to_string() + }; let value = self.module().add_function(&name, r#type, linkage); if self.debug_info().is_some() { @@ -524,9 +545,16 @@ impl<'ctx> Context<'ctx> { name: &str, is_frontend: bool, ) -> Option>>> { - self.functions - .get(&self.internal_function_name(name, is_frontend)) - .cloned() + if is_frontend { + let mangled = self + .function_scope + .iter() + .rev() + .find_map(|scope| scope.get(name))?; + self.functions.get(mangled).cloned() + } else { + self.functions.get(name).cloned() + } } /// Returns a shared reference to the current active function. @@ -654,6 +682,18 @@ impl<'ctx> Context<'ctx> { .expect("The current context is not in a loop") } + /// Pushes a new function scope. + pub fn push_function_scope(&mut self) { + self.function_scope.push(BTreeMap::new()); + } + + /// Pops the current function scope. + pub fn pop_function_scope(&mut self) { + self.function_scope + .pop() + .expect("ICE: tried to pop an empty function scope stack"); + } + /// Returns the debug info. pub fn debug_info(&self) -> Option<&DebugInfo<'ctx>> { self.debug_info.as_ref() @@ -1429,15 +1469,6 @@ impl<'ctx> Context<'ctx> { ) } - /// Returns the internal function name. - fn internal_function_name(&self, name: &str, is_frontend: bool) -> String { - if is_frontend { - format!("{name}_{}", self.code_type().unwrap()) - } else { - name.to_string() - } - } - /// Scans all functions in the module and removes the `MinSize` attribute /// if the function contains any large sdiv, udiv, srem, urem instructions with either unknown /// NOTE: The check here could be relaxed by checking denominator: if the denominator is diff --git a/crates/resolc/src/cli_utils.rs b/crates/resolc/src/cli_utils.rs index a34ea81..f0ab50e 100644 --- a/crates/resolc/src/cli_utils.rs +++ b/crates/resolc/src/cli_utils.rs @@ -24,6 +24,12 @@ pub const YUL_MEMSET_CONTRACT_PATH: &str = "src/tests/data/yul/memset.yul"; pub const YUL_RETURN_CONTRACT_PATH: &str = "src/tests/data/yul/return.yul"; /// The invalid YUL contract test fixture path (hex literal with odd number of nibbles). pub const YUL_INVALID_HEX_NIBBLES_PATH: &str = "src/tests/data/yul/invalid_hex_nibbles.yul"; +/// Yul contract with duplicate function names in switch cases. +pub const YUL_DUPLICATE_FUNCTIONS_SWITCH_PATH: &str = + "src/tests/data/yul/duplicate_functions_switch.yul"; +/// Yul contract with duplicate function names in deeply nested switch cases. +pub const YUL_DUPLICATE_FUNCTIONS_DEEP_NESTING_PATH: &str = + "src/tests/data/yul/duplicate_functions_deep_nesting.yul"; /// The standard JSON contracts test fixture path. pub const STANDARD_JSON_CONTRACTS_PATH: &str = diff --git a/crates/resolc/src/tests/cli/yul.rs b/crates/resolc/src/tests/cli/yul.rs index ffd7463..13bf03d 100644 --- a/crates/resolc/src/tests/cli/yul.rs +++ b/crates/resolc/src/tests/cli/yul.rs @@ -2,7 +2,9 @@ use crate::cli_utils::{ assert_command_failure, assert_command_success, assert_equal_exit_codes, execute_resolc, - execute_solc, RESOLC_YUL_FLAG, SOLC_YUL_FLAG, YUL_CONTRACT_PATH, YUL_INVALID_HEX_NIBBLES_PATH, + execute_solc, RESOLC_YUL_FLAG, SOLC_YUL_FLAG, YUL_CONTRACT_PATH, + YUL_DUPLICATE_FUNCTIONS_DEEP_NESTING_PATH, YUL_DUPLICATE_FUNCTIONS_SWITCH_PATH, + YUL_INVALID_HEX_NIBBLES_PATH, }; #[test] @@ -35,3 +37,29 @@ fn bails_with_invalid_input_file() { let solc_result = execute_solc(&[YUL_INVALID_HEX_NIBBLES_PATH, SOLC_YUL_FLAG]); assert_equal_exit_codes(&solc_result, &resolc_result); } + +#[test] +fn duplicate_functions_in_switch_cases() { + let resolc_result = execute_resolc(&[ + YUL_DUPLICATE_FUNCTIONS_SWITCH_PATH, + RESOLC_YUL_FLAG, + "--bin", + ]); + assert_command_success( + &resolc_result, + "Duplicate function names in different switch cases", + ); +} + +#[test] +fn duplicate_functions_deep_nesting() { + let resolc_result = execute_resolc(&[ + YUL_DUPLICATE_FUNCTIONS_DEEP_NESTING_PATH, + RESOLC_YUL_FLAG, + "--bin", + ]); + assert_command_success( + &resolc_result, + "Duplicate function names in deeply nested switch cases", + ); +} diff --git a/crates/resolc/src/tests/data/yul/duplicate_functions_deep_nesting.yul b/crates/resolc/src/tests/data/yul/duplicate_functions_deep_nesting.yul new file mode 100644 index 0000000..ad5dc99 --- /dev/null +++ b/crates/resolc/src/tests/data/yul/duplicate_functions_deep_nesting.yul @@ -0,0 +1,58 @@ +object "Test" { + code { + { + let size := datasize("Test_deployed") + codecopy(0, dataoffset("Test_deployed"), size) + return(0, size) + } + } + object "Test_deployed" { + code { + { + switch calldataload(0) + case 0 { + function f() -> ret { + ret := 1 + } + + switch calldataload(32) + case 0 { + function g() -> ret { + ret := 10 + } + mstore(0, add(f(), g())) + return(0, 32) + } + case 1 { + function g() -> ret { + ret := 20 + } + mstore(0, add(f(), g())) + return(0, 32) + } + } + case 1 { + function f() -> ret { + ret := 2 + } + + switch calldataload(32) + case 0 { + function g() -> ret { + ret := 30 + } + mstore(0, add(f(), g())) + return(0, 32) + } + case 1 { + function g() -> ret { + ret := 40 + } + mstore(0, add(f(), g())) + return(0, 32) + } + } + } + } + } +} diff --git a/crates/resolc/src/tests/data/yul/duplicate_functions_switch.yul b/crates/resolc/src/tests/data/yul/duplicate_functions_switch.yul new file mode 100644 index 0000000..184678f --- /dev/null +++ b/crates/resolc/src/tests/data/yul/duplicate_functions_switch.yul @@ -0,0 +1,30 @@ +object "Test" { + code { + { + let size := datasize("Test_deployed") + codecopy(0, dataoffset("Test_deployed"), size) + return(0, size) + } + } + object "Test_deployed" { + code { + { + switch calldataload(0) + case 0 { + function f() -> ret { + ret := 1 + } + mstore(0, f()) + return(0, 32) + } + case 1 { + function f() -> ret { + ret := 2 + } + mstore(0, f()) + return(0, 32) + } + } + } + } +} diff --git a/crates/resolc/src/tests/unit/mod.rs b/crates/resolc/src/tests/unit/mod.rs index 48b3b4f..097d3c8 100644 --- a/crates/resolc/src/tests/unit/mod.rs +++ b/crates/resolc/src/tests/unit/mod.rs @@ -9,3 +9,4 @@ mod remappings; mod runtime_code; mod standard_json; mod unsupported_opcodes; +mod yul_function_scoping; diff --git a/crates/resolc/src/tests/unit/yul_function_scoping.rs b/crates/resolc/src/tests/unit/yul_function_scoping.rs new file mode 100644 index 0000000..5f07e31 --- /dev/null +++ b/crates/resolc/src/tests/unit/yul_function_scoping.rs @@ -0,0 +1,43 @@ +//! Tests for Yul function scoping and duplicate function name handling. + +use crate::test_utils::build_yul; + +/// Reproducer from GH-474: duplicate `f` across switch cases. +#[test] +fn duplicate_function_names_in_switch_cases() { + let code = r#" +object "Test" { + code { + { + let size := datasize("Test_deployed") + codecopy(0, dataoffset("Test_deployed"), size) + return(0, size) + } + } + object "Test_deployed" { + code { + { + switch calldataload(0) + case 0 { + function f() -> ret { + ret := 1 + } + mstore(0, f()) + return(0, 32) + } + case 1 { + function f() -> ret { + ret := 2 + } + mstore(0, f()) + return(0, 32) + } + } + } + } +} +"#; + + build_yul(&[("test.yul", code)]) + .expect("should compile duplicate function names in switch cases"); +} diff --git a/crates/yul/src/parser/statement/block.rs b/crates/yul/src/parser/statement/block.rs index 6e9c0ca..34633f8 100644 --- a/crates/yul/src/parser/statement/block.rs +++ b/crates/yul/src/parser/statement/block.rs @@ -136,6 +136,7 @@ impl Block { impl PolkaVMWriteLLVM for Block { fn into_llvm(self, context: &mut PolkaVMContext) -> anyhow::Result<()> { + context.push_function_scope(); let current_function = context.current_function().borrow().name().to_owned(); let current_block = context.basic_block(); @@ -223,6 +224,7 @@ impl PolkaVMWriteLLVM for Block { } context.pop_debug_scope(); + context.pop_function_scope(); Ok(()) }