Merge pull request #2481 from ethereum/mload-optim

MLOAD has side-effects, treat it like that in the optimiser

Changelog.md

@@ -20,6 +20,7 @@ Features:
  * Static Analyzer: Warn about deprecation of ``callcode``.
 
 Bugfixes:
+ * Assembly: mark ``MLOAD`` to have side effects in the optimiser.
  * Code generator: Use ``REVERT`` instead of ``INVALID`` for generated input validation routines.
  * Type Checker: Fix address literals not being treated as compile-time constants.
  * Type Checker: Disallow invoking the same modifier multiple times.
@@ -33,6 +34,7 @@ Bugfixes:
  * Code Generator: Fix negative stack size checks.
  * Inline Assembly: Enforce function arguments when parsing functional instructions.
  * Fixed segfault with constant function parameters
+ * Optimizer: Disallow optimizations involving ``MLOAD`` because it changes ``MSIZE``.
 
 ### 0.4.11 (2017-05-03)
 

libevmasm/Instruction.cpp

@@ -216,7 +216,7 @@ static const std::map<Instruction, InstructionInfo> c_instructionInfo =
 	{ Instruction::DIFFICULTY,	{ "DIFFICULTY",		0, 0, 1, false, Tier::Base } },
 	{ Instruction::GASLIMIT,	{ "GASLIMIT",		0, 0, 1, false, Tier::Base } },
 	{ Instruction::POP,			{ "POP",			0, 1, 0, false, Tier::Base } },
-	{ Instruction::MLOAD,		{ "MLOAD",			0, 1, 1, false, Tier::VeryLow } },
+	{ Instruction::MLOAD,		{ "MLOAD",			0, 1, 1, true, Tier::VeryLow } },
 	{ Instruction::MSTORE,		{ "MSTORE",			0, 2, 0, true, Tier::VeryLow } },
 	{ Instruction::MSTORE8,		{ "MSTORE8",		0, 2, 0, true, Tier::VeryLow } },
 	{ Instruction::SLOAD,		{ "SLOAD",			0, 1, 1, false, Tier::Special } },

test/libsolidity/SolidityOptimizer.cpp

@@ -697,59 +697,6 @@ BOOST_AUTO_TEST_CASE(cse_interleaved_storage_at_known_location_offset)
 	});
 }
 
-BOOST_AUTO_TEST_CASE(cse_interleaved_memory_at_known_location_offset)
-{
-	// stores and reads to/from two locations which are known to be different,
-	// should not optimize away the first store, because the location overlaps with the load,
-	// but it should optimize away the second, because we know that the location is different by 32
-	AssemblyItems input{
-		u256(0x50),
-		Instruction::DUP2,
-		u256(2),
-		Instruction::ADD,
-		Instruction::MSTORE, // ["DUP1"+2] = 0x50
-		u256(0x60),
-		Instruction::DUP2,
-		u256(32),
-		Instruction::ADD,
-		Instruction::MSTORE, // ["DUP1"+32] = 0x60
-		Instruction::DUP1,
-		Instruction::MLOAD, // read from "DUP1"
-		u256(0x70),
-		Instruction::DUP3,
-		u256(32),
-		Instruction::ADD,
-		Instruction::MSTORE, // ["DUP1"+32] = 0x70
-		u256(0x80),
-		Instruction::DUP3,
-		u256(2),
-		Instruction::ADD,
-		Instruction::MSTORE, // ["DUP1"+2] = 0x80
-	};
-	// If the actual code changes too much, we could also simply check that the output contains
-	// exactly 3 MSTORE and exactly 1 MLOAD instruction.
-	checkCSE(input, {
-		u256(0x50),
-		u256(2),
-		Instruction::DUP3,
-		Instruction::ADD,
-		Instruction::SWAP1,
-		Instruction::DUP2,
-		Instruction::MSTORE, // ["DUP1"+2] = 0x50
-		Instruction::DUP2,
-		Instruction::MLOAD, // read from "DUP1"
-		u256(0x70),
-		u256(32),
-		Instruction::DUP5,
-		Instruction::ADD,
-		Instruction::MSTORE, // ["DUP1"+32] = 0x70
-		u256(0x80),
-		Instruction::SWAP1,
-		Instruction::SWAP2,
-		Instruction::MSTORE // ["DUP1"+2] = 0x80
-	});
-}
-
 BOOST_AUTO_TEST_CASE(cse_deep_stack)
 {
 	AssemblyItems input{