From a86c5117132eb8308f37899713197d93c4b852a0 Mon Sep 17 00:00:00 2001 From: a3d4 Date: Wed, 11 Mar 2020 00:32:01 +0100 Subject: [PATCH] Replaced "assert" with "if" (incorrect contract code is not supposed to trigger asserts). --- Changelog.md | 1 + libsolidity/analysis/TypeChecker.cpp | 6 +++--- libsolidity/ast/ASTUtils.cpp | 5 +++-- libsolidity/ast/ASTUtils.h | 5 +++-- libsolidity/codegen/ContractCompiler.cpp | 7 ++++++- .../inlineAssembly/const_from_non_const.sol | 12 ++++++++++++ .../syntaxTests/inlineAssembly/const_from_this.sol | 12 ++++++++++++ 7 files changed, 40 insertions(+), 8 deletions(-) create mode 100644 test/libsolidity/syntaxTests/inlineAssembly/const_from_non_const.sol create mode 100644 test/libsolidity/syntaxTests/inlineAssembly/const_from_this.sol diff --git a/Changelog.md b/Changelog.md index a5d4aab67..492b0c754 100644 --- a/Changelog.md +++ b/Changelog.md @@ -8,6 +8,7 @@ Compiler Features: Bugfixes: + * Inline Assembly: Fix internal error when accessing incorrect constant variables. ### 0.6.4 (2020-03-10) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 5cf003bed..a9b2b5b9f 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -643,14 +643,14 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly) solAssert(var->type(), "Expected variable type!"); if (var->isConstant()) { - var = rootVariableDeclaration(*var); + var = rootConstVariableDeclaration(*var); - if (!var->value()) + if (var && !var->value()) { m_errorReporter.typeError(_identifier.location, "Constant has no value."); return size_t(-1); } - else if (!type(*var)->isValueType() || ( + else if (!var || !type(*var)->isValueType() || ( dynamic_cast(var->value().get()) == nullptr && type(*var->value())->category() != Type::Category::RationalNumber )) diff --git a/libsolidity/ast/ASTUtils.cpp b/libsolidity/ast/ASTUtils.cpp index d903a1b4b..483b75301 100644 --- a/libsolidity/ast/ASTUtils.cpp +++ b/libsolidity/ast/ASTUtils.cpp @@ -21,7 +21,7 @@ namespace solidity::frontend { -VariableDeclaration const* rootVariableDeclaration(VariableDeclaration const& _varDecl) +VariableDeclaration const* rootConstVariableDeclaration(VariableDeclaration const& _varDecl) { solAssert(_varDecl.isConstant(), "Constant variable expected"); @@ -30,7 +30,8 @@ VariableDeclaration const* rootVariableDeclaration(VariableDeclaration const& _v while ((identifier = dynamic_cast(rootDecl->value().get()))) { auto referencedVarDecl = dynamic_cast(identifier->annotation().referencedDeclaration); - solAssert(referencedVarDecl && referencedVarDecl->isConstant(), "Identifier is not referencing a variable declaration"); + if (!referencedVarDecl || !referencedVarDecl->isConstant()) + return nullptr; rootDecl = referencedVarDecl; } return rootDecl; diff --git a/libsolidity/ast/ASTUtils.h b/libsolidity/ast/ASTUtils.h index 7624080a9..af77b60f1 100644 --- a/libsolidity/ast/ASTUtils.h +++ b/libsolidity/ast/ASTUtils.h @@ -22,8 +22,9 @@ namespace solidity::frontend class VariableDeclaration; -/// Find the topmost referenced variable declaration when the given variable +/// Find the topmost referenced constant variable declaration when the given variable /// declaration value is an identifier. Works only for constant variable declarations. -VariableDeclaration const* rootVariableDeclaration(VariableDeclaration const& _varDecl); +/// Returns nullptr if an identifier in the chain is not referencing a constant variable declaration. +VariableDeclaration const* rootConstVariableDeclaration(VariableDeclaration const& _varDecl); } diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 8b9061af3..21886faaf 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -682,7 +682,12 @@ bool ContractCompiler::visit(InlineAssembly const& _inlineAssembly) { if (variable->isConstant()) { - variable = rootVariableDeclaration(*variable); + variable = rootConstVariableDeclaration(*variable); + // If rootConstVariableDeclaration fails and returns nullptr, + // it should have failed in TypeChecker already, causing a compilation error. + // In such case we should not get here. + solAssert(variable, ""); + u256 value; if (variable->value()->annotation().type->category() == Type::Category::RationalNumber) { diff --git a/test/libsolidity/syntaxTests/inlineAssembly/const_from_non_const.sol b/test/libsolidity/syntaxTests/inlineAssembly/const_from_non_const.sol new file mode 100644 index 000000000..fbd5ab6ee --- /dev/null +++ b/test/libsolidity/syntaxTests/inlineAssembly/const_from_non_const.sol @@ -0,0 +1,12 @@ +contract C { + bool nc = false; + bool constant c = nc; + function f() public { + assembly { + let t := c + } + } +} +// ---- +// TypeError: (52-54): Initial value for constant variable has to be compile-time constant. +// TypeError: (112-113): Only direct number constants and references to such constants are supported by inline assembly. diff --git a/test/libsolidity/syntaxTests/inlineAssembly/const_from_this.sol b/test/libsolidity/syntaxTests/inlineAssembly/const_from_this.sol new file mode 100644 index 000000000..563d47851 --- /dev/null +++ b/test/libsolidity/syntaxTests/inlineAssembly/const_from_this.sol @@ -0,0 +1,12 @@ +contract C { + bool constant c = this; + function f() public { + assembly { + let t := c + } + } +} +// ---- +// TypeError: (33-37): Type contract C is not implicitly convertible to expected type bool. +// TypeError: (33-37): Initial value for constant variable has to be compile-time constant. +// TypeError: (95-96): Only direct number constants and references to such constants are supported by inline assembly.