The old optimizer breaks internal function equality

[chatziko]

Description

We (@chatziko, @MichaelXanth) ran into an optimizer issue that can be reproduced in the following minimal contract:

pragma solidity ^0.8.18;

contract Test {
    function v1() public {}
    function v2() public {}

    function equalityTest() external returns(bool) {
        return v1 == v2;
    }
}

equalityTest() returnsfalse when the optimizer is disabled. However, with the old optimizer enabled, the equivalent functions v1 and v2 are merged into one, causing v1 == v2 to be true. I am not sure whether there is a clear specification of function type comparison, but the two functions have distinct selectors so it makes sense to consider them as non-equal. In any case, the optimizer should not alter the contract's semantics.

This behaviour does not appear in the Yul optimizer.

Environment

• Compiler version: 0.8.18
  • Also with the unreleased 0.8.19
• Target EVM version (as per compiler settings): istanbul
• Framework/IDE (e.g. Truffle or Remix): hardhat

Steps to Reproduce

Compile the above contract with and without the optimizer.

The produced bytecode can be compared below:

https://library.dedaub.com/decompile

Non-Optimized MD5: e1a4db0287ad6624f24ab4adbc61475b
Optimized MD5: 531e2a5364fdbc3e00c9733a4372d948

[ekpyron]

Just to be clear here: I take it that this.v1 == this.v2 will actually not be affected by this?

For context: v1 and v2 are internal function pointers - you can also only assign them to internal function pointers and only call them internally. If the optimizer collapses identical internal functions, the described behaviour is at least in some sense consistent.

On the other hand, this.v1 and this.v2 are external function pointers, which actually involve encoding the address of this and the selector together. For those, the optimizer changing the behaviour of an equality comparison would definitely be a serious bug.

Just to make this distinction clear here - all that being said, it still seems a bit weird and misleading that this happens - out of my head I'm not quite sure what to do about this. An option would be to disallow comparisons of internal function types altogether - although there might be uses for that.

[chatziko]

Indeed this.v1 == this.v2 is not affect, the issue concerns internal function pointers.

Maybe a function should not be candidate for collapsing if it's used as a function pointer anywhere in the code?

[ekpyron]

I'd rather consider disallowing comparisons on internal functions - did you actually intend to compare internal functions here? What was your use case for that?

[chatziko]

No, not really, we accidentally ran into this, we don't need it.

[cameel]

Decision from the call: we'll add a warning about comparing internal functions and in 0.9.0 we'll make it an error.

[ekpyron]

Changing the milestone for the warning part to 0.8.20. We can then flip to 0.9.0 for making it an error.