From b16cdbb57e2f3bdc99b2cf367e40a7f78b4c72ee Mon Sep 17 00:00:00 2001 From: Yoichi Hirai Date: Thu, 10 Nov 2016 11:44:31 +0100 Subject: test: add a test that witnesses #1318 --- test/libsolidity/SolidityEndToEndTest.cpp | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 98ea92ca..62e9a457 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -4466,6 +4466,34 @@ BOOST_AUTO_TEST_CASE(super_overload) BOOST_CHECK(callContractFunction("h()") == encodeArgs(2)); } +BOOST_AUTO_TEST_CASE(bool_conversion) +{ + char const* sourceCode = R"( + contract C { + function f(bool _b) returns(uint) { + if (_b) + return 1; + else + return 0; + } + function g(bool _in) returns (bool _out) { + _out = _in; + } + } + )"; + compileAndRun(sourceCode, 0, "C"); + BOOST_CHECK(callContractFunction("f(bool)", 0) == encodeArgs(0)); + BOOST_CHECK(callContractFunction("f(bool)", 1) == encodeArgs(1)); + BOOST_CHECK(callContractFunction("f(bool)", 2) == encodeArgs(1)); + BOOST_CHECK(callContractFunction("f(bool)", 3) == encodeArgs(1)); + BOOST_CHECK(callContractFunction("f(bool)", 255) == encodeArgs(1)); + BOOST_CHECK(callContractFunction("g(bool)", 0) == encodeArgs(0)); + BOOST_CHECK(callContractFunction("g(bool)", 1) == encodeArgs(1)); + BOOST_CHECK(callContractFunction("g(bool)", 2) == encodeArgs(1)); + BOOST_CHECK(callContractFunction("g(bool)", 3) == encodeArgs(1)); + BOOST_CHECK(callContractFunction("g(bool)", 255) == encodeArgs(1)); +} + BOOST_AUTO_TEST_CASE(packed_storage_signed) { char const* sourceCode = R"( -- cgit From 03ccc6df704aae4ea19698f0167798013c14536e Mon Sep 17 00:00:00 2001 From: Yoichi Hirai Date: Thu, 10 Nov 2016 11:33:15 +0100 Subject: codegen: truncate a boolean calldata down to one bit --- Changelog.md | 1 + libsolidity/codegen/CompilerUtils.cpp | 2 ++ 2 files changed, 3 insertions(+) diff --git a/Changelog.md b/Changelog.md index b5e48173..86e0125a 100644 --- a/Changelog.md +++ b/Changelog.md @@ -5,6 +5,7 @@ Features: Bugfixes: * Type checker: string literals that are not valid UTF-8 cannot be converted to string type + * Code generator: higher bits in a boolean argument are ignored. ### 0.4.6 (2016-11-22) diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp index d5361ac6..ff0f8b9c 100644 --- a/libsolidity/codegen/CompilerUtils.cpp +++ b/libsolidity/codegen/CompilerUtils.cpp @@ -925,6 +925,8 @@ unsigned CompilerUtils::loadFromMemoryHelper(Type const& _type, bool _fromCallda if (leftAligned) m_context << shiftFactor << Instruction::MUL; } + if (_fromCalldata && _type.category() == Type::Category::Bool) + m_context << Instruction::ISZERO << Instruction::ISZERO; return numBytes; } -- cgit From 0123e74a2eeac35bfa55886d0c6db391c07e7ec6 Mon Sep 17 00:00:00 2001 From: Yoichi Hirai Date: Thu, 10 Nov 2016 14:41:50 +0100 Subject: codegen: cleanup booleans before storing them into memory --- libsolidity/codegen/CompilerUtils.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp index ff0f8b9c..ce5bb1d2 100644 --- a/libsolidity/codegen/CompilerUtils.cpp +++ b/libsolidity/codegen/CompilerUtils.cpp @@ -950,6 +950,8 @@ unsigned CompilerUtils::prepareMemoryStore(Type const& _type, bool _padToWordBou else { solAssert(numBytes <= 32, "Memory store of more than 32 bytes requested."); + if (_type.category() == Type::Category::Bool) + m_context << Instruction::ISZERO << Instruction::ISZERO; if (numBytes != 32 && !leftAligned && !_padToWordBoundaries) // shift the value accordingly before storing m_context << (u256(1) << ((32 - numBytes) * 8)) << Instruction::MUL; -- cgit From fb9babce54f76251b9616f192822e0c015411159 Mon Sep 17 00:00:00 2001 From: Yoichi Hirai Date: Thu, 10 Nov 2016 15:38:08 +0100 Subject: codegen: truncate booleans before they enter storage --- libsolidity/codegen/LValue.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/libsolidity/codegen/LValue.cpp b/libsolidity/codegen/LValue.cpp index 23fe2d4e..b9e141d8 100644 --- a/libsolidity/codegen/LValue.cpp +++ b/libsolidity/codegen/LValue.cpp @@ -223,7 +223,6 @@ void StorageItem::storeValue(Type const& _sourceType, SourceLocation const& _loc { solAssert(m_dataType->storageBytes() <= 32, "Invalid storage bytes size."); solAssert(m_dataType->storageBytes() > 0, "Invalid storage bytes size."); - if (m_dataType->storageBytes() == 32) { solAssert(m_dataType->sizeOnStack() == 1, "Invalid stack size."); -- cgit From 7959ee49beb664238e58dcb589f386b2dff7438c Mon Sep 17 00:00:00 2001 From: Yoichi Hirai Date: Thu, 10 Nov 2016 15:56:12 +0100 Subject: docs: describe when and how overflown values are cleaned --- Changelog.md | 2 +- docs/miscellaneous.rst | 52 +++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/Changelog.md b/Changelog.md index 86e0125a..ea1b1b58 100644 --- a/Changelog.md +++ b/Changelog.md @@ -5,7 +5,7 @@ Features: Bugfixes: * Type checker: string literals that are not valid UTF-8 cannot be converted to string type - * Code generator: higher bits in a boolean argument are ignored. + * Code generator: any non-zero value given as a boolean argument is now converted into 1. ### 0.4.6 (2016-11-22) diff --git a/docs/miscellaneous.rst b/docs/miscellaneous.rst index 9f733979..7e9cee44 100644 --- a/docs/miscellaneous.rst +++ b/docs/miscellaneous.rst @@ -56,6 +56,8 @@ So for the following contract snippet:: The position of ``data[4][9].b`` is at ``keccak256(uint256(9) . keccak256(uint256(4) . uint256(1))) + 1``. +.. index: memory layout + **************** Layout in Memory **************** @@ -72,7 +74,8 @@ Solidity always places new objects at the free memory pointer and memory is neve .. warning:: There are some operations in Solidity that need a temporary memory area larger than 64 bytes and therefore will not fit into the scratch space. They will be placed where the free memory points to, but given their short lifecycle, the pointer is not updated. The memory may or may not be zeroed out. Because of this, one shouldn't expect the free memory to be zeroed out. -.. index: memory layout + +.. index: calldata layout ******************* Layout of Call Data @@ -85,6 +88,53 @@ specification ABI specification requires arguments to be padded to multiples of 32 bytes. The internal function calls use a different convention. + +.. index: overflow + +**************************************** +Internals - Cleaning Up Overflows in EVM +**************************************** + +When a value is shorter than 256-bit, sometimes the remaining bits +must be cleaned. +The Solidity compiler is designed to clean such remaining bits before any operations +that might be broken by the garbage in the remaining bits. For +example, before writing a value to the memory, the remaining bits need +to be cleared because the memory contents can be used for computing +hashes or sent as the data of a message call. Similarly, before +storing a value in the storage, the remaining bits need to be cleaned +because otherwise the garbled value can be observed. + +On the other hand, we do not clean the bits if the immediately +following operation is not affected. For instance, since any non-zero +value is considered ``true`` by ``JUMPI`` instruction, we do not clean +the boolean values before they are used as the condition for +``JUMPI``. + +Different types have different rules for cleaning up overflows: + ++---------------+---------------+------------------+ +|Type |Cleaned Form |Overflow Means | ++===============+===============+==================+ +|enum of n |0 until n - 1 |exception | +|members | | | ++---------------+---------------+------------------+ +|bool |0 or 1 |zero or nonzero | ++---------------+---------------+------------------+ +|signed integers|sign-extended |currently silently| +| |word |wraps; in the | +| | |future exceptions | +| | |will be thrown | +| | | | +| | | | ++---------------+---------------+------------------+ +|unsigned |higher bits |currently silently| +|integers |zeroed |wraps; in the | +| | |future exceptions | +| | |will be thrown | ++---------------+---------------+------------------+ + + ***************** Esoteric Features ***************** -- cgit From 547deec4be55fc10b44de9ff92bb2d598d5b04f5 Mon Sep 17 00:00:00 2001 From: Yoichi Hirai Date: Thu, 24 Nov 2016 11:57:28 +0100 Subject: codegen: clean any data from the input --- docs/miscellaneous.rst | 3 +++ libsolidity/codegen/CompilerUtils.cpp | 4 ++-- test/libsolidity/Assembly.cpp | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/miscellaneous.rst b/docs/miscellaneous.rst index 7e9cee44..251d77d2 100644 --- a/docs/miscellaneous.rst +++ b/docs/miscellaneous.rst @@ -111,6 +111,9 @@ value is considered ``true`` by ``JUMPI`` instruction, we do not clean the boolean values before they are used as the condition for ``JUMPI``. +In addition to the design principle above, the Solidity compiler +cleans input data when it is loaded onto the stack. + Different types have different rules for cleaning up overflows: +---------------+---------------+------------------+ diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp index ce5bb1d2..6763e995 100644 --- a/libsolidity/codegen/CompilerUtils.cpp +++ b/libsolidity/codegen/CompilerUtils.cpp @@ -925,8 +925,8 @@ unsigned CompilerUtils::loadFromMemoryHelper(Type const& _type, bool _fromCallda if (leftAligned) m_context << shiftFactor << Instruction::MUL; } - if (_fromCalldata && _type.category() == Type::Category::Bool) - m_context << Instruction::ISZERO << Instruction::ISZERO; + if (_fromCalldata) + convertType(_type, _type, true); return numBytes; } diff --git a/test/libsolidity/Assembly.cpp b/test/libsolidity/Assembly.cpp index cc77bd4c..ed92ca2b 100644 --- a/test/libsolidity/Assembly.cpp +++ b/test/libsolidity/Assembly.cpp @@ -116,7 +116,7 @@ BOOST_AUTO_TEST_CASE(location_test) shared_ptr n = make_shared(""); AssemblyItems items = compileContract(sourceCode); vector locations = - vector(16, SourceLocation(2, 75, n)) + + vector(18, SourceLocation(2, 75, n)) + vector(27, SourceLocation(20, 72, n)) + vector{SourceLocation(42, 51, n), SourceLocation(65, 67, n)} + vector(2, SourceLocation(58, 67, n)) + -- cgit From 5d7a1fda39d29329df675985b0d3e50548348b72 Mon Sep 17 00:00:00 2001 From: Yoichi Hirai Date: Fri, 25 Nov 2016 10:03:39 +0100 Subject: docs: remove the word overflow when we are talking about invalid values --- docs/miscellaneous.rst | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/docs/miscellaneous.rst b/docs/miscellaneous.rst index 251d77d2..6e1b575a 100644 --- a/docs/miscellaneous.rst +++ b/docs/miscellaneous.rst @@ -114,28 +114,28 @@ the boolean values before they are used as the condition for In addition to the design principle above, the Solidity compiler cleans input data when it is loaded onto the stack. -Different types have different rules for cleaning up overflows: - -+---------------+---------------+------------------+ -|Type |Cleaned Form |Overflow Means | -+===============+===============+==================+ -|enum of n |0 until n - 1 |exception | -|members | | | -+---------------+---------------+------------------+ -|bool |0 or 1 |zero or nonzero | -+---------------+---------------+------------------+ -|signed integers|sign-extended |currently silently| -| |word |wraps; in the | -| | |future exceptions | -| | |will be thrown | -| | | | -| | | | -+---------------+---------------+------------------+ -|unsigned |higher bits |currently silently| -|integers |zeroed |wraps; in the | -| | |future exceptions | -| | |will be thrown | -+---------------+---------------+------------------+ +Different types have different rules for cleaning up invalid values: + ++---------------+---------------+-------------------+ +|Type |Valid Valies |Invalid Values Mean| ++===============+===============+===================+ +|enum of n |0 until n - 1 |exception | +|members | | | ++---------------+---------------+-------------------+ +|bool |0 or 1 |1 | ++---------------+---------------+-------------------+ +|signed integers|sign-extended |currently silently | +| |word |wraps; in the | +| | |future exceptions | +| | |will be thrown | +| | | | +| | | | ++---------------+---------------+-------------------+ +|unsigned |higher bits |currently silently | +|integers |zeroed |wraps; in the | +| | |future exceptions | +| | |will be thrown | ++---------------+---------------+-------------------+ ***************** -- cgit From d77c8f730ce6a7c4caa86bb8e3bf1e5ca13b2117 Mon Sep 17 00:00:00 2001 From: Yoichi Hirai Date: Fri, 25 Nov 2016 10:17:40 +0100 Subject: codegen: clean not only booleans but all types before storing them into memory --- libsolidity/codegen/CompilerUtils.cpp | 5 ++--- libsolidity/codegen/CompilerUtils.h | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp index 6763e995..edf457a1 100644 --- a/libsolidity/codegen/CompilerUtils.cpp +++ b/libsolidity/codegen/CompilerUtils.cpp @@ -941,7 +941,7 @@ void CompilerUtils::cleanHigherOrderBits(IntegerType const& _typeOnStack) m_context << ((u256(1) << _typeOnStack.numBits()) - 1) << Instruction::AND; } -unsigned CompilerUtils::prepareMemoryStore(Type const& _type, bool _padToWordBoundaries) const +unsigned CompilerUtils::prepareMemoryStore(Type const& _type, bool _padToWordBoundaries) { unsigned numBytes = _type.calldataEncodedSize(_padToWordBoundaries); bool leftAligned = _type.category() == Type::Category::FixedBytes; @@ -950,8 +950,7 @@ unsigned CompilerUtils::prepareMemoryStore(Type const& _type, bool _padToWordBou else { solAssert(numBytes <= 32, "Memory store of more than 32 bytes requested."); - if (_type.category() == Type::Category::Bool) - m_context << Instruction::ISZERO << Instruction::ISZERO; + convertType(_type, _type, true); if (numBytes != 32 && !leftAligned && !_padToWordBoundaries) // shift the value accordingly before storing m_context << (u256(1) << ((32 - numBytes) * 8)) << Instruction::MUL; diff --git a/libsolidity/codegen/CompilerUtils.h b/libsolidity/codegen/CompilerUtils.h index 4baf48ff..0a5d8e1c 100644 --- a/libsolidity/codegen/CompilerUtils.h +++ b/libsolidity/codegen/CompilerUtils.h @@ -185,7 +185,7 @@ private: void cleanHigherOrderBits(IntegerType const& _typeOnStack); /// Prepares the given type for storing in memory by shifting it if necessary. - unsigned prepareMemoryStore(Type const& _type, bool _padToWordBoundaries) const; + unsigned prepareMemoryStore(Type const& _type, bool _padToWordBoundaries); /// Loads type from memory assuming memory offset is on stack top. unsigned loadFromMemoryHelper(Type const& _type, bool _fromCalldata, bool _padToWordBoundaries); -- cgit From 868a8a8fa03fc3cf47fd66335a7924d7876f1c0e Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Wed, 30 Nov 2016 11:00:27 +0000 Subject: docs: update overflow cleanup wording --- docs/miscellaneous.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/miscellaneous.rst b/docs/miscellaneous.rst index 6e1b575a..963edf26 100644 --- a/docs/miscellaneous.rst +++ b/docs/miscellaneous.rst @@ -95,11 +95,11 @@ bytes. The internal function calls use a different convention. Internals - Cleaning Up Overflows in EVM **************************************** -When a value is shorter than 256-bit, sometimes the remaining bits +When a value is shorter than 256-bit, in some cases the remaining bits must be cleaned. The Solidity compiler is designed to clean such remaining bits before any operations -that might be broken by the garbage in the remaining bits. For -example, before writing a value to the memory, the remaining bits need +that might be adversely affected by the potential garbage in the remaining bits. +For example, before writing a value to the memory, the remaining bits need to be cleared because the memory contents can be used for computing hashes or sent as the data of a message call. Similarly, before storing a value in the storage, the remaining bits need to be cleaned @@ -117,7 +117,7 @@ cleans input data when it is loaded onto the stack. Different types have different rules for cleaning up invalid values: +---------------+---------------+-------------------+ -|Type |Valid Valies |Invalid Values Mean| +|Type |Valid Values |Invalid Values Mean| +===============+===============+===================+ |enum of n |0 until n - 1 |exception | |members | | | -- cgit From e7760417e83cf9e313c76cdd44f860aeec1b798c Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Thu, 1 Dec 2016 12:14:21 +0000 Subject: docs: rename overflow to variable cleanup --- docs/miscellaneous.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/miscellaneous.rst b/docs/miscellaneous.rst index 963edf26..56716dca 100644 --- a/docs/miscellaneous.rst +++ b/docs/miscellaneous.rst @@ -89,11 +89,11 @@ ABI specification requires arguments to be padded to multiples of 32 bytes. The internal function calls use a different convention. -.. index: overflow +.. index: variable cleanup -**************************************** -Internals - Cleaning Up Overflows in EVM -**************************************** +********************************* +Internals - Cleaning Up Variables +********************************* When a value is shorter than 256-bit, in some cases the remaining bits must be cleaned. -- cgit