From 0e471ab811a8e2c2b274187e328b1d602a84490a Mon Sep 17 00:00:00 2001
From: chriseth <chris@ethereum.org>
Date: Mon, 18 Feb 2019 15:16:14 +0100
Subject: [PATCH] Review comments.

---
 test/tools/yulInterpreter/CMakeLists.txt        |  1 -
 .../EVMInstructionInterpreter.cpp               | 17 ++++++++++++-----
 .../yulInterpreter/EVMInstructionInterpreter.h  | 13 +++++++------
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/test/tools/yulInterpreter/CMakeLists.txt b/test/tools/yulInterpreter/CMakeLists.txt
index 5f1b138e1..52fe0e3c6 100644
--- a/test/tools/yulInterpreter/CMakeLists.txt
+++ b/test/tools/yulInterpreter/CMakeLists.txt
@@ -3,7 +3,6 @@ set(sources
 	EVMInstructionInterpreter.cpp
 	Interpreter.h
 	Interpreter.cpp
-
 )
 
 add_library(yulInterpreter ${sources})
diff --git a/test/tools/yulInterpreter/EVMInstructionInterpreter.cpp b/test/tools/yulInterpreter/EVMInstructionInterpreter.cpp
index 79ee94df9..c829ad1fa 100644
--- a/test/tools/yulInterpreter/EVMInstructionInterpreter.cpp
+++ b/test/tools/yulInterpreter/EVMInstructionInterpreter.cpp
@@ -50,6 +50,9 @@ u256 exp256(u256 _base, u256 _exponent)
 	return result;
 }
 
+/// Reads 32 bytes from @a _data at position @a _offset bytes while
+/// interpreting @a _data to be padded with an infinite number of zero
+/// bytes beyond its end.
 u256 readZeroExtended(bytes const& _data, u256 const& _offset)
 {
 	if (_offset >= _data.size())
@@ -70,6 +73,10 @@ u256 readZeroExtended(bytes const& _data, u256 const& _offset)
 	}
 }
 
+/// Copy @a _size bytes of @a _source at offset @a _sourceOffset to
+/// @a _target at offset @a _targetOffset. Behaves as if @a _source would
+/// continue with an infinite sequence of zero bytes beyond its end.
+/// Asserts the target is large enough to hold the copied segment.
 void copyZeroExtended(
 	bytes& _target, bytes const& _source,
 	size_t _targetOffset, size_t _sourceOffset, size_t _size
@@ -137,11 +144,11 @@ u256 EVMInstructionInterpreter::eval(
 	case Instruction::XOR:
 		return arg[0] ^ arg[1];
 	case Instruction::BYTE:
-		return arg[0] >= 32 ? 0 : (arg[1] >> unsigned(8 * (31 - (arg[0] & 0xff)))) & 0xff;
+		return arg[0] >= 32 ? 0 : (arg[1] >> unsigned(8 * (31 - arg[0]))) & 0xff;
 	case Instruction::SHL:
-		return arg[0] > 255 ? 0 : (arg[1] << unsigned(arg[0] & 0xff));
+		return arg[0] > 255 ? 0 : (arg[1] << unsigned(arg[0]));
 	case Instruction::SHR:
-		return arg[0] > 255 ? 0 : (arg[1] >> unsigned(arg[0] & 0xff));
+		return arg[0] > 255 ? 0 : (arg[1] >> unsigned(arg[0]));
 	case Instruction::SAR:
 	{
 		static u256 const hibit = u256(1) << 255;
@@ -149,7 +156,7 @@ u256 EVMInstructionInterpreter::eval(
 			return arg[1] & hibit ? u256(-1) : 0;
 		else
 		{
-			unsigned amount = unsigned(arg[0] & 0xff);
+			unsigned amount = unsigned(arg[0]);
 			u256 v = arg[1] >> amount;
 			if (arg[1] & hibit)
 				v |= u256(-1) << (256 - amount);
@@ -165,7 +172,7 @@ u256 EVMInstructionInterpreter::eval(
 			return arg[0];
 		else
 		{
-			unsigned testBit = unsigned(arg[0] & 0xff) * 8 + 7;
+			unsigned testBit = unsigned(arg[0]) * 8 + 7;
 			u256 ret = arg[1];
 			u256 mask = ((u256(1) << testBit) - 1);
 			if (boost::multiprecision::bit_test(ret, testBit))
diff --git a/test/tools/yulInterpreter/EVMInstructionInterpreter.h b/test/tools/yulInterpreter/EVMInstructionInterpreter.h
index 57c65b9aa..70033976f 100644
--- a/test/tools/yulInterpreter/EVMInstructionInterpreter.h
+++ b/test/tools/yulInterpreter/EVMInstructionInterpreter.h
@@ -45,19 +45,20 @@ struct InterpreterState;
  * Interprets EVM instructions based on the current state and logs instructions with
  * side-effects.
  *
- * Since this is mainly meant to be used for testing, it is focused on a single
- * contract only, does not do any gas counting and differs from the correct
- * implementation in the following ways:
+ * Since this is mainly meant to be used for differential fuzz testing, it is focused
+ * on a single contract only, does not do any gas counting and differs from the correct
+ * implementation in many ways:
  *
  * - If memory access to a "large" memory position is performed, a deterministic
- *    value is returned. Data that is stored in a "large" memory position is not
- *    retained.
+ *   value is returned. Data that is stored in a "large" memory position is not
+ *   retained.
  * - The blockhash instruction returns a fixed value if the argument is in range.
  * - Extcodesize returns a deterministic value depending on the address.
  * - Extcodecopy copies a deterministic value depending on the address.
  * - And many other things
- * - TODO
  *
+ * The main focus is that the generated execution trace is the same for equivalent executions
+ * and likely to be different for non-eqivalent executions.
  */
 class EVMInstructionInterpreter
 {