argotorg / argotorg/solidity

`ConstantEvaluator` does not truncate the result of left shift operations

Open
#16,596 0 comments 0 reactions 1 assignee Claimed by @matheusaaguiar View on GitHub
bug :bug:
Dominant language
C++
Stars
25.7k
Forks
6.2k
Avg merge
2d 19h
Merged PRs (30d)
29

Description

The `ConstantEvaluator` attempts to convert the result of a left shift operation to the target type, which fails if the value is out of range.
This happens, for example, when evaluating array lengths or storage layout specifiers.

The behavior is inconsistent with codegen, where the value is cleaned up either lazily (evmasm) or at the end of the operation (IR) to fit the type width.

The [documentation section on shifts](https://docs.soliditylang.org/en/latest/types.html#shifts) describes the correct process, stating that the result of a shift operation has the type of the left operand, overflow checks are never performed and the value is always truncated.

This problem affects only left shifts, since for right shifts the result is always smaller than or equal to the left operand.
Also important to note that expressions formed only by literals have `RationalNumberType` and unlimited precision and are not susceptible to this error.

Minimal repro:
```solidity
// SPDX-License-Identifier: NONE
pragma solidity ^0.8.0;
uint8 constant FIVE = 5;
uint8 constant SIXTEEN = 16;
contract C {

uint8 constant CONST_AND_LITERAL = SIXTEEN << 5;
uint8 constant CONST_ONLY = SIXTEEN << FIVE;

// TypeError 2643: Arithmetic error when computing constant value.
uint[CONST_AND_LITERAL] array1;
// TypeError 2643: Arithmetic error when computing constant value.
uint[CONST_ONLY] array2;
// TypeError 2643: Arithmetic error when computing constant value.
uint[SIXTEEN << 5] array3;
// TypeError 2643: Arithmetic error when computing constant value.
uint[2**255 << ONE] array4;

function f() public pure returns (uint8) {
uint8 x = 16;
// Ok, returns 0
return x << 5;
}
}
```

Analysis
---
After calculating the result of the binary operation, ConstantEvaluator tries to convert the value, which is held as an unlimited-precision rational number, to the type of the operation result.

The computed value of the operation should be truncated to the type of the left operand as stated in the [shift operator documentation section](https://docs.soliditylang.org/en/latest/types.html#shifts).
Note that in this case, literals are assigned either `int256` or `uint256`.
ConstantEvaluator instead of truncating, tries converting the value to the left operand type.
This triggers error `2643`, whenever the value does not fit into the type of the operation result.

Codegen however performs cleanup/masking of the result so it fits properly into the left operand type.

In the evmasm pipeline, the final value is not truncated immediately.
The compiler does not clean the bits if the following operation is not affected, it only performs the cleanup if the next operation uses the value.

On the other hand, the IR pipeline explicitly does the cleanup on the result (see `YulUtilFunctions::typedShiftLeftFunction()`).

Either way, the result is always truncated to the result type.

`ConstantEvaluator` needs to process shift operation results in a separate function/branch from ordinary (mostly arithmetic) operations.
I suggest that a `cleanup` function is added and used after shift operation is evaluated.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.