Merge pull request #12276 from ethereum/fuzz-RSE

Yul interpreter: Changes required before merging Redundant store eliminator

test/libyul/EwasmTranslationTest.cpp

@@ -108,13 +108,18 @@ string EwasmTranslationTest::interpret()
 	state.maxExprNesting = 64;
 	try
 	{
-		Interpreter::run(state, WasmDialect{}, *m_object->code);
+		Interpreter::run(
+			state,
+			WasmDialect{},
+			*m_object->code,
+			/*disableMemoryTracing=*/false
+		);
 	}
 	catch (InterpreterTerminatedGeneric const&)
 	{
 	}
 
 	stringstream result;
-	state.dumpTraceAndState(result);
+	state.dumpTraceAndState(result, false);
 	return result.str();
 }

test/libyul/YulInterpreterTest.cpp

@@ -94,13 +94,18 @@ string YulInterpreterTest::interpret()
 	state.maxExprNesting = 64;
 	try
 	{
-		Interpreter::run(state, EVMDialect::strictAssemblyForEVMObjects(langutil::EVMVersion{}), *m_ast);
+		Interpreter::run(
+			state,
+			EVMDialect::strictAssemblyForEVMObjects(langutil::EVMVersion{}),
+			*m_ast,
+			/*disableMemoryTracing=*/false
+		);
 	}
 	catch (InterpreterTerminatedGeneric const&)
 	{
 	}
 
 	stringstream result;
-	state.dumpTraceAndState(result);
+	state.dumpTraceAndState(result, false);
 	return result.str();
 }

test/tools/ossfuzz/strictasm_diff_ossfuzz.cpp

@@ -81,10 +81,15 @@ extern "C" int LLVMFuzzerTestOneInput(uint8_t const* _data, size_t _size)
 
 	ostringstream os1;
 	ostringstream os2;
+	// Disable memory tracing to avoid false positive reports
+	// such as unused write to memory e.g.,
+	// { mstore(0, 1) }
+	// that would be removed by the redundant store eliminator.
 	yulFuzzerUtil::TerminationReason termReason = yulFuzzerUtil::interpret(
 		os1,
 		stack.parserResult()->code,
-		EVMDialect::strictAssemblyForEVMObjects(langutil::EVMVersion())
+		EVMDialect::strictAssemblyForEVMObjects(langutil::EVMVersion()),
+		/*disableMemoryTracing=*/true
 	);
 	if (yulFuzzerUtil::resourceLimitsExceeded(termReason))
 		return 0;
@@ -93,7 +98,8 @@ extern "C" int LLVMFuzzerTestOneInput(uint8_t const* _data, size_t _size)
 	termReason = yulFuzzerUtil::interpret(
 		os2,
 		stack.parserResult()->code,
-		EVMDialect::strictAssemblyForEVMObjects(langutil::EVMVersion())
+		EVMDialect::strictAssemblyForEVMObjects(langutil::EVMVersion()),
+		/*disableMemoryTracing=*/true
 	);
 
 	if (yulFuzzerUtil::resourceLimitsExceeded(termReason))

test/tools/ossfuzz/yulFuzzerCommon.cpp

@@ -26,6 +26,7 @@ yulFuzzerUtil::TerminationReason yulFuzzerUtil::interpret(
 	ostream& _os,
 	shared_ptr<yul::Block> _ast,
 	Dialect const& _dialect,
+	bool _disableMemoryTracing,
 	bool _outputStorageOnly,
 	size_t _maxSteps,
 	size_t _maxTraceSize,
@@ -52,7 +53,7 @@ yulFuzzerUtil::TerminationReason yulFuzzerUtil::interpret(
 	TerminationReason reason = TerminationReason::None;
 	try
 	{
-		Interpreter::run(state, _dialect, *_ast);
+		Interpreter::run(state, _dialect, *_ast, _disableMemoryTracing);
 	}
 	catch (StepLimitReached const&)
 	{
@@ -74,7 +75,7 @@ yulFuzzerUtil::TerminationReason yulFuzzerUtil::interpret(
 	if (_outputStorageOnly)
 		state.dumpStorage(_os);
 	else
-		state.dumpTraceAndState(_os);
+		state.dumpTraceAndState(_os, _disableMemoryTracing);
 	return reason;
 }
 

test/tools/ossfuzz/yulFuzzerCommon.h

@@ -32,10 +32,17 @@ struct yulFuzzerUtil
 		None
 	};
 
+	/// Interprets the Yul AST pointed to by @param _ast. Flag @param _outputStorageOnly
+	/// (unset by default) outputs an execution trace of both memory and storage;
+	/// if set, only storage contents are output as part of the execution trace. The
+	/// latter avoids false positives that will be produced by the fuzzer when certain
+	/// optimizer steps are activated e.g., Redundant store eliminator, Equal store
+	/// eliminator.
 	static TerminationReason interpret(
 		std::ostream& _os,
 		std::shared_ptr<yul::Block> _ast,
 		Dialect const& _dialect,
+		bool _disableMemoryTracing = false,
 		bool _outputStorageOnly = false,
 		size_t _maxSteps = maxSteps,
 		size_t _maxTraceSize = maxTraceSize,

test/tools/ossfuzz/yulProto_diff_ossfuzz.cpp

@@ -88,10 +88,15 @@ DEFINE_PROTO_FUZZER(Program const& _input)
 
 	ostringstream os1;
 	ostringstream os2;
+	// Disable memory tracing to avoid false positive reports
+	// such as unused write to memory e.g.,
+	// { mstore(0, 1) }
+	// that would be removed by the redundant store eliminator.
 	yulFuzzerUtil::TerminationReason termReason = yulFuzzerUtil::interpret(
 		os1,
 		stack.parserResult()->code,
-		EVMDialect::strictAssemblyForEVMObjects(version)
+		EVMDialect::strictAssemblyForEVMObjects(version),
+		/*disableMemoryTracing=*/true
 	);
 
 	if (yulFuzzerUtil::resourceLimitsExceeded(termReason))
@@ -107,12 +112,18 @@ DEFINE_PROTO_FUZZER(Program const& _input)
 	termReason = yulFuzzerUtil::interpret(
 		os2,
 		astBlock,
-		EVMDialect::strictAssemblyForEVMObjects(version)
+		EVMDialect::strictAssemblyForEVMObjects(version),
+		true
 	);
 	if (yulFuzzerUtil::resourceLimitsExceeded(termReason))
 		return;
 
 	bool isTraceEq = (os1.str() == os2.str());
-	yulAssert(isTraceEq, "Interpreted traces for optimized and unoptimized code differ.");
+	if (!isTraceEq)
+	{
+		cout << os1.str() << endl;
+		cout << os2.str() << endl;
+		yulAssert(false, "Interpreted traces for optimized and unoptimized code differ.");
+	}
 	return;
 }

test/tools/yulInterpreter/EVMInstructionInterpreter.cpp

@@ -27,6 +27,7 @@
 #include <libyul/AST.h>
 
 #include <libevmasm/Instruction.h>
+#include <libevmasm/SemanticInformation.h>
 
 #include <libsolutil/Keccak256.h>
 #include <libsolutil/Numeric.h>
@@ -35,6 +36,7 @@
 
 using namespace std;
 using namespace solidity;
+using namespace solidity::evmasm;
 using namespace solidity::yul;
 using namespace solidity::yul::test;
 
@@ -99,6 +101,7 @@ u256 EVMInstructionInterpreter::eval(
 	switch (_instruction)
 	{
 	case Instruction::STOP:
+		logTrace(_instruction);
 		BOOST_THROW_EXCEPTION(ExplicitlyTerminated());
 	// --------------- arithmetic ---------------
 	case Instruction::ADD:
@@ -204,6 +207,7 @@ u256 EVMInstructionInterpreter::eval(
 	case Instruction::CALLDATASIZE:
 		return m_state.calldata.size();
 	case Instruction::CALLDATACOPY:
+		logTrace(_instruction, arg);
 		if (accessMemory(arg[0], arg[2]))
 			copyZeroExtended(
 				m_state.memory, m_state.calldata,
@@ -213,6 +217,7 @@ u256 EVMInstructionInterpreter::eval(
 	case Instruction::CODESIZE:
 		return m_state.code.size();
 	case Instruction::CODECOPY:
+		logTrace(_instruction, arg);
 		if (accessMemory(arg[0], arg[2]))
 			copyZeroExtended(
 				m_state.memory, m_state.code,
@@ -339,12 +344,18 @@ u256 EVMInstructionInterpreter::eval(
 	case Instruction::REVERT:
 		accessMemory(arg[0], arg[1]);
 		logTrace(_instruction, arg);
+		m_state.storage.clear();
+		m_state.trace.clear();
 		BOOST_THROW_EXCEPTION(ExplicitlyTerminated());
 	case Instruction::INVALID:
 		logTrace(_instruction);
+		m_state.storage.clear();
+		m_state.trace.clear();
 		BOOST_THROW_EXCEPTION(ExplicitlyTerminated());
 	case Instruction::SELFDESTRUCT:
 		logTrace(_instruction, arg);
+		m_state.storage.clear();
+		m_state.trace.clear();
 		BOOST_THROW_EXCEPTION(ExplicitlyTerminated());
 	case Instruction::POP:
 		break;
@@ -507,12 +518,28 @@ void EVMInstructionInterpreter::writeMemoryWord(u256 const& _offset, u256 const&
 }
 
 
-void EVMInstructionInterpreter::logTrace(evmasm::Instruction _instruction, std::vector<u256> const& _arguments, bytes const& _data)
+void EVMInstructionInterpreter::logTrace(
+	evmasm::Instruction _instruction,
+	std::vector<u256> const& _arguments,
+	bytes const& _data
+)
 {
-	logTrace(evmasm::instructionInfo(_instruction).name, _arguments, _data);
+	logTrace(
+		evmasm::instructionInfo(_instruction).name,
+		SemanticInformation::memory(_instruction) == SemanticInformation::Effect::Write,
+		_arguments,
+		_data
+	);
 }
 
-void EVMInstructionInterpreter::logTrace(std::string const& _pseudoInstruction, std::vector<u256> const& _arguments, bytes const& _data)
+void EVMInstructionInterpreter::logTrace(
+	std::string const& _pseudoInstruction,
+	bool _writesToMemory,
+	std::vector<u256> const& _arguments,
+	bytes const& _data
+)
+{
+	if (!(_writesToMemory && memWriteTracingDisabled()))
 	{
 		string message = _pseudoInstruction + "(";
 		for (size_t i = 0; i < _arguments.size(); ++i)
@@ -527,3 +554,4 @@ void EVMInstructionInterpreter::logTrace(std::string const& _pseudoInstruction,
 			BOOST_THROW_EXCEPTION(TraceLimitReached());
 		}
 	}
+}

test/tools/yulInterpreter/EVMInstructionInterpreter.h

@@ -66,8 +66,9 @@ struct InterpreterState;
 class EVMInstructionInterpreter
 {
 public:
-	explicit EVMInstructionInterpreter(InterpreterState& _state):
-		m_state(_state)
+	explicit EVMInstructionInterpreter(InterpreterState& _state, bool _disableMemWriteTrace):
+		m_state(_state),
+		m_disableMemoryWriteInstructions(_disableMemWriteTrace)
 	{}
 	/// Evaluate instruction
 	u256 eval(evmasm::Instruction _instruction, std::vector<u256> const& _arguments);
@@ -93,12 +94,29 @@ private:
 	/// Does not adjust msize, use @a accessMemory for that
 	void writeMemoryWord(u256 const& _offset, u256 const& _value);
 
-	void logTrace(evmasm::Instruction _instruction, std::vector<u256> const& _arguments = {}, bytes const& _data = {});
+	void logTrace(
+		evmasm::Instruction _instruction,
+		std::vector<u256> const& _arguments = {},
+		bytes const& _data = {}
+	);
 	/// Appends a log to the trace representing an instruction or similar operation by string,
-	/// with arguments and auxiliary data (if nonempty).
-	void logTrace(std::string const& _pseudoInstruction, std::vector<u256> const& _arguments = {}, bytes const& _data = {});
+	/// with arguments and auxiliary data (if nonempty). Flag @param _writesToMemory indicates
+	/// whether the instruction writes to (true) or does not write to (false) memory.
+	void logTrace(
+		std::string const& _pseudoInstruction,
+		bool _writesToMemory,
+		std::vector<u256> const& _arguments = {},
+		bytes const& _data = {}
+	);
+	/// @returns disable trace flag.
+	bool memWriteTracingDisabled()
+	{
+		return m_disableMemoryWriteInstructions;
+	}
 
 	InterpreterState& m_state;
+	/// Flag to disable trace of instructions that write to memory.
+	bool m_disableMemoryWriteInstructions;
 };
 
 } // solidity::yul::test

test/tools/yulInterpreter/Interpreter.cpp

@@ -55,11 +55,13 @@ void InterpreterState::dumpStorage(ostream& _out) const
 			_out << "  " << slot.first.hex() << ": " << slot.second.hex() << endl;
 }
 
-void InterpreterState::dumpTraceAndState(ostream& _out) const
+void InterpreterState::dumpTraceAndState(ostream& _out, bool _disableMemoryTrace) const
 {
 	_out << "Trace:" << endl;
 	for (auto const& line: trace)
 		_out << "  " << line << endl;
+	if (!_disableMemoryTrace)
+	{
 		_out << "Memory dump:\n";
 		map<u256, u256> words;
 		for (auto const& [offset, value]: memory)
@@ -67,14 +69,20 @@ void InterpreterState::dumpTraceAndState(ostream& _out) const
 		for (auto const& [offset, value]: words)
 			if (value != 0)
 				_out << "  " << std::uppercase << std::hex << std::setw(4) << offset << ": " << h256(value).hex() << endl;
+	}
 	_out << "Storage dump:" << endl;
 	dumpStorage(_out);
 }
 
-void Interpreter::run(InterpreterState& _state, Dialect const& _dialect, Block const& _ast)
+void Interpreter::run(
+	InterpreterState& _state,
+	Dialect const& _dialect,
+	Block const& _ast,
+	bool _disableMemoryTrace
+)
 {
 	Scope scope;
-	Interpreter{_state, _dialect, scope}(_ast);
+	Interpreter{_state, _dialect, scope, _disableMemoryTrace}(_ast);
 }
 
 void Interpreter::operator()(ExpressionStatement const& _expressionStatement)
@@ -209,14 +217,14 @@ void Interpreter::operator()(Block const& _block)
 
 u256 Interpreter::evaluate(Expression const& _expression)
 {
-	ExpressionEvaluator ev(m_state, m_dialect, *m_scope, m_variables);
+	ExpressionEvaluator ev(m_state, m_dialect, *m_scope, m_variables, m_disableMemoryTrace);
 	ev.visit(_expression);
 	return ev.value();
 }
 
 vector<u256> Interpreter::evaluateMulti(Expression const& _expression)
 {
-	ExpressionEvaluator ev(m_state, m_dialect, *m_scope, m_variables);
+	ExpressionEvaluator ev(m_state, m_dialect, *m_scope, m_variables, m_disableMemoryTrace);
 	ev.visit(_expression);
 	return ev.values();
 }
@@ -279,7 +287,7 @@ void ExpressionEvaluator::operator()(FunctionCall const& _funCall)
 	{
 		if (BuiltinFunctionForEVM const* fun = dialect->builtin(_funCall.functionName.name))
 		{
-			EVMInstructionInterpreter interpreter(m_state);
+			EVMInstructionInterpreter interpreter(m_state, m_disableMemoryTrace);
 			setValue(interpreter.evalBuiltin(*fun, _funCall.arguments, values()));
 			return;
 		}
@@ -308,7 +316,7 @@ void ExpressionEvaluator::operator()(FunctionCall const& _funCall)
 		variables[fun->returnVariables.at(i).name] = 0;
 
 	m_state.controlFlowState = ControlFlowState::Default;
-	Interpreter interpreter(m_state, m_dialect, *scope, std::move(variables));
+	Interpreter interpreter(m_state, m_dialect, *scope, m_disableMemoryTrace, std::move(variables));
 	interpreter(fun->body);
 	m_state.controlFlowState = ControlFlowState::Default;
 

test/tools/yulInterpreter/Interpreter.h

@@ -102,7 +102,10 @@ struct InterpreterState
 	ControlFlowState controlFlowState = ControlFlowState::Default;
 
 	/// Prints execution trace and non-zero storage to @param _out.
-	void dumpTraceAndState(std::ostream& _out) const;
+	/// Flag @param _disableMemoryTrace, if set, does not produce a memory dump. This
+	/// avoids false positives reports by the fuzzer when certain optimizer steps are
+	/// activated e.g., Redundant store eliminator, Equal store eliminator.
+	void dumpTraceAndState(std::ostream& _out, bool _disableMemoryTrace) const;
 	/// Prints non-zero storage to @param _out.
 	void dumpStorage(std::ostream& _out) const;
 };
@@ -124,18 +127,29 @@ struct Scope
 class Interpreter: public ASTWalker
 {
 public:
-	static void run(InterpreterState& _state, Dialect const& _dialect, Block const& _ast);
+	/// Executes the Yul interpreter. Flag @param _disableMemoryTracing if set ensures that
+	/// instructions that write to memory do not affect @param _state. This
+	/// avoids false positives reports by the fuzzer when certain optimizer steps are
+	/// activated e.g., Redundant store eliminator, Equal store eliminator.
+	static void run(
+		InterpreterState& _state,
+		Dialect const& _dialect,
+		Block const& _ast,
+		bool _disableMemoryTracing
+	);
 
 	Interpreter(
 		InterpreterState& _state,
 		Dialect const& _dialect,
 		Scope& _scope,
+		bool _disableMemoryTracing,
 		std::map<YulString, u256> _variables = {}
 	):
 		m_dialect(_dialect),
 		m_state(_state),
 		m_variables(std::move(_variables)),
-		m_scope(&_scope)
+		m_scope(&_scope),
+		m_disableMemoryTrace(_disableMemoryTracing)
 	{
 	}
 
@@ -173,6 +187,7 @@ private:
 	/// Values of variables.
 	std::map<YulString, u256> m_variables;
 	Scope* m_scope;
+	bool m_disableMemoryTrace;
 };
 
 /**
@@ -185,12 +200,14 @@ public:
 		InterpreterState& _state,
 		Dialect const& _dialect,
 		Scope& _scope,
-		std::map<YulString, u256> const& _variables
+		std::map<YulString, u256> const& _variables,
+		bool _disableMemoryTrace
 	):
 		m_state(_state),
 		m_dialect(_dialect),
 		m_variables(_variables),
-		m_scope(_scope)
+		m_scope(_scope),
+		m_disableMemoryTrace(_disableMemoryTrace)
 	{}
 
 	void operator()(Literal const&) override;
@@ -226,6 +243,8 @@ private:
 	std::vector<u256> m_values;
 	/// Current expression nesting level
 	unsigned m_nestingLevel = 0;
+	/// Flag to disable memory tracing
+	bool m_disableMemoryTrace;
 };
 
 }

test/tools/yulrun.cpp

@@ -87,13 +87,13 @@ void interpret(string const& _source)
 	try
 	{
 		Dialect const& dialect(EVMDialect::strictAssemblyForEVMObjects(langutil::EVMVersion{}));
-		Interpreter::run(state, dialect, *ast);
+		Interpreter::run(state, dialect, *ast, /*disableMemoryTracing=*/false);
 	}
 	catch (InterpreterTerminatedGeneric const&)
 	{
 	}
 
-	state.dumpTraceAndState(cout);
+	state.dumpTraceAndState(cout, /*disableMemoryTracing=*/false);
 }
 
 }