Merge pull request #3523 from ethereum/throwMulmod

Make addmod and mulmod revert if the last argument is zero.

5 files changed, 37 insertions, 5 deletions

diff --git a/Changelog.md b/Changelog.md
index 58604ca2..d4e91c26 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -1,6 +1,7 @@
 ### 0.4.21 (unreleased)
 
 Features:
+ * Code Generator: Assert that ``k != 0`` for ``molmod(a, b, k)`` and ``addmod(a, b, k)`` as experimental 0.5.0 feature.
  * Type Checker: Disallow uninitialized storage pointers as experimental 0.5.0 feature.
 
 
diff --git a/docs/miscellaneous.rst b/docs/miscellaneous.rst
index 1c4f918c..328ec6ea 100644
--- a/docs/miscellaneous.rst
+++ b/docs/miscellaneous.rst
@@ -327,8 +327,8 @@ Global Variables
 - ``sha256(...) returns (bytes32)``: compute the SHA-256 hash of the :ref:`(tightly packed) arguments <abi_packed_mode>`
 - ``ripemd160(...) returns (bytes20)``: compute the RIPEMD-160 hash of the :ref:`(tightly packed) arguments <abi_packed_mode>`
 - ``ecrecover(bytes32 hash, uint8 v, bytes32 r, bytes32 s) returns (address)``: recover address associated with the public key from elliptic curve signature, return zero on error
-- ``addmod(uint x, uint y, uint k) returns (uint)``: compute ``(x + y) % k`` where the addition is performed with arbitrary precision and does not wrap around at ``2**256``
-- ``mulmod(uint x, uint y, uint k) returns (uint)``: compute ``(x * y) % k`` where the multiplication is performed with arbitrary precision and does not wrap around at ``2**256``
+- ``addmod(uint x, uint y, uint k) returns (uint)``: compute ``(x + y) % k`` where the addition is performed with arbitrary precision and does not wrap around at ``2**256``. Assert that ``k != 0`` starting from version 0.5.0.
+- ``mulmod(uint x, uint y, uint k) returns (uint)``: compute ``(x * y) % k`` where the multiplication is performed with arbitrary precision and does not wrap around at ``2**256``. Assert that ``k != 0`` starting from version 0.5.0.
 - ``this`` (current contract's type): the current contract, explicitly convertible to ``address``
 - ``super``: the contract one level higher in the inheritance hierarchy
 - ``selfdestruct(address recipient)``: destroy the current contract, sending its funds to the given address
diff --git a/docs/units-and-global-variables.rst b/docs/units-and-global-variables.rst
index ce58cf56..8f49c600 100644
--- a/docs/units-and-global-variables.rst
+++ b/docs/units-and-global-variables.rst
@@ -107,9 +107,9 @@ Mathematical and Cryptographic Functions
 ----------------------------------------
 
 ``addmod(uint x, uint y, uint k) returns (uint)``:
-    compute ``(x + y) % k`` where the addition is performed with arbitrary precision and does not wrap around at ``2**256``.
+    compute ``(x + y) % k`` where the addition is performed with arbitrary precision and does not wrap around at ``2**256``. Assert that ``k != 0`` starting from version 0.5.0.
 ``mulmod(uint x, uint y, uint k) returns (uint)``:
-    compute ``(x * y) % k`` where the multiplication is performed with arbitrary precision and does not wrap around at ``2**256``.
+    compute ``(x * y) % k`` where the multiplication is performed with arbitrary precision and does not wrap around at ``2**256``. Assert that ``k != 0`` starting from version 0.5.0.
 ``keccak256(...) returns (bytes32)``:
     compute the Ethereum-SHA-3 (Keccak-256) hash of the :ref:`(tightly packed) arguments <abi_packed_mode>`
 ``sha256(...) returns (bytes32)``:
diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp
index 8e1cf019..61920592 100644
--- a/libsolidity/codegen/ExpressionCompiler.cpp
+++ b/libsolidity/codegen/ExpressionCompiler.cpp
@@ -765,7 +765,11 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 		case FunctionType::Kind::AddMod:
 		case FunctionType::Kind::MulMod:
 		{
-			for (unsigned i = 0; i < 3; i ++)
+			arguments[2]->accept(*this);
+			utils().convertType(*arguments[2]->annotation().type, IntegerType(256));
+			m_context << Instruction::DUP1 << Instruction::ISZERO;
+			m_context.appendConditionalInvalid();
+			for (unsigned i = 1; i < 3; i ++)
 			{
 				arguments[2 - i]->accept(*this);
 				utils().convertType(*arguments[2 - i]->annotation().type, IntegerType(256));
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index 0611e71d..4d20f4f5 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -7459,6 +7459,33 @@ BOOST_AUTO_TEST_CASE(addmod_mulmod)
 	ABI_CHECK(callContractFunction("test()"), encodeArgs(u256(0)));
 }
 
+BOOST_AUTO_TEST_CASE(addmod_mulmod_zero)
+{
+	char const* sourceCode = R"(
+		contract C {
+			function f() pure returns (uint) {
+				addmod(1, 2, 0);
+				return 2;
+			}
+			function g() pure returns (uint) {
+				mulmod(1, 2, 0);
+				return 2;
+			}
+			function h() pure returns (uint) {
+				mulmod(0, 1, 2);
+				mulmod(1, 0, 2);
+				addmod(0, 1, 2);
+				addmod(1, 0, 2);
+				return 2;
+			}
+		}
+	)";
+	compileAndRun(sourceCode);
+	ABI_CHECK(callContractFunction("f()"), encodeArgs());
+	ABI_CHECK(callContractFunction("g()"), encodeArgs());
+	ABI_CHECK(callContractFunction("h()"), encodeArgs(2));
+}
+
 BOOST_AUTO_TEST_CASE(divisiod_by_zero)
 {
 	char const* sourceCode = R"(