From 2cdd3b20818a472bf94c66713acdf6674a8df652 Mon Sep 17 00:00:00 2001 From: hrkrshnn Date: Mon, 6 Sep 2021 12:02:55 +0200 Subject: [PATCH] Resolving Keccak-256: check if arguments are identifiers early. Previously, the check on whether the optimization was useful gas wise was done before checking if the keccak256 opcode had identifier as arguments. Since the gas meter crashes when encountering certain Yul opcodes (create, dataoffset, etc.), this optimizer step crashed. --- Changelog.md | 1 + libyul/optimiser/LoadResolver.cpp | 14 +++++++------- .../loadResolver/keccak_crash.yul | 13 +++++++++++++ 3 files changed, 21 insertions(+), 7 deletions(-) create mode 100644 test/libyul/yulOptimizerTests/loadResolver/keccak_crash.yul diff --git a/Changelog.md b/Changelog.md index b1a350350..f363d7164 100644 --- a/Changelog.md +++ b/Changelog.md @@ -12,6 +12,7 @@ Compiler Features: * SMTChecker: Add constraints to better correlate ``address(this).balance`` and ``msg.value``. * SMTChecker: Support the ``value`` option for external function calls. * Commandline Interface: Disallowed the ``--experimental-via-ir`` option to be used with Standard Json, Assembler and Linker modes. + * Yul Optimizer: Fix a crash in LoadResolver, when ``keccak256`` has particular non-identifier arguments. Bugfixes: diff --git a/libyul/optimiser/LoadResolver.cpp b/libyul/optimiser/LoadResolver.cpp index b968e1bef..550ca6fb3 100644 --- a/libyul/optimiser/LoadResolver.cpp +++ b/libyul/optimiser/LoadResolver.cpp @@ -97,6 +97,13 @@ void LoadResolver::tryEvaluateKeccak( std::vector const& _arguments ) { + yulAssert(_arguments.size() == 2, ""); + Identifier const* memoryKey = std::get_if(&_arguments.at(0)); + Identifier const* length = std::get_if(&_arguments.at(1)); + + if (!memoryKey || !length) + return; + // The costs are only correct for hashes of 32 bytes or 1 word (when rounded up). GasMeter gasMeter{ dynamic_cast(m_dialect), @@ -122,13 +129,6 @@ void LoadResolver::tryEvaluateKeccak( if (costOfLiteral > costOfKeccak) return; - yulAssert(_arguments.size() == 2, ""); - Identifier const* memoryKey = std::get_if(&_arguments.at(0)); - Identifier const* length = std::get_if(&_arguments.at(1)); - - if (!memoryKey || !length) - return; - auto memoryValue = util::valueOrNullptr(m_memory, memoryKey->name); if (memoryValue && inScope(*memoryValue)) { diff --git a/test/libyul/yulOptimizerTests/loadResolver/keccak_crash.yul b/test/libyul/yulOptimizerTests/loadResolver/keccak_crash.yul new file mode 100644 index 000000000..dd6a164eb --- /dev/null +++ b/test/libyul/yulOptimizerTests/loadResolver/keccak_crash.yul @@ -0,0 +1,13 @@ +// This test used to crash: https://github.com/ethereum/solidity/issues/11801 +{ + for {} addmod(keccak256(0x0,create(0x0, 0x0, 0x0)), 0x0, 0x0) {} {} +} +// ---- +// step: loadResolver +// +// { +// for { } +// addmod(keccak256(0x0, create(0x0, 0x0, 0x0)), 0x0, 0x0) +// { } +// { } +// }