[Optimizer] Legacy optimizer precomputes successive keccak256 from the same location but different size incorrectly

[bshastry]

Description

The following test does not revert when run on an EVM client after it has been optimised by the legacy optimiser (i.e., solc --optimize test.sol).

contract C {
  uint[] data;

  function val() public {
    assembly {
        sstore(0, 2)
        mstore(0, 0)
        sstore(keccak256(0, 32), 234)
        sstore(add(keccak256(0, 23), 1), 123)
    }
    assert(data[1] == 123);
  }
}

The following assembly code is produced (only relevant snippet has been pasted)

...
      0x02
        /* "test/libsolidity/semanticTests/test.sol":85:86  0 */
      0x00
        /* "test/libsolidity/semanticTests/test.sol":78:90  sstore(0, 2) */
      swap1
      dup2
      sstore
        /* "test/libsolidity/semanticTests/test.sol":99:111  mstore(0, 0) */
      dup1
      mstore
        /* "test/libsolidity/semanticTests/test.sol":145:148  234 */
      0xea
        /* "test/libsolidity/semanticTests/test.sol":127:143  keccak256(0, 32) */
      0x290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563
        /* "test/libsolidity/semanticTests/test.sol":120:149  sstore(keccak256(0, 32), 234) */
      sstore
        /* "test/libsolidity/semanticTests/test.sol":189:192  123 */
      0x7b
        /* "test/libsolidity/semanticTests/test.sol":165:187  add(keccak256(0,23),1) */
      0x290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e564
        /* "test/libsolidity/semanticTests/test.sol":158:193  sstore(add(keccak256(0,23),1), 123) */
      sstore

The values of interest are 0x290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563 (keccak256(0, 32)) and 0x290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e564 (add(keccak256(0, 23), 1)).

The bug is that the legacy optimiser "pre-computes" keccak256(0, 23) to be somehow equal to keccak256(0, 32) which is incorrect. Although the value at memory location is zero, keccak256(0, 32) = 0x290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563 but keccak256(0, 23) = 0xe2b9f9f9430b05bfa9a3abd3bac9a181434d23a707ef1cde8bd25d30203538d8.

Because of the incorrect code, it turns out that data[1] == 123 although the assembly operation sstore(add(keccak256(0, 23), 1), 123) is actually storing the value 123 to some unrelated storage slot.

Please note that the old code generator without optimisation enabled produces correct result (failing assertion because both the keccaks are not precomputed). The same holds true for new code gen with or without optimisation enabled.

Environment

• Compiler version: 6e1d61a
• Target EVM version (as per compiler settings): NA

Steps to Reproduce

• Copy paste test into Remix, compile with optimisation enabled (defaulting to 200 runs)
• Deploy and run val()
• Expected result: Run should fail
• Actual result: Run succeeds

[bshastry]

At first glance, it looks like this may be the root cause (HT @hrkrshnn )

solidity/libevmasm/KnownState.cpp

Lines 381 to 390 in 6e1d61a

	for (u256 i = 0; i < *l; i += 32)
	{
	Id slot = m_expressionClasses->find(
	AssemblyItem(Instruction::ADD, _location),
	{_start, m_expressionClasses->find(i)}
	);
	arguments.push_back(loadFromMemory(slot, _location));
	}
	if (m_knownKeccak256Hashes.count(arguments))
	return m_knownKeccak256Hashes.at(arguments);

It looks like loadFromMemory always works at 32-byte granularity, so the fact that we are actually doing a sub-32-byte-word keccak is not taken into consideration. Anyway, good to get a quick ack, thanks once again for the pointer hrkrshnn