Merge pull request #1417 from ethereum/fixOptimizer

Optimizer: Clear state for JUMPDESTs.

Author: pirapira

Changelog.md

@@ -1,3 +1,8 @@
+### 0.4.6 (2016-11-22)
+
+Bugfixes:
+ * Optimizer: Knowledge about state was not correctly cleared for JUMPDESTs
+
 ### 0.4.5 (2016-11-21)
 
 Features:

libevmasm/Assembly.cpp

@@ -360,46 +360,35 @@ map<u256, u256> Assembly::optimiseInternal(bool _enable, bool _isCreation, size_
 			auto iter = m_items.begin();
 			while (iter != m_items.end())
 			{
-				auto end = iter;
-				while (end != m_items.end())
-					if (SemanticInformation::altersControlFlow(*end++))
-						break;
-
 				KnownState emptyState;
 				CommonSubexpressionEliminator eliminator(emptyState);
-				auto blockIter = iter;
-				auto const blockEnd = end;
-				while (blockIter < blockEnd)
+				auto orig = iter;
+				iter = eliminator.feedItems(iter, m_items.end());
+				bool shouldReplace = false;
+				AssemblyItems optimisedChunk;
+				try
+				{
+					optimisedChunk = eliminator.getOptimizedItems();
+					shouldReplace = (optimisedChunk.size() < size_t(iter - orig));
+				}
+				catch (StackTooDeepException const&)
+				{
+					// This might happen if the opcode reconstruction is not as efficient
+					// as the hand-crafted code.
+				}
+				catch (ItemNotAvailableException const&)
 				{
-					auto orig = blockIter;
-					blockIter = eliminator.feedItems(blockIter, blockEnd);
-					bool shouldReplace = false;
-					AssemblyItems optimisedChunk;
-					try
-					{
-						optimisedChunk = eliminator.getOptimizedItems();
-						shouldReplace = (optimisedChunk.size() < size_t(blockIter - orig));
-					}
-					catch (StackTooDeepException const&)
-					{
-						// This might happen if the opcode reconstruction is not as efficient
-						// as the hand-crafted code.
-					}
-					catch (ItemNotAvailableException const&)
-					{
-						// This might happen if e.g. associativity and commutativity rules
-						// reorganise the expression tree, but not all leaves are available.
-					}
-
-					if (shouldReplace)
-					{
-						count++;
-						optimisedItems += optimisedChunk;
-					}
-					else
-						copy(orig, blockIter, back_inserter(optimisedItems));
+					// This might happen if e.g. associativity and commutativity rules
+					// reorganise the expression tree, but not all leaves are available.
 				}
-				iter = end;
+
+				if (shouldReplace)
+				{
+					count++;
+					optimisedItems += optimisedChunk;
+				}
+				else
+					copy(orig, iter, back_inserter(optimisedItems));
 			}
 			if (optimisedItems.size() < m_items.size())
 			{

test/libsolidity/SolidityOptimizer.cpp

@@ -1246,6 +1246,26 @@ BOOST_AUTO_TEST_CASE(dead_code_elimination_across_assemblies)
 	compareVersions("test()");
 }
 
+BOOST_AUTO_TEST_CASE(invalid_state_at_control_flow_join)
+{
+	char const* sourceCode = R"(
+		contract Test {
+			uint256 public totalSupply = 100;
+			function f() returns (uint r) {
+				if (false)
+					r = totalSupply;
+				totalSupply -= 10;
+			}
+			function test() returns (uint) {
+				f();
+				return this.totalSupply();
+			}
+		}
+	)";
+	compileBothVersions(sourceCode);
+	compareVersions("test()");
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }