Compile factory dependencies that are only present as nested Yul objects

[xermicus]

Closes #631

[elle-j]

A resolved contract will have the key <file>:<Solidity name>, while an unresolved factory dependency will have the key <parent file>:<dependency's Yul name> (the parent being the file that imports the dependency). So contract.identifier.full_path could collide here, and this line would then replace the first contract stored at the shared key, with the second contract stored at the same key.

This example shows how there can be 2 different contracts getting the key B.sol:K_4:

Example:

• File A.sol:
  • Declares contract K.
    • Gets AST ID 4. Yul name: K_4.
• File B.sol (imports A.sol):
  • Declares contract K_4.
    • K_4's full path: B.sol:K_4 (i.e. <file>:<Solidity name>)
  • Declares contract P which contains:
    • new K() (from the dependency)
      • K's full path: B.sol:K_4 (i.e. <parent file>:<dependency's Yul name>)

Output contracts["B.sol"]["K_4"] now contains contract K from A.sol.

{"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"]}}}}

[xermicus]

Good catch thanks. Just errors out now

[elle-j]

This is still unresolved. For instance, you can compare the previous JSON repro (which selects only B.sol in the outputSelection) with the same file with the only change being selectiong both A.sol and B.sol:

repro.json (selects B.sol):

{"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"]}}}}

repro-all.json (selects A.sol and B.sol):

{"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":{"A.sol":{"*":["evm.bytecode"]},"B.sol":{"*":["evm.bytecode"]}}}}

for input in repro.json repro-all.json; do
  resolc --standard-json < $input | jq -c '.contracts | {"A.sol:K": .["A.sol"].K.hash[0:8], "B.sol:K_4": .["B.sol"].K_4.hash[0:8]}'
done

# Output (solc v0.8.36 used):
# {"A.sol:K":null,"B.sol:K_4":"55af49df"}
# {"A.sol:K":"55af49df","B.sol:K_4":"b96a268a"}

With both files selected (second output row), it shows that A.sol:K hash is 55af49df. For the repro with only B.sol selected (first output row), it shows that B.sol:K_4 hash is also 55af49df (i.e. B.sol's K_4 was replaced by A.sol's K, and the new K_4() in contract P would deploy the wrong contract).

Possible fix (reaches the error from #600):

crates/resolc/src/project/mod.rs:

 while let Some(contract) = unresolved_contracts.pop() {
-    if identifier_paths.contains_key(contract.object_identifier()) {
+    if identifier_paths.contains_key(contract.object_identifier())
+        || contracts.contains_key(&contract.identifier.full_path)
+    {
         continue;
     }

[elle-j]

This line can silently replace a contract when there are nested Yul objects with the same name in different scopes.

The K nested in A returns 222 (i.e. it's the outer K's contract):

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) } }
    }
}

[xermicus]

Same here

[elle-j]

Can still repro this. If I name the previous Yul code shadow.yul:

# (0x6f is 111)
# prints 0
resolc --yul shadow.yul --asm | grep -c 0x6f

# (0xde is 222)
# prints 1
resolc --yul shadow.yul --asm | grep -c 0xde

The output has no 0x6f (111, from the inner K), only 0xde (222, from the outer K). So A deploys the wrong code.

[xermicus]

So sorry, my push with the fix didn't get through. Can you check again?

[elle-j]

The E2E test is failing but it may be due what I mention in the comment below.

[elle-j]

This only removes fields named location, but Continue, Break, and Leave keep that as unnamed:

revive/crates/yul/src/parser/statement/mod.rs

Lines 63 to 68 in 351dfdc

	/// The `continue` statement.
	Continue(Location),
	/// The `break` statement.
	Break(Location),
	/// The `leave` statement.
	Leave(Location),

This should compile but fails with exit 1: DuplicateObjectName { ..., identifier: "K_17" }:

contract K {
    function f(uint256 n) external pure returns (uint256) {
        if (n > 1) return 1;
        return 2;
    }
}

contract P {
    address public k;

    constructor() {
        k = address(new K());
    }

    function mk() external returns (address) {
        return address(new K());
    }
}