From f7b5a27581b2fe91ee36e52a7f373196e9cad068 Mon Sep 17 00:00:00 2001
From: Bhargava Shastry <bhargava.shastry@ethereum.org>
Date: Tue, 19 Mar 2019 10:44:39 +0100
Subject: [PATCH] Fixes bug in byte optimization rule and adds tests.

---
 Changelog.md                              |  3 +++
 docs/bugs.json                            | 11 +++++++++++
 docs/bugs_by_version.json                 |  5 ++++-
 libevmasm/RuleList.h                      |  2 +-
 test/libevmasm/Optimiser.cpp              | 20 ++++++++++++++++++++
 test/libsolidity/SolidityEndToEndTest.cpp | 21 +++++++++++++++++++++
 6 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/Changelog.md b/Changelog.md
index 3292041fe..e84204f01 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -1,5 +1,8 @@
 ### 0.5.7 (unreleased)
 
+Important Bugfixes:
+ * Optimizer: Fix wrong ordering of arguments in byte optimization rule for constants.
+
 Language Features:
 
 
diff --git a/docs/bugs.json b/docs/bugs.json
index 327e54a18..0d8a64849 100644
--- a/docs/bugs.json
+++ b/docs/bugs.json
@@ -1,4 +1,15 @@
 [
+    {
+        "name": "IncorrectByteInstructionOptimization",
+	"summary": "The optimizer incorrectly handles byte opcodes whose second argument is 31 or a constant expression that evaluates to 31. This can result in unexpected values.",
+        "description": "The optimizer incorrectly handles byte opcodes that use the constant 31 as second argument. This can happen when performing index access on bytesNN types with a compile-time constant value (not index) of 31 or when using the byte opcode in inline assembly.",
+        "introduced": "0.5.5",
+        "fixed": "0.5.7",
+        "severity": "very low",
+        "conditions": {
+            "optimizer": true
+        }
+    },
     {
         "name": "DoubleShiftSizeOverflow",
         "summary": "Double bitwise shifts by large constants whose sum overflows 256 bits can result in unexpected values.",
diff --git a/docs/bugs_by_version.json b/docs/bugs_by_version.json
index dd779e0ed..b0fe056d6 100644
--- a/docs/bugs_by_version.json
+++ b/docs/bugs_by_version.json
@@ -631,12 +631,15 @@
     },
     "0.5.5": {
         "bugs": [
+            "IncorrectByteInstructionOptimization",
             "DoubleShiftSizeOverflow"
         ],
         "released": "2019-03-05"
     },
     "0.5.6": {
-        "bugs": [],
+        "bugs": [
+            "IncorrectByteInstructionOptimization"
+        ],
         "released": "2019-03-13"
     }
 }
\ No newline at end of file
diff --git a/libevmasm/RuleList.h b/libevmasm/RuleList.h
index 5f5bb6352..c091fc21a 100644
--- a/libevmasm/RuleList.h
+++ b/libevmasm/RuleList.h
@@ -153,7 +153,7 @@ std::vector<SimplificationRule<Pattern>> simplificationRuleListPart2(
 		{{Instruction::GT, {0, X}}, [=]{ return u256(0); }, true},
 		{{Instruction::LT, {X, 0}}, [=]{ return u256(0); }, true},
 		{{Instruction::AND, {{Instruction::BYTE, {X, Y}}, {u256(0xff)}}}, [=]() -> Pattern { return {Instruction::BYTE, {X, Y}}; }, false},
-		{{Instruction::BYTE, {X, 31}}, [=]() -> Pattern { return {Instruction::AND, {X, u256(0xff)}}; }, false}
+		{{Instruction::BYTE, {31, X}}, [=]() -> Pattern { return {Instruction::AND, {X, u256(0xff)}}; }, false}
 	};
 }
 
diff --git a/test/libevmasm/Optimiser.cpp b/test/libevmasm/Optimiser.cpp
index 6d76c201a..97632e348 100644
--- a/test/libevmasm/Optimiser.cpp
+++ b/test/libevmasm/Optimiser.cpp
@@ -268,6 +268,26 @@ BOOST_AUTO_TEST_CASE(cse_double_shift_left_overflow)
 	}
 }
 
+BOOST_AUTO_TEST_CASE(cse_byte_ordering_bug)
+{
+	AssemblyItems input{
+		u256(31),
+		Instruction::CALLVALUE,
+		Instruction::BYTE
+	};
+	checkCSE(input, {u256(31), Instruction::CALLVALUE, Instruction::BYTE});
+}
+
+BOOST_AUTO_TEST_CASE(cse_byte_ordering_fix)
+{
+	AssemblyItems input{
+		Instruction::CALLVALUE,
+		u256(31),
+		Instruction::BYTE
+	};
+	checkCSE(input, {u256(0xff), Instruction::CALLVALUE, Instruction::AND});
+}
+
 BOOST_AUTO_TEST_CASE(cse_storage)
 {
 	AssemblyItems input{
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index 0dc1ad22c..127e60e19 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -10495,6 +10495,27 @@ BOOST_AUTO_TEST_CASE(fixed_bytes_length_access)
 	ABI_CHECK(callContractFunction("f(bytes32)", "789"), encodeArgs(u256(32), u256(16), u256(8)));
 }
 
+BOOST_AUTO_TEST_CASE(byte_optimization_bug)
+{
+	char const* sourceCode = R"(
+		contract C {
+			function f(uint x) public returns (uint a) {
+				assembly {
+					a := byte(x, 31)
+				}
+			}
+			function g(uint x) public returns (uint a) {
+				assembly {
+					a := byte(31, x)
+				}
+			}
+		}
+	)";
+	compileAndRun(sourceCode, 0, "C");
+	ABI_CHECK(callContractFunction("f(uint256)", u256(2)), encodeArgs(u256(0)));
+	ABI_CHECK(callContractFunction("g(uint256)", u256(2)), encodeArgs(u256(2)));
+}
+
 BOOST_AUTO_TEST_CASE(inline_assembly_write_to_stack)
 {
 	char const* sourceCode = R"(