From 1020434d2c0bfa7461a766eb92572b4d23b76f8a Mon Sep 17 00:00:00 2001 From: xermicus Date: Wed, 30 Sep 2026 14:03:09 +0200 Subject: [PATCH 1/2] Compile factory dependencies that are only present as nested Yul objects Signed-off-by: xermicus --- crates/resolc/src/cli_utils.rs | 9 ++ crates/resolc/src/project/contract/ir/mod.rs | 33 ++++++- crates/resolc/src/project/contract/mod.rs | 16 +++ crates/resolc/src/project/mod.rs | 21 +++- crates/resolc/src/tests/cli/standard_json.rs | 30 ++++++ crates/resolc/src/tests/cli/yul.rs | 24 ++++- .../factory_dependency_nested_object.sol | 2 + ...y_dependency_outside_output_selection.json | 1 + .../yul/factory_dependency_nested_object.yul | 97 +++++++++++++++++++ crates/yul/src/parser/statement/object.rs | 16 ++- 10 files changed, 236 insertions(+), 13 deletions(-) create mode 100644 crates/resolc/src/tests/data/solidity/factory_dependency_nested_object.sol create mode 100644 crates/resolc/src/tests/data/standard_json/factory_dependency_outside_output_selection.json create mode 100644 crates/resolc/src/tests/data/yul/factory_dependency_nested_object.yul diff --git a/crates/resolc/src/cli_utils.rs b/crates/resolc/src/cli_utils.rs index db9487a28..5f869ec97 100644 --- a/crates/resolc/src/cli_utils.rs +++ b/crates/resolc/src/cli_utils.rs @@ -25,6 +25,9 @@ pub const SOLIDITY_LIBRARY_CALL_CONTRACT_PATH: &str = "src/tests/data/solidity/l pub const SOLIDITY_COLLISION_NESTED_PATH: &str = "src/tests/data/solidity/collision/a/c.sol"; /// The paritytech/revive#622 reproducer contract at `a_c.sol`. pub const SOLIDITY_COLLISION_UNDERSCORE_PATH: &str = "src/tests/data/solidity/collision/a_c.sol"; +/// The verbatim reproducer from paritytech/revive#631: a contract creating another contract. +pub const SOLIDITY_FACTORY_DEPENDENCY_NESTED_OBJECT_PATH: &str = + "src/tests/data/solidity/factory_dependency_nested_object.sol"; /// The simple YUL contract test fixture path. pub const YUL_CONTRACT_PATH: &str = "src/tests/data/yul/contract.yul"; @@ -44,6 +47,9 @@ pub const YUL_DUPLICATE_FUNCTIONS_DEEP_NESTING_PATH: &str = pub const YUL_EMPTY_RUNTIME_OBJECT_PATH: &str = "src/tests/data/yul/empty_runtime_object.yul"; /// Yul contract carrying deploy code only, without the `_deployed` runtime sub-object. pub const YUL_DEPLOY_ONLY_OBJECT_PATH: &str = "src/tests/data/yul/deploy_only_object.yul"; +/// The solc IR of [`SOLIDITY_FACTORY_DEPENDENCY_NESTED_OBJECT_PATH`], with the dependency as a nested object. +pub const YUL_FACTORY_DEPENDENCY_NESTED_OBJECT_PATH: &str = + "src/tests/data/yul/factory_dependency_nested_object.yul"; /// The standard JSON contracts test fixture path. pub const STANDARD_JSON_CONTRACTS_PATH: &str = @@ -64,6 +70,9 @@ pub const STANDARD_JSON_NO_EVM_CODEGEN_COMPLEX_PATH: &str = /// The verbatim reproducer from paritytech/revive#627: a library address used only in a switch expression. pub const STANDARD_JSON_SWITCH_MISSING_LIBRARIES_PATH: &str = "src/tests/data/standard_json/switch_missing_libraries.json"; +/// The verbatim reproducer from paritytech/revive#631: a factory dependency outside the output selection. +pub const STANDARD_JSON_FACTORY_DEPENDENCY_OUTSIDE_OUTPUT_SELECTION_PATH: &str = + "src/tests/data/standard_json/factory_dependency_outside_output_selection.json"; /// The standard JSON PVM codegen all wildcard test fixture path. /// /// These contracts are similar to ones used in an example project. diff --git a/crates/resolc/src/project/contract/ir/mod.rs b/crates/resolc/src/project/contract/ir/mod.rs index 2390df5c9..a16a621a0 100644 --- a/crates/resolc/src/project/contract/ir/mod.rs +++ b/crates/resolc/src/project/contract/ir/mod.rs @@ -1,5 +1,6 @@ //! The contract source code. +use std::collections::BTreeMap; use std::collections::BTreeSet; use serde::Deserialize; @@ -30,11 +31,39 @@ impl IR { /// Drains the list of factory dependencies. pub fn drain_factory_dependencies(&mut self) -> BTreeSet { match self { - IR::Yul(ref mut yul) => yul.object.factory_dependencies.drain().collect(), + IR::Yul(ref mut yul) => std::mem::take(&mut yul.object.factory_dependencies), IR::NewYork(ref mut newyork) => { - newyork.yul_object.factory_dependencies.drain().collect() + std::mem::take(&mut newyork.yul_object.factory_dependencies) } } + .into_keys() + .collect() + } + + /// Returns the nested factory dependency objects missing in `identifier_paths`, by identifier. + pub fn unresolved_factory_dependencies( + &self, + identifier_paths: &BTreeMap, + ) -> Vec<(String, Self)> { + let factory_dependencies = match self { + IR::Yul(yul) => &yul.object.factory_dependencies, + IR::NewYork(newyork) => &newyork.yul_object.factory_dependencies, + }; + factory_dependencies + .iter() + .filter(|(identifier, _)| !identifier_paths.contains_key(identifier.as_str())) + .map(|(identifier, object)| { + let ir = match self { + IR::Yul(_) => Self::Yul(Yul { + object: object.to_owned(), + }), + IR::NewYork(_) => Self::NewYork(NewYork { + yul_object: object.to_owned(), + }), + }; + (identifier.to_owned(), ir) + }) + .collect() } /// Get the list of missing deployable libraries. diff --git a/crates/resolc/src/project/contract/mod.rs b/crates/resolc/src/project/contract/mod.rs index e81ece906..3b517160e 100644 --- a/crates/resolc/src/project/contract/mod.rs +++ b/crates/resolc/src/project/contract/mod.rs @@ -59,6 +59,22 @@ impl Contract { } } + /// Returns the nested factory dependencies missing in `identifier_paths` as contracts. + pub fn unresolved_factory_dependencies( + &self, + identifier_paths: &BTreeMap, + ) -> Vec { + self.ir + .unresolved_factory_dependencies(identifier_paths) + .into_iter() + .map(|(identifier, ir)| { + let identifier = + ContractIdentifier::new(self.identifier.path.to_owned(), Some(identifier)); + Self::new(identifier, ir, serde_json::Value::Null) + }) + .collect() + } + /// Compiles the specified contract, setting its build artifacts. pub fn compile( self, diff --git a/crates/resolc/src/project/mod.rs b/crates/resolc/src/project/mod.rs index 387daa3c2..d063121eb 100644 --- a/crates/resolc/src/project/mod.rs +++ b/crates/resolc/src/project/mod.rs @@ -49,9 +49,11 @@ pub struct Project { impl Project { /// A shortcut constructor. + /// + /// Nested factory dependency objects not otherwise part of `contracts` are added as contracts. pub fn new( version: Option, - contracts: BTreeMap, + mut contracts: BTreeMap, libraries: SolcStandardJsonInputSettingsLibraries, ) -> Self { let mut identifier_paths = BTreeMap::new(); @@ -59,6 +61,23 @@ impl Project { identifier_paths.insert(contract.object_identifier().to_owned(), path.to_owned()); } + let mut unresolved_contracts: Vec = contracts + .values() + .flat_map(|contract| contract.unresolved_factory_dependencies(&identifier_paths)) + .collect(); + while let Some(contract) = unresolved_contracts.pop() { + if identifier_paths.contains_key(contract.object_identifier()) { + continue; + } + identifier_paths.insert( + contract.object_identifier().to_owned(), + contract.identifier.full_path.to_owned(), + ); + unresolved_contracts + .extend(contract.unresolved_factory_dependencies(&identifier_paths)); + contracts.insert(contract.identifier.full_path.to_owned(), contract); + } + Self { version, contracts, diff --git a/crates/resolc/src/tests/cli/standard_json.rs b/crates/resolc/src/tests/cli/standard_json.rs index 178ee9663..adc134a14 100644 --- a/crates/resolc/src/tests/cli/standard_json.rs +++ b/crates/resolc/src/tests/cli/standard_json.rs @@ -8,6 +8,7 @@ use revive_solc_json_interface::{ use crate::cli_utils::{ assert_command_success, assert_equal_exit_codes, execute_resolc_with_stdin_input, execute_solc_with_stdin_input, STANDARD_JSON_ALL_OUTPUTS_PATH, STANDARD_JSON_CONTRACTS_PATH, + STANDARD_JSON_FACTORY_DEPENDENCY_OUTSIDE_OUTPUT_SELECTION_PATH, STANDARD_JSON_NEWYORK_DISABLED_PATH, STANDARD_JSON_NEWYORK_ENABLED_PATH, STANDARD_JSON_NO_EVM_CODEGEN_COMPLEX_PATH, STANDARD_JSON_NO_EVM_CODEGEN_PATH, STANDARD_JSON_NO_PVM_CODEGEN_PER_FILE_PATH, STANDARD_JSON_PVM_CODEGEN_ALL_WILDCARD_PATH, @@ -712,3 +713,32 @@ fn switch_expression_missing_libraries() { ); } } + +/// A factory dependency outside the output selection is compiled too. +#[test] +fn factory_dependency_outside_output_selection() { + let result = execute_resolc_with_stdin_input( + &[JSON_OPTION], + STANDARD_JSON_FACTORY_DEPENDENCY_OUTSIDE_OUTPUT_SELECTION_PATH, + ); + assert_command_success( + &result, + "the factory dependency outside output selection fixture", + ); + assert_no_errors(&to_solc_standard_json_output(&result.stdout)); + + let output: serde_json::Value = serde_json::from_str(&result.stdout).unwrap(); + let factory_dependencies = output["contracts"]["A.sol"]["P"]["factoryDependencies"] + .as_object() + .expect("the contract `A.sol:P` should have factory dependencies"); + assert_eq!(factory_dependencies.len(), 1); + for dependency in factory_dependencies.values() { + let (path, name) = dependency.as_str().unwrap().split_once(':').unwrap(); + assert!( + output["contracts"][path][name]["evm"]["bytecode"]["object"] + .as_str() + .is_some_and(|bytecode| !bytecode.is_empty()), + "the dependency `{dependency}` should be compiled" + ); + } +} diff --git a/crates/resolc/src/tests/cli/yul.rs b/crates/resolc/src/tests/cli/yul.rs index 12888e9eb..eaa117189 100644 --- a/crates/resolc/src/tests/cli/yul.rs +++ b/crates/resolc/src/tests/cli/yul.rs @@ -4,7 +4,7 @@ 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_DEPLOY_ONLY_OBJECT_PATH, YUL_DUPLICATE_FUNCTIONS_DEEP_NESTING_PATH, YUL_DUPLICATE_FUNCTIONS_SWITCH_PATH, - YUL_INVALID_HEX_NIBBLES_PATH, + YUL_FACTORY_DEPENDENCY_NESTED_OBJECT_PATH, YUL_INVALID_HEX_NIBBLES_PATH, }; #[test] @@ -75,3 +75,25 @@ fn compiles_object_without_a_runtime_sub_object() { let solc_result = execute_solc(&[YUL_DEPLOY_ONLY_OBJECT_PATH, SOLC_YUL_FLAG]); assert_equal_exit_codes(&solc_result, &resolc_result); } + +/// A factory dependency given only as a nested object is compiled too. +#[test] +fn compiles_factory_dependency_nested_object() { + let resolc_result = execute_resolc(&[ + YUL_FACTORY_DEPENDENCY_NESTED_OBJECT_PATH, + RESOLC_YUL_FLAG, + "--bin", + ]); + assert_command_success(&resolc_result, "A factory dependency as a nested object"); + for object in ["K_4", "P_19"] { + assert!( + resolc_result.stdout.contains(&format!( + "{YUL_FACTORY_DEPENDENCY_NESTED_OBJECT_PATH}:{object}" + )), + "the object `{object}` should be compiled" + ); + } + + let solc_result = execute_solc(&[YUL_FACTORY_DEPENDENCY_NESTED_OBJECT_PATH, SOLC_YUL_FLAG]); + assert_equal_exit_codes(&solc_result, &resolc_result); +} diff --git a/crates/resolc/src/tests/data/solidity/factory_dependency_nested_object.sol b/crates/resolc/src/tests/data/solidity/factory_dependency_nested_object.sol new file mode 100644 index 000000000..6b9cfa171 --- /dev/null +++ b/crates/resolc/src/tests/data/solidity/factory_dependency_nested_object.sol @@ -0,0 +1,2 @@ +contract K { uint256 public v = 7; } +contract P { function mk() external returns (address) { return address(new K()); } } diff --git a/crates/resolc/src/tests/data/standard_json/factory_dependency_outside_output_selection.json b/crates/resolc/src/tests/data/standard_json/factory_dependency_outside_output_selection.json new file mode 100644 index 000000000..0e3bf9b6e --- /dev/null +++ b/crates/resolc/src/tests/data/standard_json/factory_dependency_outside_output_selection.json @@ -0,0 +1 @@ +{"language":"Solidity","sources":{"A.sol":{"content":"import \"B.sol\"; contract P { function mk() external returns (address) { return address(new K()); } }"},"B.sol":{"content":"contract K { uint256 public v = 7; }"}},"settings":{"optimizer":{"enabled":true},"outputSelection":{"A.sol":{"*":["evm.bytecode"]}}}} diff --git a/crates/resolc/src/tests/data/yul/factory_dependency_nested_object.yul b/crates/resolc/src/tests/data/yul/factory_dependency_nested_object.yul new file mode 100644 index 000000000..03627a332 --- /dev/null +++ b/crates/resolc/src/tests/data/yul/factory_dependency_nested_object.yul @@ -0,0 +1,97 @@ +// solc 0.8.37 --ir-optimized --optimize src/tests/data/solidity/factory_dependency_nested_object.sol +/// @use-src 0:"src/tests/data/solidity/factory_dependency_nested_object.sol" +object "P_19" { + code { + { + /// @src 0:37:121 "contract P { function mk() external returns (address) { return address(new K()); } }" + let _1 := memoryguard(0x80) + mstore(64, _1) + if callvalue() { revert(0, 0) } + let _2 := datasize("P_19_deployed") + codecopy(_1, dataoffset("P_19_deployed"), _2) + return(_1, _2) + } + } + /// @use-src 0:"src/tests/data/solidity/factory_dependency_nested_object.sol" + object "P_19_deployed" { + code { + { + /// @src 0:37:121 "contract P { function mk() external returns (address) { return address(new K()); } }" + let _1 := memoryguard(0x80) + mstore(64, _1) + if iszero(lt(calldatasize(), 4)) + { + if eq(0x2f81b0a0, shr(224, calldataload(0))) + { + if callvalue() { revert(0, 0) } + if slt(add(calldatasize(), not(3)), 0) { revert(0, 0) } + /// @src 0:108:115 "new K()" + let _2 := datasize("K_4") + let _3 := add(_1, _2) + if or(gt(_3, 0xffffffffffffffff), lt(_3, _1)) + { + /// @src 0:37:121 "contract P { function mk() external returns (address) { return address(new K()); } }" + mstore(0, shl(224, 0x4e487b71)) + mstore(4, 0x41) + revert(0, 0x24) + } + /// @src 0:108:115 "new K()" + datacopy(_1, dataoffset("K_4"), _2) + let expr_address := create(/** @src 0:37:121 "contract P { function mk() external returns (address) { return address(new K()); } }" */ 0, /** @src 0:108:115 "new K()" */ _1, sub(_3, _1)) + if iszero(expr_address) + { + /// @src 0:37:121 "contract P { function mk() external returns (address) { return address(new K()); } }" + let pos := mload(64) + returndatacopy(pos, 0, returndatasize()) + revert(pos, returndatasize()) + } + let memPos := mload(64) + mstore(memPos, and(/** @src 0:100:116 "address(new K())" */ expr_address, /** @src 0:37:121 "contract P { function mk() external returns (address) { return address(new K()); } }" */ sub(shl(160, /** @src 0:108:115 "new K()" */ 1), 1))) + /// @src 0:37:121 "contract P { function mk() external returns (address) { return address(new K()); } }" + return(memPos, 32) + } + } + revert(0, 0) + } + } + /// @use-src 0:"src/tests/data/solidity/factory_dependency_nested_object.sol" + object "K_4" { + code { + { + /// @src 0:0:36 "contract K { uint256 public v = 7; }" + let _1 := memoryguard(0x80) + mstore(64, _1) + if callvalue() { revert(0, 0) } + sstore(/** @src 0:32:33 "7" */ 0x00, 0x07) + /// @src 0:0:36 "contract K { uint256 public v = 7; }" + let _2 := datasize("K_4_deployed") + codecopy(_1, dataoffset("K_4_deployed"), _2) + return(_1, _2) + } + } + /// @use-src 0:"src/tests/data/solidity/factory_dependency_nested_object.sol" + object "K_4_deployed" { + code { + { + /// @src 0:0:36 "contract K { uint256 public v = 7; }" + let _1 := memoryguard(0x80) + mstore(64, _1) + if iszero(lt(calldatasize(), 4)) + { + if eq(0x7c2efcba, shr(224, calldataload(0))) + { + if callvalue() { revert(0, 0) } + if slt(add(calldatasize(), not(3)), 0) { revert(0, 0) } + mstore(_1, sload(0)) + return(_1, 32) + } + } + revert(0, 0) + } + } + data ".metadata" hex"a264697066735822122033635a5e157e493f8335a094bffd8f41ae34e8025d755b3a8ae9dc36603f510664736f6c63430008250033" + } + } + data ".metadata" hex"a264697066735822122098ff7d7667f1889a4f7fed09b13869dbb95240199d269a8f3c82ff8e4c50169264736f6c63430008250033" + } +} diff --git a/crates/yul/src/parser/statement/object.rs b/crates/yul/src/parser/statement/object.rs index 209124707..a3edc60a0 100644 --- a/crates/yul/src/parser/statement/object.rs +++ b/crates/yul/src/parser/statement/object.rs @@ -1,7 +1,7 @@ //! The YUL object. +use std::collections::BTreeMap; use std::collections::BTreeSet; -use std::collections::HashSet; use inkwell::debug_info::AsDIScope; use revive_llvm_context::PolkaVMCodeType; @@ -40,10 +40,8 @@ pub struct Object { pub code: Code, /// The optional inner object, representing the runtime code. pub inner_object: Option>, - /// The factory dependency objects, which are represented by nested Yul object. The nested - /// objects are duplicates of the upper-level objects describing the dependencies, so only - /// their identifiers are preserved. The identifiers are used to address upper-level objects. - pub factory_dependencies: HashSet, + /// The factory dependency objects, which are represented by nested Yul objects, by identifier. + pub factory_dependencies: BTreeMap, } impl Object { @@ -100,7 +98,7 @@ impl Object { let code = Code::parse(lexer, None)?; let mut inner_object = None; - let mut factory_dependencies = HashSet::new(); + let mut factory_dependencies = BTreeMap::new(); if !is_runtime_code { inner_object = match lexer.peek()? { @@ -119,7 +117,7 @@ impl Object { .into()); } - factory_dependencies.extend(object.factory_dependencies.drain()); + factory_dependencies.append(&mut object.factory_dependencies); Some(Box::new(object)) } _ => Some(Box::new(Self::implicit_runtime_code(&identifier, location))), @@ -149,7 +147,7 @@ impl Object { .. } => { let dependency = Self::parse(lexer, Some(token))?; - factory_dependencies.insert(dependency.identifier); + factory_dependencies.insert(dependency.identifier.clone(), dependency); } Token { lexeme: Lexeme::Identifier(identifier), @@ -197,7 +195,7 @@ impl Object { }, }, inner_object: None, - factory_dependencies: HashSet::new(), + factory_dependencies: BTreeMap::new(), } } From 68c14081ae9ff09c6321fa48cb59f3931a05af15 Mon Sep 17 00:00:00 2001 From: xermicus Date: Wed, 7 Oct 2026 15:15:38 +0200 Subject: [PATCH 2/2] review Signed-off-by: xermicus --- Cargo.lock | 1 + crates/resolc/src/cli_utils.rs | 5 + crates/resolc/src/project/mod.rs | 24 ++-- crates/resolc/src/tests/cli/standard_json.rs | 17 +++ crates/resolc/src/tests/cli/yul_objects.rs | 15 ++- .../factory_dependency_path_collision.json | 1 + .../tests/data/yul/duplicate_object_name.yul | 28 +++++ crates/yul/Cargo.toml | 1 + crates/yul/src/parser/error.rs | 10 ++ crates/yul/src/parser/statement/object.rs | 107 +++++++++++++++++- 10 files changed, 198 insertions(+), 11 deletions(-) create mode 100644 crates/resolc/src/tests/data/standard_json/factory_dependency_path_collision.json create mode 100644 crates/resolc/src/tests/data/yul/duplicate_object_name.yul diff --git a/Cargo.lock b/Cargo.lock index 0882ab930..8ebc59025 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9651,6 +9651,7 @@ dependencies = [ "revive-common", "revive-llvm-context", "serde", + "serde_json", "thiserror 2.0.18", ] diff --git a/crates/resolc/src/cli_utils.rs b/crates/resolc/src/cli_utils.rs index 4df00fe0e..464633724 100644 --- a/crates/resolc/src/cli_utils.rs +++ b/crates/resolc/src/cli_utils.rs @@ -73,6 +73,8 @@ pub const YUL_FACTORY_DEPENDENCY_NESTED_OBJECT_PATH: &str = pub const YUL_SIBLING_OBJECTS_PATH: &str = "src/tests/data/yul/sibling_objects.yul"; /// Yul contract addressing a nested object through the dotted notation. pub const YUL_DOTTED_OBJECT_PATH: &str = "src/tests/data/yul/dotted_object_path.yul"; +/// Yul contract with two different nested objects of the same name in different scopes. +pub const YUL_DUPLICATE_OBJECT_NAME_PATH: &str = "src/tests/data/yul/duplicate_object_name.yul"; /// The standard JSON contracts test fixture path. pub const STANDARD_JSON_CONTRACTS_PATH: &str = @@ -102,6 +104,9 @@ pub const STANDARD_JSON_FACTORY_DEPENDENCY_OUTSIDE_OUTPUT_SELECTION_PATH: &str = /// A factory dependency outside the output selection calling a library. pub const STANDARD_JSON_FACTORY_DEPENDENCY_OUTSIDE_OUTPUT_SELECTION_LIBRARIES_PATH: &str = "src/tests/data/standard_json/factory_dependency_outside_output_selection_libraries.json"; +/// A factory dependency outside the output selection whose Yul name is a contract name in the creator's file. +pub const STANDARD_JSON_FACTORY_DEPENDENCY_PATH_COLLISION_PATH: &str = + "src/tests/data/standard_json/factory_dependency_path_collision.json"; /// The standard JSON PVM codegen all wildcard test fixture path. /// /// These contracts are similar to ones used in an example project. diff --git a/crates/resolc/src/project/mod.rs b/crates/resolc/src/project/mod.rs index f197a8844..769a68ebe 100644 --- a/crates/resolc/src/project/mod.rs +++ b/crates/resolc/src/project/mod.rs @@ -51,11 +51,12 @@ impl Project { /// A shortcut constructor. /// /// Nested factory dependency objects not otherwise part of `contracts` are added as contracts. + /// A nested object whose path is already taken by another contract is an error. pub fn new( version: Option, mut contracts: BTreeMap, libraries: SolcStandardJsonInputSettingsLibraries, - ) -> Self { + ) -> anyhow::Result { let mut identifier_paths = BTreeMap::new(); for (path, contract) in contracts.iter() { identifier_paths.insert(contract.object_identifier().to_owned(), path.to_owned()); @@ -69,6 +70,15 @@ impl Project { if identifier_paths.contains_key(contract.object_identifier()) { continue; } + if contracts.contains_key(contract.identifier.full_path.as_str()) { + anyhow::bail!( + "the nested object `{}` can not be emitted as `{}`, which is the path of another \ + contract. Adding the source file of `{}` to the output selection avoids this.", + contract.object_identifier(), + contract.identifier.full_path, + contract.object_identifier(), + ); + } identifier_paths.insert( contract.object_identifier().to_owned(), contract.identifier.full_path.to_owned(), @@ -78,12 +88,12 @@ impl Project { contracts.insert(contract.identifier.full_path.to_owned(), contract); } - Self { + Ok(Self { version, contracts, identifier_paths, libraries, - } + }) } /// Compiles all contracts, returning their build artifacts. @@ -278,7 +288,7 @@ impl Project { }, } } - Ok(Self::new(None, contracts, libraries)) + Self::new(None, contracts, libraries) } /// Converts the `solc` JSON output into a convenient project. @@ -329,11 +339,7 @@ impl Project { Err(error) => solc_output.push_error(Some(path), error), } } - Ok(Project::new( - Some(solc_version.clone()), - contracts, - libraries, - )) + Project::new(Some(solc_version.clone()), contracts, libraries) } } diff --git a/crates/resolc/src/tests/cli/standard_json.rs b/crates/resolc/src/tests/cli/standard_json.rs index 467051b7f..ad282c668 100644 --- a/crates/resolc/src/tests/cli/standard_json.rs +++ b/crates/resolc/src/tests/cli/standard_json.rs @@ -13,6 +13,7 @@ use crate::cli_utils::{ STANDARD_JSON_FACTORY_DEPENDENCY_LIBRARIES_PATH, STANDARD_JSON_FACTORY_DEPENDENCY_OUTSIDE_OUTPUT_SELECTION_LIBRARIES_PATH, STANDARD_JSON_FACTORY_DEPENDENCY_OUTSIDE_OUTPUT_SELECTION_PATH, + STANDARD_JSON_FACTORY_DEPENDENCY_PATH_COLLISION_PATH, STANDARD_JSON_HEAP_SIZE_LINKER_FAILURE_PATH, STANDARD_JSON_HEAP_SIZE_OUT_OF_RANGE_PATH, STANDARD_JSON_IMMUTABLES_OVER_LIMIT_PATH, STANDARD_JSON_MIX_ALL_WILDCARD_AND_FILE_SELECTION_PATH, STANDARD_JSON_NEWYORK_DISABLED_PATH, @@ -903,6 +904,22 @@ fn factory_dependency_outside_output_selection_missing_libraries() { } } +/// A factory dependency outside the output selection must not replace a contract of the same path. +#[test] +fn factory_dependency_path_collision() { + let result = execute_resolc_with_stdin_input( + &[JSON_OPTION], + STANDARD_JSON_FACTORY_DEPENDENCY_PATH_COLLISION_PATH, + ); + assert_command_success(&result, "the factory dependency path collision fixture"); + + let output = to_solc_standard_json_output(&result.stdout); + assert_standard_json_errors_contain( + &output, + "`B.sol:K_4`, which is the path of another contract", + ); +} + #[test] fn revert_strings_input_setting() { let default = diff --git a/crates/resolc/src/tests/cli/yul_objects.rs b/crates/resolc/src/tests/cli/yul_objects.rs index f36755373..a473aca0d 100644 --- a/crates/resolc/src/tests/cli/yul_objects.rs +++ b/crates/resolc/src/tests/cli/yul_objects.rs @@ -7,7 +7,7 @@ use crate::cli_utils::{ absolute_path, assert_command_failure, execute_resolc, CommandResult, RESOLC_YUL_FLAG, - YUL_DOTTED_OBJECT_PATH, YUL_SIBLING_OBJECTS_PATH, + YUL_DOTTED_OBJECT_PATH, YUL_DUPLICATE_OBJECT_NAME_PATH, YUL_SIBLING_OBJECTS_PATH, }; /// Asserts the command failed with a diagnostic rather than by aborting. @@ -47,3 +47,16 @@ fn reports_the_dotted_object_notation() { result.stderr ); } + +#[test] +fn reports_a_duplicate_object_name() { + let path = absolute_path(YUL_DUPLICATE_OBJECT_NAME_PATH); + let result = execute_resolc(&[&path, RESOLC_YUL_FLAG, "--bin"]); + + assert_reported_not_aborted(&result, "Two different objects of the same name"); + assert!( + result.stderr.contains("DuplicateObjectName"), + "the object should be named in the diagnostic, got: {}", + result.stderr + ); +} diff --git a/crates/resolc/src/tests/data/standard_json/factory_dependency_path_collision.json b/crates/resolc/src/tests/data/standard_json/factory_dependency_path_collision.json new file mode 100644 index 000000000..4f886ccd3 --- /dev/null +++ b/crates/resolc/src/tests/data/standard_json/factory_dependency_path_collision.json @@ -0,0 +1 @@ +{"language":"Solidity","sources":{"A.sol":{"content":"contract K { uint256 public v = 7; }"},"B.sol":{"content":"import \"A.sol\"; contract K_4 { function w() external pure returns (uint256) { return 42; } } contract P { function mk() external returns (address) { return address(new K()); } function mk4() external returns (address) { return address(new K_4()); } }"}},"settings":{"optimizer":{"enabled":true},"outputSelection":{"B.sol":{"*":["evm.bytecode"]}}}} diff --git a/crates/resolc/src/tests/data/yul/duplicate_object_name.yul b/crates/resolc/src/tests/data/yul/duplicate_object_name.yul new file mode 100644 index 000000000..d479bdf14 --- /dev/null +++ b/crates/resolc/src/tests/data/yul/duplicate_object_name.yul @@ -0,0 +1,28 @@ +object "A" { + code { + datacopy(0, dataoffset("A_deployed"), datasize("A_deployed")) + return(0, datasize("A_deployed")) + } + object "A_deployed" { + code { + datacopy(0, dataoffset("K"), datasize("K")) + let child := create(0, 0, datasize("K")) + if iszero(call(gas(), child, 0, 0, 0, 0, 32)) { revert(0, 0) } + return(0, 32) + } + object "K" { + code { + datacopy(0, dataoffset("K_deployed"), datasize("K_deployed")) + return(0, datasize("K_deployed")) + } + object "K_deployed" { code { mstore(0, 111) return(0, 32) } } + } + } + object "K" { + code { + datacopy(0, dataoffset("K_deployed"), datasize("K_deployed")) + return(0, datasize("K_deployed")) + } + object "K_deployed" { code { mstore(0, 222) return(0, 32) } } + } +} diff --git a/crates/yul/Cargo.toml b/crates/yul/Cargo.toml index 10a873e26..cbeea118c 100644 --- a/crates/yul/Cargo.toml +++ b/crates/yul/Cargo.toml @@ -14,6 +14,7 @@ inkwell = { workspace = true } num = { workspace = true } regex = { workspace = true } serde = { workspace = true } +serde_json = { workspace = true } thiserror = { workspace = true } revive-common = { workspace = true } diff --git a/crates/yul/src/parser/error.rs b/crates/yul/src/parser/error.rs index 74ce6c727..fe7c43b61 100644 --- a/crates/yul/src/parser/error.rs +++ b/crates/yul/src/parser/error.rs @@ -47,6 +47,16 @@ pub enum Error { /// The invalid identifier. found: String, }, + /// Two different objects with the same name. + #[error( + "{location} The object `{identifier}` is declared twice with different code, which is not supported" + )] + DuplicateObjectName { + /// The second object location. + location: Location, + /// The duplicate identifier. + identifier: String, + }, /// Invalid code length. #[error("The line or column length exceed the maximum of u32::MAX")] InvalidLength, diff --git a/crates/yul/src/parser/statement/object.rs b/crates/yul/src/parser/statement/object.rs index 6def1ed37..00f07c8d4 100644 --- a/crates/yul/src/parser/statement/object.rs +++ b/crates/yul/src/parser/statement/object.rs @@ -29,6 +29,9 @@ use crate::parser::statement::Statement; use crate::visitor::AstNode; use crate::visitor::AstVisitor; +/// The name of the source location field of the serialized syntax tree nodes. +const LOCATION_FIELD: &str = "location"; + /// The upper-level YUL object, representing the deploy code. #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)] pub struct Object { @@ -151,7 +154,7 @@ impl Object { .. } => { let dependency = Self::parse(lexer, Some(token))?; - factory_dependencies.insert(dependency.identifier.clone(), dependency); + Self::insert_factory_dependency(&mut factory_dependencies, dependency)?; } Token { lexeme: Lexeme::Identifier(identifier), @@ -181,6 +184,49 @@ impl Object { }) } + /// Inserts `dependency`, keeping an existing copy that differs in source locations only. + /// + /// Objects are resolved by name alone, so a different object of the same name is an error. + fn insert_factory_dependency( + factory_dependencies: &mut BTreeMap, + dependency: Self, + ) -> Result<(), Error> { + match factory_dependencies.get(&dependency.identifier) { + Some(existing) if existing.without_locations() != dependency.without_locations() => { + Err(ParserError::DuplicateObjectName { + location: dependency.location, + identifier: dependency.identifier, + } + .into()) + } + Some(_) => Ok(()), + None => { + factory_dependencies.insert(dependency.identifier.clone(), dependency); + Ok(()) + } + } + } + + /// Returns the object as JSON with all source locations removed. + fn without_locations(&self) -> serde_json::Value { + fn remove_locations(value: &mut serde_json::Value) { + match value { + serde_json::Value::Object(fields) => { + fields.remove(LOCATION_FIELD); + fields.values_mut().for_each(remove_locations); + } + serde_json::Value::Array(elements) => { + elements.iter_mut().for_each(remove_locations) + } + _ => {} + } + } + + let mut value = serde_json::to_value(self).expect("Always valid"); + remove_locations(&mut value); + value + } + /// A short-hand constructor returning an object with a call to `stop`. fn implicit_runtime_code(identifier: &str, location: Location) -> Self { let stop = Statement::Expression(Expression::FunctionCall(FunctionCall { @@ -593,4 +639,63 @@ object "Test" { statement => panic!("expected an `invalid()` call, found {statement:?}"), } } + + #[test] + fn ok_duplicate_object_copies() { + let input = r#" +object "P" { + code { } + object "P_deployed" { + code { } + object "K" { + code { } + object "K_deployed" { code { stop() } } + } + } + object "K" { + code { } + object "K_deployed" { + code { + stop() + } + } + } +} + "#; + + let mut lexer = Lexer::new(input.to_owned()); + let object = Object::parse(&mut lexer, None).expect("copies of an object are accepted"); + assert!(object.factory_dependencies.contains_key("K")); + } + + #[test] + fn error_duplicate_object_name() { + let input = r#" +object "P" { + code { } + object "P_deployed" { + code { } + object "K" { + code { } + object "K_deployed" { code { stop() } } + } + } + object "K" { + code { } + object "K_deployed" { code { invalid() } } + } +} + "#; + + let mut lexer = Lexer::new(input.to_owned()); + let result = Object::parse(&mut lexer, None); + assert_eq!( + result, + Err(Error::DuplicateObjectName { + location: Location::new(11, 5), + identifier: "K".to_owned(), + } + .into()) + ); + } }