0.8.25 with VIA_IR fails with "InternalCompilerError: Assembly exception for bytecode" (0.8.24 or without via_ir works ok) about solidity HOT 9 CLOSED

@jubeira Your case seems to be resolved too on develop.

Repro instructions directly with solc 0.8.26:

git clone https://github.com/balancer/balancer-v3-monorepo
cd balancer-v3-monorepo/
yarn install
cd pkg/vault/
solc \
    contracts/VaultExplorer.sol \
    @balancer-labs/=../../node_modules/@balancer-labs/ \
    @openzeppelin/=../../node_modules/@openzeppelin/ \
    permit2/=../../node_modules/permit2/ \
    --allow-paths ../../ \
    --bin \
    --via-ir \
    --optimize \
    --optimize-runs 999 \
    --evm-version cancun \
    --yul-optimizations 'dhfoDgvulfnTUtnIf [ xa[r]EscLM cCTUtTOntnfDIul Lcul Vcul [j] Tpeul xa[rul] xa[r]cL gvif CTUca[r]LSsTFOtfDnca[r]Iulc ] jmul[jul] VcTOcul jmul : fDnTOcmu'

Internal compiler error:
/solidity/libsolidity/interface/CompilerStack.cpp(1408): Throw in function void solidity::frontend::CompilerStack::assembleYul(const solidity::frontend::ContractDefinition&, std::shared_ptr<solidity::evmasm::Assembly>, std::shared_ptr<solidity::evmasm::Assembly>)
Dynamic exception type: boost::wrapexcept<solidity::langutil::InternalCompilerError>
std::exception::what: Assembly exception for bytecode
[solidity::util::tag_comment*] = Assembly exception for bytecode

Hope this helps as a clue! I'd love to have this fixed.

The will be released as a part of 0.8.27, some time next week.

from solidity.

By the way, @jubeira, looks like you're using the default sequence from 0.8.25, I'm assuming to avoid the "Stack too deep" errors. You should be able to avoid them by making your assembly memory-safe. Have you tried that?

Thanks for the fix and the hint! We'll give it a shot; we've added memory-safe directives only very recently.

from solidity.

An ICE is not really something that you should be seeing, so it does look like a bug. Looking a bit further into it, the assertion that's triggered is this one.

Was likely introduced here for the 0.8.25 release.

from solidity.

We discussed it on the chat today and it seems to be a regression introduced by the fix for #14863. @ekpyron's diagnosis sounds very plausible:

But yeah, before #14864 we generally overallocated the tag sizes - after that PR, we rely on the code size estimate to be accurate (as in: precise or overestimating) - which is probably broken in a few corner cases (I've seen some in the PUSH0 PR even IIRC) - which would break with that assert in some corner cases - that's my best guess.

Also, regarding the possibility that it's 0.8.24 that is broken and the assertion wasn't being triggered due to incorrect code being generated:

In fact the miscalculation we had in that code before could have lead to something quite similar to that (i.e. allocating too small of space for tags and then just overwriting code when inserting them), but we did already have assertions against that back then

Overall, this is all still speculation since the repro is quite unwieldy (it's a whole repo - a small snippet would be much more helpful) so we'll need to confirm this before fixing, but it's probably correct considering all the info we have so far.

Finally, this bug also highlights a small related issue, i.e. that "assembly exception" for some reason does not include the original message from the assertion that @nikola-matic pointed at. Having it there would have made things clearer from the beginning. We should fix that too.

from solidity.

I have this exact same issue with Balancer V3.

I am using via-ir with the default optimizing steps in 0.8.24 which works fine; 0.8.25 and 0.8.26 do not.
To reproduce, run build instructions (yarn && yarn build) from this commit.

I've narrowed down the problem to the Vault package, and curiously the contract that is causing the problem seems to be VaultExplorer, which is rather simple in comparison to the rest. That branch works if VaultExplorer is deleted from the files or compiled with 0.8.24 as in here.

Hope this helps as a clue! I'd love to have this fixed.

from solidity.

I can confirm that this no longer happens on the current develop, so it seems that #15354 must have fixed it.

On 0.8.26 it was indeed reproducible with:

git clone https://github.com/lens-protocol/core.git
cd core/
forge install
forge build \
    --use 0.8.26 \
    --optimize \
    --via-ir \
    --evm-version cancun \
    --no-cache \
    --skip test

[⠊] Compiling...
[⠒] Compiling 251 files with Solc 0.8.26
[⠢] Solc 0.8.26 finished in 3.99s
Error:
Compiler run failed:
Error: Internal compiler error (/solidity/libsolidity/interface/CompilerStack.cpp:1408):Assembly exception for bytecode
InternalCompilerError: Assembly exception for bytecode

from solidity.

By the way, @jubeira, looks like you're using the default sequence from 0.8.25, I'm assuming to avoid the "Stack too deep" errors. You should be able to avoid them by making your assembly memory-safe. Have you tried that?

from solidity.

Just for the record: the code does not compile with 0.8.26 with via-IR and the new default optimization steps (stack too deep error). All assembly blocks are memory safe as far as I can tell now, so it might be something else.

The previous optimization sequence from 0.8.24/5 does work.

from solidity.

Does the error also say memoryguard was present? If it doesn't, there's still something that prevents the compiler from using stack-to-memory mover, which is the mechanism through which the compiler addresses StackTooDeep in the hardest cases for now. There are some things other than unsafe assembly that could prevent it from kicking in, e.g. recursive functions, though the assembly is the most common cause.

But yeah, unfortunately there are still some rare cases in which even with stack-to-memory mover the Yul->EVM transform is unable to generate code and it could just as well be one of those. The change of the sequence did expose some of those in code that was on the verge of running into StackTooDeep due to stack pressure (though it also helped resolve StackTooDeep in other cases so it's a wash). We're working on improving the transform to address these cases, but if you're running into this then I guess using the old sequence is probably the simplest workaround.

from solidity.