Merge pull request #9433 from ethereum/reportAllStackErrors

Report all stack errors

libevmasm/Assembly.cpp

@@ -522,6 +522,7 @@ map<u256, u256> Assembly::optimiseInternal(
 
 LinkerObject const& Assembly::assemble() const
 {
+	assertThrow(!m_invalid, AssemblyException, "Attempted to assemble invalid Assembly object.");
 	// Return the already assembled object, if present.
 	if (!m_assembledObject.bytecode.empty())
 		return m_assembledObject;

libevmasm/Assembly.h

@@ -143,6 +143,9 @@ public:
 		std::map<std::string, unsigned> const& _sourceIndices = std::map<std::string, unsigned>()
 	) const;
 
+	/// Mark this assembly as invalid. Calling ``assemble`` on it will throw.
+	void markAsInvalid() { m_invalid = true; }
+
 protected:
 	/// Does the same operations as @a optimise, but should only be applied to a sub and
 	/// returns the replaced tags. Also takes an argument containing the tags of this assembly
@@ -162,6 +165,7 @@ private:
 	);
 	static std::string toStringInHex(u256 _value);
 
+	bool m_invalid = false;
 protected:
 	/// 0 is reserved for exception
 	unsigned m_usedTags = 1;

libyul/CompilabilityChecker.cpp

@@ -59,14 +59,7 @@ map<YulString, int> CompilabilityChecker::run(
 			builtinContext,
 			_optimizeStackAllocation
 		);
-		try
+		transform(*_object.code);
-		{
-			transform(*_object.code);
-		}
-		catch (StackTooDeepError const&)
-		{
-			yulAssert(!transform.stackErrors().empty(), "Got stack too deep exception that was not stored.");
-		}
 
 		std::map<YulString, int> functions;
 		for (StackTooDeepError const& error: transform.stackErrors())

libyul/backends/evm/AbstractAssembly.h

@@ -112,6 +112,9 @@ public:
 	virtual void appendImmutable(std::string const& _identifier) = 0;
 	/// Appends an assignment to an immutable variable.
 	virtual void appendImmutableAssignment(std::string const& _identifier) = 0;
+
+	/// Mark this assembly as invalid. Any attempt to request bytecode from it should throw.
+	virtual void markAsInvalid() = 0;
 };
 
 enum class IdentifierContext { LValue, RValue, VariableDeclaration };

libyul/backends/evm/AsmCodeGen.cpp

@@ -183,6 +183,11 @@ void EthAssemblyAdapter::appendImmutableAssignment(std::string const& _identifie
 	m_assembly.appendImmutableAssignment(_identifier);
 }
 
+void EthAssemblyAdapter::markAsInvalid()
+{
+	m_assembly.markAsInvalid();
+}
+
 EthAssemblyAdapter::LabelID EthAssemblyAdapter::assemblyTagToIdentifier(evmasm::AssemblyItem const& _tag)
 {
 	u256 id = _tag.data();
@@ -232,18 +237,12 @@ void CodeGenerator::assemble(
 		_identifierAccess,
 		_useNamedLabelsForFunctions
 	);
-	try
+	transform(_parsedData);
-	{
+	if (!transform.stackErrors().empty())
-		transform(_parsedData);
-	}
-	catch (StackTooDeepError const& _e)
-	{
 		assertThrow(
 			false,
 			langutil::StackTooDeepError,
 			"Stack too deep when compiling inline assembly" +
-			(_e.comment() ? ": " + *_e.comment() : ".")
+			(transform.stackErrors().front().comment() ? ": " + *transform.stackErrors().front().comment() : ".")
 		);
-	}
-	yulAssert(transform.stackErrors().empty(), "Stack errors present but not thrown.");
 }

libyul/backends/evm/AsmCodeGen.h

@@ -65,6 +65,8 @@ public:
 	void appendImmutable(std::string const& _identifier) override;
 	void appendImmutableAssignment(std::string const& _identifier) override;
 
+	void markAsInvalid() override;
+
 private:
 	static LabelID assemblyTagToIdentifier(evmasm::AssemblyItem const& _tag);
 	void appendJumpInstruction(evmasm::Instruction _instruction, JumpType _jumpType);

libyul/backends/evm/EVMAssembly.cpp

@@ -157,6 +157,7 @@ void EVMAssembly::appendReturnsub(int _returns, int _stackDiffAfter)
 
 evmasm::LinkerObject EVMAssembly::finalize()
 {
+	yulAssert(!m_invalid, "Attempted to finalize invalid assembly object.");
 	size_t bytecodeSize = m_bytecode.size();
 	for (auto const& ref: m_assemblySizePositions)
 		updateReference(ref, assemblySizeReferenceSize, u256(bytecodeSize));

libyul/backends/evm/EVMAssembly.h

@@ -87,6 +87,8 @@ public:
 	void appendImmutable(std::string const& _identifier) override;
 	void appendImmutableAssignment(std::string const& _identifier) override;
 
+	void markAsInvalid() override { m_invalid = true; }
+
 	/// Resolves references inside the bytecode and returns the linker object.
 	evmasm::LinkerObject finalize();
 
@@ -103,6 +105,7 @@ private:
 	std::map<LabelID, size_t> m_labelPositions;
 	std::map<size_t, LabelID> m_labelReferences;
 	std::vector<size_t> m_assemblySizePositions;
+	bool m_invalid = false;
 };
 
 }

libyul/backends/evm/EVMCodeTransform.cpp

@@ -238,6 +238,7 @@ void CodeTransform::stackError(StackTooDeepError _error, int _targetStackHeight)
 		m_assembly.appendConstant(u256(0));
 	// Store error.
 	m_stackErrors.emplace_back(std::move(_error));
+	m_assembly.markAsInvalid();
 }
 
 void CodeTransform::operator()(Assignment const& _assignment)
@@ -436,31 +437,28 @@ void CodeTransform::operator()(FunctionDefinition const& _function)
 	m_context->functionExitPoints.push(
 		CodeTransformContext::JumpInfo{m_assembly.newLabelId(), m_assembly.stackHeight()}
 	);
-	try
+	CodeTransform subTransform(
+		m_assembly,
+		m_info,
+		_function.body,
+		m_allowStackOpt,
+		m_dialect,
+		m_builtinContext,
+		m_evm15,
+		m_identifierAccess,
+		m_useNamedLabelsForFunctions,
+		m_context
+	);
+	subTransform(_function.body);
+	if (!subTransform.m_stackErrors.empty())
 	{
-		CodeTransform(
+		m_assembly.markAsInvalid();
-			m_assembly,
+		for (StackTooDeepError& stackError: subTransform.m_stackErrors)
-			m_info,
+		{
-			_function.body,
+			if (stackError.functionName.empty())
-			m_allowStackOpt,
+				stackError.functionName = _function.name;
-			m_dialect,
+			m_stackErrors.emplace_back(std::move(stackError));
-			m_builtinContext,
+		}
-			m_evm15,
-			m_identifierAccess,
-			m_useNamedLabelsForFunctions,
-			m_context
-		)(_function.body);
-	}
-	catch (StackTooDeepError const& _error)
-	{
-		// This exception will be re-thrown after the end of the surrounding block.
-		// It enables us to see which functions compiled successfully and which did not.
-		// Even if we emit actual code, add an illegal instruction to make sure that tests
-		// will catch it.
-		StackTooDeepError error(_error);
-		if (error.functionName.empty())
-			error.functionName = _function.name;
-		stackError(std::move(error), static_cast<int>(height));
 	}
 
 	m_assembly.appendLabel(m_context->functionExitPoints.top().label);
@@ -606,9 +604,6 @@ void CodeTransform::operator()(Block const& _block)
 
 	finalizeBlock(_block, blockStartStackHeight);
 	m_scope = originalScope;
-
-	if (!m_stackErrors.empty())
-		BOOST_THROW_EXCEPTION(m_stackErrors.front());
 }
 
 AbstractAssembly::LabelID CodeTransform::functionEntryID(YulString _name, Scope::Function const& _function)
@@ -729,7 +724,8 @@ size_t CodeTransform::variableHeightDiff(Scope::Variable const& _var, YulString
 			to_string(heightDiff - limit) +
 			" slot(s) too deep inside the stack."
 		);
-		BOOST_THROW_EXCEPTION(m_stackErrors.back());
+		m_assembly.markAsInvalid();
+		return _forSwap ? 2 : 1;
 	}
 	return heightDiff;
 }

libyul/backends/evm/EVMCodeTransform.h

@@ -202,6 +202,8 @@ private:
 	/// Determines the stack height difference to the given variables. Throws
 	/// if it is not yet in scope or the height difference is too large. Returns
 	/// the (positive) stack height difference otherwise.
+	/// @param _forSwap if true, produces stack error if the difference is invalid for a swap
+	///                 opcode, otherwise checks for validity for a dup opcode.
 	size_t variableHeightDiff(Scope::Variable const& _var, YulString _name, bool _forSwap);
 
 	void expectDeposit(int _deposit, int _oldHeight) const;

libyul/backends/evm/EVMObjectCompiler.cpp

@@ -60,5 +60,6 @@ void EVMObjectCompiler::run(Object& _object, bool _optimize)
 	// which should be native to this part of the code.
 	CodeTransform transform{m_assembly, *_object.analysisInfo, *_object.code, m_dialect, context, _optimize, m_evm15};
 	transform(*_object.code);
-	yulAssert(transform.stackErrors().empty(), "Stack errors present but not thrown.");
+	if (!transform.stackErrors().empty())
+		BOOST_THROW_EXCEPTION(transform.stackErrors().front());
 }

libyul/backends/evm/NoOutputAssembly.h

@@ -75,6 +75,8 @@ public:
 	void appendImmutable(std::string const& _identifier) override;
 	void appendImmutableAssignment(std::string const& _identifier) override;
 
+	void markAsInvalid() override {}
+
 private:
 	bool m_evm15 = false; ///< if true, switch to evm1.5 mode
 	int m_stackHeight = 0;

test/libyul/CompilabilityChecker.cpp

@@ -187,7 +187,7 @@ BOOST_AUTO_TEST_CASE(nested)
 			x := add(add(add(add(add(add(add(add(add(add(add(add(x, r12), r11), r10), r9), r8), r7), r6), r5), r4), r3), r2), r1)
 		}
 	})");
-	BOOST_CHECK_EQUAL(out, "h: 9 ");
+	BOOST_CHECK_EQUAL(out, "h: 9 g: 5 f: 5 ");
 }
 
 BOOST_AUTO_TEST_CASE(also_in_outer_block)
@@ -216,7 +216,7 @@ BOOST_AUTO_TEST_CASE(also_in_outer_block)
 			function g(s1, s2, s3, s4, s5, s6, s7, s8, s9, s10, s11, s12, s13, s14, s15, s16, s17, s18, s19) -> w, v {
 			}
 	})");
-	BOOST_CHECK_EQUAL(out, ": 9 ");
+	BOOST_CHECK_EQUAL(out, "g: 5 : 9 ");
 }
 
 BOOST_AUTO_TEST_SUITE_END()