Solidity: Run AsmAnalyzer after each yul optimizer test.

[xwvvvvwx]

Description

I stumbled over a few yul optimizer test cases that seem potentially incorrect, and I thought it was worth raising an issue here to discuss.

Test Cases

1. test/libyul/yulOptimizerTests/fullsuite/stack_compressor_msize.yul

The expected output contains the following function defintion:

function foo_singlereturn_1(in, in_1) -> out
{ extcodecopy(1, msize(), 1, 1) }

Which causes the following error when compiled:

Error: Expected identifier but got reserved keyword 'in'
  --> /tmp/tmp.CT5gEi0NOm:19:34:
   |
19 |      function foo_singlereturn_1(in, in_1) -> out
   |                                  ^^

This seems like an example of the optimizer generating invalid code?

1. test/libyul/yulOptimizerTests/expressionSplitter/object_access.yul

The input to the test contains a data object abc. Maybe this is just a quirk of the test format that I'm not understanding, but this data object appears to have been incorrectly eliminated from the program in the test output, leading to a compilation failure with the following error:

Error: Unknown data object "abc".
 --> /tmp/tmp.jmSwm42qoW:4:15:
  |
4 |      let a := datasize("abc")
  |               ^^^^^^^^

Error: Unknown data object "abc".
 --> /tmp/tmp.jmSwm42qoW:5:15:
  |
5 |      let x := dataoffset("abc")
  |               ^^^^^^^^^^

Environment

• Compiler version: solc 0.6.7
• Operating system: nixos

[axic]

Nice catch!

The first issue (using keywords) we have actually this discussed under #9330 and there are a few outstanding PRs addressing this.

The second issue we discussed today on gitter. My opinion is we should change the test output/expectation to be in the object format not to miss any details. The input can be flexible and accept both the object and simplified code formats.

[xwvvvvwx]

ah nice that there is a PR for the first issue already :slightly_smiling_face:

Switching to the object format for test outputs seems like a sensible fix for the second one :+1:

[xwvvvvwx]

I think that I may have found two more invalid tests:

1. test/libyul/yulOptimizerTests/varNameCleaner/function_names.yul

This test produces code with a duplicated variable name which fails to compile with:

Error: Variable name f_1 already taken in this scope.
 --> hi.yul:4:7:
  |
4 |     { let f_1 }
  |       ^^^^^^^

1. test/libyul/yulOptimizerTests/commonSubexpressionEliminator/object_access.yul

This is subject to the same issue with the missing data object issue as the test in the expressionSplitter suite.

[chriseth]

I think we should maybe just run a validation step on the output as part of the test :)

[xwvvvvwx]

What would the validation step look like? Many tests do not produce code that can be compiled by solc.

As far as I can tell the reasons for this are the following:

• usage of typed yul
• stack to deep errors
• wrong number of args to to opcodes (these are all in the wordSizeTransform sutie, and according to @leonardoalt are expected)

[chriseth]

The wordSizeTransform is indeed a problem, but I think we can exclude it. For the others, we should at least do run the AsmAnalyzer even if we cannot do full assembly to bytecode.

[axic]

Issue 2. and https://github.com/ethereum/solidity/issues/9500#issuecomment-663874400 have been fixed (via #9572 and #9575).

Issue 1. will be fixed with #9329.

Btw, @xwvvvvwx usually it is better to create separate issues so we can clearly close them in each PR.

[chriseth]

Fixed according to the comment by @axic above.