Solidity: Incomplete sourcemaps for constructors when immutables are used

[alcuadrado]

I think there's a bug that makes solc output fewer source maps than instructions. Not that it's emitting sourcemaps that don't point to a file, but there's just fewer source maps than instructions.

I believe this is related to immutable, but I can't confirm it.

Environment

• Compiler version: At least soljson-v0.6.8+commit.0bbfe453.js and soljson-v0.6.9+commit.3e3065ac.js
• Target EVM version (as per compiler settings): latest
• Framework/IDE (e.g. Truffle or Remix): Not relevant
• EVM execution environment / backend / blockchain client: Not relevant
• Operating system: Not relevant

Steps to Reproduce

Compile this without optimizations using soljson-v0.6.9+commit.3e3065ac.js.

pragma solidity ^0.6.0;

contract D {
  uint immutable x = 11;

  function get2X () public returns (uint) {
    return 2*x;
  }

  function fail(uint arg) public returns (uint) {
    revert("D failed");
    return x;
  }

}


contract C {

  function test() public {
    D d = new D();
    d.fail(0);
  }

}

The output you get for D's deployment bytecode is:

          "bytecode": {
            "linkReferences": {},
            "object": "60a0604052600b60809081525034801561001857600080fd5b5060805161016c61003260003980609f525061016c6000f3fe608060405234801561001057600080fd5b50600436106100365760003560e01c806309ba654b1461003b578063132e4f3c14610059575b600080fd5b61004361009b565b6040518082815260200191505060405180910390f35b6100856004803603602081101561006f57600080fd5b81019080803590602001909291905050506100c6565b6040518082815260200191505060405180910390f35b60007f0000000000000000000000000000000000000000000000000000000000000000600202905090565b60006040517f08c379a00000000000000000000000000000000000000000000000000000000081526004018080602001828103825260088152602001807f44206661696c656400000000000000000000000000000000000000000000000081525060200191505060405180910390fdfea2646970667358221220f2632f8405529f2e4d418b0e4668ef248857cdbe66ec75300b4e2857dab9ddd664736f6c63430006090033",
            "opcodes": "PUSH1 0xA0 PUSH1 0x40 MSTORE PUSH1 0xB PUSH1 0x80 SWAP1 DUP2 MSTORE POP CALLVALUE DUP1 ISZERO PUSH2 0x18 JUMPI PUSH1 0x0 DUP1 REVERT JUMPDEST POP PUSH1 0x80 MLOAD PUSH2 0x16C PUSH2 0x32 PUSH1 0x0 CODECOPY DUP1 PUSH1 0x9F MSTORE POP PUSH2 0x16C PUSH1 0x0 RETURN INVALID PUSH1 0x80 PUSH1 0x40 MSTORE CALLVALUE DUP1 ISZERO PUSH2 0x10 JUMPI PUSH1 0x0 DUP1 REVERT JUMPDEST POP PUSH1 0x4 CALLDATASIZE LT PUSH2 0x36 JUMPI PUSH1 0x0 CALLDATALOAD PUSH1 0xE0 SHR DUP1 PUSH4 0x9BA654B EQ PUSH2 0x3B JUMPI DUP1 PUSH4 0x132E4F3C EQ PUSH2 0x59 JUMPI JUMPDEST PUSH1 0x0 DUP1 REVERT JUMPDEST PUSH2 0x43 PUSH2 0x9B JUMP JUMPDEST PUSH1 0x40 MLOAD DUP1 DUP3 DUP2 MSTORE PUSH1 0x20 ADD SWAP2 POP POP PUSH1 0x40 MLOAD DUP1 SWAP2 SUB SWAP1 RETURN JUMPDEST PUSH2 0x85 PUSH1 0x4 DUP1 CALLDATASIZE SUB PUSH1 0x20 DUP2 LT ISZERO PUSH2 0x6F JUMPI PUSH1 0x0 DUP1 REVERT JUMPDEST DUP2 ADD SWAP1 DUP1 DUP1 CALLDATALOAD SWAP1 PUSH1 0x20 ADD SWAP1 SWAP3 SWAP2 SWAP1 POP POP POP PUSH2 0xC6 JUMP JUMPDEST PUSH1 0x40 MLOAD DUP1 DUP3 DUP2 MSTORE PUSH1 0x20 ADD SWAP2 POP POP PUSH1 0x40 MLOAD DUP1 SWAP2 SUB SWAP1 RETURN JUMPDEST PUSH1 0x0 PUSH32 0x0 PUSH1 0x2 MUL SWAP1 POP SWAP1 JUMP JUMPDEST PUSH1 0x0 PUSH1 0x40 MLOAD PUSH32 0x8C379A000000000000000000000000000000000000000000000000000000000 DUP2 MSTORE PUSH1 0x4 ADD DUP1 DUP1 PUSH1 0x20 ADD DUP3 DUP2 SUB DUP3 MSTORE PUSH1 0x8 DUP2 MSTORE PUSH1 0x20 ADD DUP1 PUSH32 0x44206661696C6564000000000000000000000000000000000000000000000000 DUP2 MSTORE POP PUSH1 0x20 ADD SWAP2 POP POP PUSH1 0x40 MLOAD DUP1 SWAP2 SUB SWAP1 REVERT INVALID LOG2 PUSH5 0x6970667358 0x22 SLT KECCAK256 CALLCODE PUSH4 0x2F840552 SWAP16 0x2E 0x4D COINBASE DUP12 0xE CHAINID PUSH9 0xEF248857CDBE66EC75 ADDRESS SIGNEXTEND 0x4E 0x28 JUMPI 0xDA 0xB9 0xDD 0xD6 PUSH5 0x736F6C6343 STOP MOD MULMOD STOP CALLER ",
            "sourceMap": "25:202:0:-:0;;;59:2;40:21;;;;;25:202;;;;;;;;;;;;;;;;;;;"
          },

It has 29 source maps, but there are 32 instructions, excluding the final INVALID.

[alcuadrado]

Also, I was using the number of source maps to figure out where the actual code ends. Is there another way of doing this?

[chriseth]

I think your assessment is correct - the elements in the source maps refer to instructions, but the "assign immutable" pseudo-instruction translates to multiple instructions in the resulting bytecode. This is not too critical, because the source map at this point is usually empty anyway, but we should still fix it.

Your way of finding out where the code ends seems to be pretty solid. You could combine it with a check that there is an invalid opcode right after the code.

[chriseth]

The AssignImmutable assembly item gets translated into multiple opcodes, also depending on how often the immutable was referred (Assembly.cpp:668). In AssemblyItem::computeSourceMapping we need to take this into account and just add as many ; as we have opcodes generated from the AssignImmutable. The information about how many refereces we have needs to be retained for that, too.

[alcuadrado]

This is not too critical, because the source map at this point is usually empty anyway, but we should still fix it.

Can AssignImmutable be inlined in the middle of the bytecode? If that's the case, sourcemaps are pointing to incorrectly sections of the code, right?

You could combine it with a check that there is an invalid opcode right after the code.

I think I'll add this. Thanks

[alcuadrado]

Can AssignImmutable be inlined in the middle of the bytecode? If that's the case, sourcemaps are pointing to incorrectly sections of the code, right?

I think I can reply to this myself now:

1. These unmapped instructions are right before the part of the bytecode that returns the runtime bytecode. Like that part, they would have been mapped to the entire constructor.
2. This somewhat reduces the impact of the issue, because it's safe to assume that any unmapped instruction would have the same sourcemap as the latest mapped one.

Can someone confirm this?

[alcuadrado]

1. These unmapped instructions are right before the part of the bytecode that returns the runtime bytecode

Also, if this is correct, I think a workaround to figure out how many instructions there are is to initially decode as many instructions as the number of source maps. Then continue decoding until you see a 0xfe/Invalid Opcode.

Would this work? Can I be sure that no 0xfe is emitted in that last unmapped part of the deployment bytecode?

[chriseth]

You are right, the AssignImmutable section should be after any user-generated code and before the runtime bytecode is returned. Tracking 0xfe should be reasonably safe.

[alcuadrado]

Thanks for your confirmation Chris.

@iamdefinitelyahuman @nebojsa94 this is probably useful for you too.