solidity icon indicating copy to clipboard operation
solidity copied to clipboard

[solc] Unassigned immutables cryptic error

Open bshastry opened this issue 4 years ago • 12 comments

contract C1 {
  bool immutable s1 = false;
  constructor() { (int8(127) * 3); }
  function f() external returns (bool) {
    if (s1)
        return true;
    else
        return false;
  }
}
$ solc --experimental-via-ir --optimize test.sol
...
Warning: Source file does not specify required compiler version!
--> test/libsolidity/semanticTests/testYulCompilerOptimisation.sol

Warning: Statement has no effect.
 --> test/libsolidity/semanticTests/testYulCompilerOptimisation.sol:3:19:
  |
3 |   constructor() { (int8(127) * 3); }
  |                   ^^^^^^^^^^^^^^^

Warning: Function state mutability can be restricted to view
 --> test/libsolidity/semanticTests/testYulCompilerOptimisation.sol:4:3:
  |
4 |   function f() external returns (bool) {
  |   ^ (Relevant source part starts here and spans across multiple lines).

Error: Some immutables were read from but never assigned, possibly because of optimization.

Legacy optimizer does not report and error. Legacy and Sol->Yul without optimization don't report either. Both legacy (with and without opt) and Sol->Yul (without opt) result in a runtime failure due to int8 overflow inside the constructor.

bshastry avatar Jul 10 '21 15:07 bshastry

The constructor always reverts (checked arithmetic), and therefore the DeadCodeEliminator removes the code following the revert, which includes setimmutable.

Not sure what should be done in this case. Perhaps improve the error to say that it's likely that the constructor will always revert?

hrkrshnn avatar Jul 12 '21 07:07 hrkrshnn

The constructor always reverts (checked arithmetic), and therefore the DeadCodeEliminator removes the code following the revert, which includes setimmutable.

Yeah, came to that conclusion.

Not sure what should be done in this case. Perhaps improve the error to say that it's likely that the constructor will always revert?

Perhaps the error could say "constructor will always revert". At least that is easier to follow than the somewhat cryptic immutables are unassigned? Just curious if it's always cos of constructor revert though, because if it is not there may be cases where even the new error may not make sense :-)

bshastry avatar Jul 12 '21 11:07 bshastry

The same error will be generated for

contract C {
uint immutable x;
constructor() {
if (false)  { x = 2; }
}
}

chriseth avatar Jul 12 '21 15:07 chriseth

Wondering if this could be a warning instead of an error.

bshastry avatar Jul 14 '21 20:07 bshastry

Small update: This is not solely a Sol->Yul issue, also happens with the legacy compiler (even without optimization turned on)

contract C {
  int8 immutable s4 = 15;
  constructor () {
      for(;;) {}
  }
  function f2() external returns (int8) {
    return s4 / 2;
  }
}
$ solc test.sol
...
Error: Some immutables were read from but never assigned, possibly because of optimization.

bshastry avatar Jul 15 '21 09:07 bshastry

Hmm, shouldn't it be a requirement that an immutable must always be assigned in the constructor? As opposed to allow the "default value" rule?

axic avatar Jul 20 '21 17:07 axic

Hmm, shouldn't it be a requirement that an immutable must always be assigned in the constructor

But how would that be enforced? One can always have a setimmutable inside a conditional branch that will never be reached.

The default value of zero for uninitialized immutable does seem consistent.

hrkrshnn avatar Jul 26 '21 12:07 hrkrshnn

Error: Some immutables were read from but never assigned, possibly because of optimization.

Then why this error? If zero-by-default is allowed, then we do not need this special error.

axic avatar Jul 26 '21 13:07 axic

You cannot assign an immutable in a conditional branch. We do not rely on the default value.

This error is about the optimizer removing the setimmutable and it is an additional low-level check because the high-level check does not catch all situations.

We can improve the error message.

It happens if you have an infinite loop or a require that always fails before you assign the immutable.

chriseth avatar Jul 26 '21 13:07 chriseth

Is this a duplicate of #12864?

cameel avatar Sep 27 '22 00:09 cameel

Is this a duplicate of #12864?

I don't think so. Here the problem is the Yul optimiser removing unconditional setimmutable because it is safe to but then erroring on immutable reaf because the said setimmutable was removed.

There the problem is conditional initialization of immutable.

bshastry avatar Sep 27 '22 08:09 bshastry

To be clear: the fix here is just to improve the error message in codegen. We should have two mechansisms to address situations where immutables are unassigned: one in the high-level language, which should account for the assignment to be syntactically missing - one in codegen, which checks the code after optimization.

The error message could just additionally to possible because of optimization say that the code path assigning the constructor is probably unreachable due to reverting or something like that.

ekpyron avatar Apr 29 '24 12:04 ekpyron