ethereum / solidity

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

Compiler bug virtual/override argument location specifcation #14603

Closed FredCoen closed 11 months ago

FredCoen commented 1 year ago

The solidity compiler allows compilation when a virtual function takes an argument in memory while the overriding implemenatinon takes it in calldata. This can lead to various bugs since these two dont behave the same way. For instance slicing of bytes is only possible in calldata and not memory.

Due to the resulting uncertain behaviour, the comiler should prevent this!

r0qs commented 1 year ago

Hi @FredCoen, do you have a code which we can reproduce the problem? Note, that external functions which have memory arguments can be overridden with calldata arguments and vice-versa, since they can only be called externally and thereby always get their arguments in calldata. Maybe that's the confusion here.

For instance, see the example below:

pragma solidity >=0.8.2 <0.9.0;

abstract contract A {
    function f(string memory s) external virtual;
}

contract B is A {
    string public a;

    function f(string calldata s) public virtual override {
        a = s;
    }
}

This is completely fine. But, if you change the visibility of function f in the contract A to anything other than external you should get a compiler error like this: Data locations of parameters have to be the same when overriding non-external functions, but they differ

You can see here when this was fixed in Solidity 0.8.14 https://github.com/ethereum/solidity/pull/12850 and read more about the bug at that time in our blog: https://soliditylang.org/blog/2022/05/17/data-location-inheritance-bug/

However, if you can still reproduce this bug in any solidity version greater than 0.8.14, please share your code with us :)

mehtavishwa30 commented 11 months ago

I believe @r0qs's explanation covers this case. Please feel free to reach us in case this bug can be still reproduced in Solidity versions > 0.8.14. Until then, we will be closing the issue.