ethereum / solidity

Solidity, the Smart Contract Programming Language
https://soliditylang.org
GNU General Public License v3.0
23.15k stars 5.74k forks source link

ICE when loading the same file multiple times via import callback #15458

Open cameel opened 1 week ago

cameel commented 1 week ago

Description

When the same path is specified under urls of multiple contracts in Standard JSON input, we get an ICE in the import callback.

Environment

Steps to Reproduce

input.json:

{
    "language": "Solidity",
    "sources": {
        "A": {"urls": ["/tmp/test.sol"]},
        "B": {"urls": ["/tmp/test.sol"]}
    }
}

/tmp/test.sol

// SPDX-License-Identifier: MIT
pragma solidity *;

contract C {}
solc --standard-json input.json --pretty-json --json-indent 4

Output:

{
    "errors": [
        {
            "component": "general",
            "formattedMessage": "Cannot import url (\"/tmp/test.sol\"): Exception in read callback: /solidity/libsolidity/interface/FileReader.cpp(164): Throw in function solidity::frontend::ReadCallback::Result solidity::frontend::FileReader::readFile(const std::string&, const std::string&)\nDynamic exception type: boost::wrapexcept<solidity::langutil::InternalCompilerError>\nstd::exception::what: Solidity assertion failed\n[solidity::util::tag_comment*] = Solidity assertion failed\n",
            "message": "Cannot import url (\"/tmp/test.sol\"): Exception in read callback: /solidity/libsolidity/interface/FileReader.cpp(164): Throw in function solidity::frontend::ReadCallback::Result solidity::frontend::FileReader::readFile(const std::string&, const std::string&)\nDynamic exception type: boost::wrapexcept<solidity::langutil::InternalCompilerError>\nstd::exception::what: Solidity assertion failed\n[solidity::util::tag_comment*] = Solidity assertion failed\n",
            "severity": "error",
            "type": "IOError"
        }
    ],
    "sources": {
        "A": {
            "id": 0
        }
    }
}
cameel commented 1 week ago

The immediate cause is that the callback has this weird flow, where the sources are not stored in a single place, but independently in both FileReader and CompilerStack. We initially fill the FileReader, then pass the sources into CompilerStack, which may add new ones loaded by the callback, but the reader still exists and updates its m_sourceCodes map: https://github.com/ethereum/solidity/blob/3ee5f2dfc09b35019fcf887f7a5e1d0054e250c5/libsolidity/interface/FileReader.cpp#L163-L165

We should get rid of that. It would be best to separate the storage of the sources from the callback. The callback should know its settings but be stateless otherwise.

gks2022004 commented 1 day ago

@cameel please assign it to me