Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect parameterSlots and returnSlots for some methods in functionDebugData #14874

Open
sifislag opened this issue Feb 20, 2024 · 1 comment
Labels
bug 🐛 low effort There is not much implementation work to be done. The task is very easy or tiny. medium impact Default level of impact must have Something we consider an essential part of Solidity 1.0.

Comments

@sifislag
Copy link

Description

The parameterSlots, and returnSlots values for the functionDebugData of some compiler generated methods implementing getters are wrong.

Environment

  • Compiler version: 0.8.24
  • Target EVM version (as per compiler settings): shanghai
  • Operating system: ubuntu 20.04

Steps to Reproduce

Compiling the following json file using solc --standard-json:

{
  "language": "Solidity",
  "sources": {
    "./function-debug-info.sol":{
      "content": "pragma solidity ^0.8.0;\ncontract BadDebugData {\n    address[] public stakes;}"
    }
  },
  "settings": {
    "optimizer": {
       "enabled": true,
        "runs": 200
    },
    "evmVersion": "shanghai",
    "outputSelection": {
      "*": {
        "*": [
          "abi",
          "evm.deployedBytecode"
        ]
      }
    }
  }
}

Returns the following bytecode:

6080604052348015600e575f80fd5b50600436106026575f3560e01c8063d5a44f8614602a575b5f80fd5b60396035366004607b565b6055565b6040516001600160a01b03909116815260200160405180910390f35b5f81815481106062575f80fd5b5f918252602090912001546001600160a01b0316905081565b5f60208284031215608a575f80fd5b503591905056fea2646970667358221220ba99621db5e1a31586b2a5a82bedfa86ba6ad44116ce59ddf26f95b5e4b9fc9664736f6c63430008180033

and functionDebugData:

"functionDebugData":{"@stakes_4":{"entryPoint":85,"id":4,"parameterSlots":0,"returnSlots":0},"abi_decode_tuple_t_uint256":{"entryPoint":123,"id":null,"parameterSlots":2,"returnSlots":1},"abi_encode_tuple_t_address__to_t_address__fromStack_reversed":{"entryPoint":null,"id":null,"parameterSlots":2,"returnSlots":1}}

Function @stakes_4 is reported to take 0 parameters and have 0 returns.
However if we disassemble the relevant blocks we can see that it consumes 1 item from the stack and leaves 1 (or 2) items on it after its execution is completed.

   0x55: JUMPDEST  
   0x56: PUSH0     
   0x57: DUP2  // copies previous stack item      
   0x58: DUP2      
   0x59: SLOAD     
   0x5a: DUP2      
   0x5b: LT        // uses copied item
   0x5c: PUSH1     0x62
   0x5e: JUMPI     
   0x5f: PUSH0     
   0x60: DUP1      
   0x61: REVERT    
   0x62: JUMPDEST  
   0x63: PUSH0     
   0x64: SWAP2     
   0x65: DUP3      
   0x66: MSTORE    
   0x67: PUSH1     0x20
   0x69: SWAP1     
   0x6a: SWAP2     
   0x6b: SHA3      
   0x6c: ADD       
   0x6d: SLOAD     
   0x6e: PUSH1     0x1
   0x70: PUSH1     0x1
   0x72: PUSH1     0xa0
   0x74: SHL       
   0x75: SUB       
   0x76: AND       // masks the SLOADed value, leaves it on the stack
   0x77: SWAP1     
   0x78: POP       
   0x79: DUP2      
   0x7a: JUMP      // return jump
@cameel
Copy link
Member

cameel commented May 9, 2024

Thanks for the report! I can confirm that this still happens on 0.8.25. And not only for shanghai. At first I thought it could be somehow related to PUSH0, but does not seems so.

I think that this may be the culprit:

size_t params = 0;
size_t returns = 0;
if (auto const* function = dynamic_cast<FunctionDefinition const*>(&_declaration))
{
FunctionType functionType(*function, FunctionType::Kind::Internal);
params = CompilerUtils::sizeOnStack(functionType.parameterTypes());
returns = CompilerUtils::sizeOnStack(functionType.returnParameterTypes());
}

The assumption there may have been that only builtin functions do not have definitions. That's not true - getters don't have them either. Still, just assuming zero does not seem to make much sense for builtins either, so I suspect that there could be other cases affected by this.

@cameel cameel added low effort There is not much implementation work to be done. The task is very easy or tiny. medium impact Default level of impact must have Something we consider an essential part of Solidity 1.0. labels May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 low effort There is not much implementation work to be done. The task is very easy or tiny. medium impact Default level of impact must have Something we consider an essential part of Solidity 1.0.
Projects
None yet
Development

No branches or pull requests

2 participants