Merge pull request #10997 from ethereum/yul-fuzzer-improvements

Yul fuzzer improvements

test/tools/ossfuzz/protoToYul.cpp

@@ -173,20 +173,6 @@ bool ProtoConverter::varDeclAvailable()
 	}
 }
 
-bool ProtoConverter::functionCallNotPossible(FunctionCall_Returns _type)
-{
-	return _type == FunctionCall::SINGLE ||
-		(_type == FunctionCall::MULTIASSIGN && !varDeclAvailable());
-}
-
-unsigned ProtoConverter::numVarsInScope()
-{
-	if (m_inFunctionDef)
-		return static_cast<unsigned>(m_currentFuncVars.size());
-	else
-		return static_cast<unsigned>(m_currentGlobalVars.size());
-}
-
 void ProtoConverter::visit(VarRef const& _x)
 {
 	if (m_inFunctionDef)
@@ -238,10 +224,11 @@ void ProtoConverter::visit(Expression const& _x)
 		visit(_x.nop());
 		break;
 	case Expression::kFuncExpr:
-		// FunctionCall must return a single value, otherwise
+		if (auto v = functionExists(NumFunctionReturns::Single); v.has_value())
-		// we output a trivial expression "1".
+		{
-		if (_x.func_expr().ret() == FunctionCall::SINGLE)
+			string functionName = v.value();
-			visit(_x.func_expr());
+			visit(_x.func_expr(), functionName, true);
+		}
 		else
 			m_output << dictionaryToken();
 		break;
@@ -905,20 +892,6 @@ void ProtoConverter::visitFunctionInputParams(FunctionCall const& _x, unsigned _
 	}
 }
 
-bool ProtoConverter::functionValid(FunctionCall_Returns _type, unsigned _numOutParams)
-{
-	switch (_type)
-	{
-	case FunctionCall::ZERO:
-		return _numOutParams == 0;
-	case FunctionCall::SINGLE:
-		return _numOutParams == 1;
-	case FunctionCall::MULTIDECL:
-	case FunctionCall::MULTIASSIGN:
-		return _numOutParams > 1;
-	}
-}
-
 void ProtoConverter::convertFunctionCall(
 	FunctionCall const& _x,
 	string const& _name,
@@ -944,130 +917,52 @@ vector<string> ProtoConverter::createVarDecls(unsigned _start, unsigned _end, bo
 	return varsVec;
 }
 
-void ProtoConverter::visit(FunctionCall const& _x)
+optional<string> ProtoConverter::functionExists(NumFunctionReturns _numReturns)
 {
-	bool functionAvailable = m_functionSigMap.size() > 0;
+	for (auto const& item: m_functionSigMap)
-	unsigned numInParams, numOutParams;
+		if (_numReturns == NumFunctionReturns::None || _numReturns == NumFunctionReturns::Single)
-	string funcName;
-	FunctionCall_Returns funcType = _x.ret();
-	if (functionAvailable)
 		{
-		yulAssert(m_functions.size() > 0, "Proto fuzzer: No function in scope");
+			if (item.second.second == static_cast<unsigned>(_numReturns))
-		funcName = m_functions[_x.func_index() % m_functions.size()];
+				return item.first;
-		auto ret = m_functionSigMap.at(funcName);
-		numInParams = ret.first;
-		numOutParams = ret.second;
 		}
 		else
 		{
-		// If there are no functions available, calls to functions that
+			if (item.second.second >= static_cast<unsigned>(_numReturns))
-		// return a single value may be replaced by a dictionary token.
+				return item.first;
-		if (funcType == FunctionCall::SINGLE)
+		}
-			m_output << dictionaryToken();
+	return nullopt;
+}
+
+void ProtoConverter::visit(FunctionCall const& _x, string const& _functionName, bool _expression)
+{
+	yulAssert(m_functionSigMap.count(_functionName), "Proto fuzzer: Invalid function.");
+	auto ret = m_functionSigMap.at(_functionName);
+	unsigned numInParams = ret.first;
+	unsigned numOutParams = ret.second;
+
+	if (numOutParams == 0)
+	{
+		convertFunctionCall(_x, _functionName, numInParams);
 		return;
 	}
-
+	else
-	// If function selected for function call does not meet interface
-	// requirements (num output values) for the function type
-	// specified, then we return early unless it is a function call
-	// that returns a single value (which may be replaced by a
-	// dictionary token.
-	if (!functionValid(funcType, numOutParams))
 	{
-		if (funcType == FunctionCall::SINGLE)
+		yulAssert(numOutParams > 0, "");
-			m_output << dictionaryToken();
+		vector<string> varsVec;
-		return;
+		if (!_expression)
-	}
-
-	// If we are here, it means that we have at least one valid
-	// function for making the function call
-	switch (funcType)
 		{
-	case FunctionCall::ZERO:
-		convertFunctionCall(_x, funcName, numInParams);
-		break;
-	case FunctionCall::SINGLE:
-		// Since functions that return a single value are used as expressions
-		// we do not print a newline because it is done by the expression
-		// visitor.
-		convertFunctionCall(_x, funcName, numInParams, /*newLine=*/false);
-		break;
-	case FunctionCall::MULTIDECL:
-	{
-		// Ensure that the chosen function returns at most 4 values
-		yulAssert(
-			numOutParams <= 4,
-			"Proto fuzzer: Function call with too many output params encountered."
-		);
-
 			// Obtain variable name suffix
 			unsigned startIdx = counter();
-		vector<string> varsVec = createVarDecls(
+			varsVec = createVarDecls(
 				startIdx,
 				startIdx + numOutParams,
 				/*isAssignment=*/true
 			);
-
+		}
-		// Create RHS of multi var decl
+		convertFunctionCall(_x, _functionName, numInParams);
-		convertFunctionCall(_x, funcName, numInParams);
 		// Add newly minted vars in the multidecl statement to current scope
+		if (!_expression)
 			addVarsToScope(varsVec);
-		break;
-	}
-	case FunctionCall::MULTIASSIGN:
-		// Ensure that the chosen function returns at most 4 values
-		yulAssert(
-			numOutParams <= 4,
-			"Proto fuzzer: Function call with too many output params encountered."
-		);
-
-		// Return early if numOutParams > number of available variables
-		if (numOutParams > numVarsInScope())
-			return;
-
-		// Copy variables in scope in order to prevent repeated references
-		vector<string> variables;
-		if (m_inFunctionDef)
-			for (auto var: m_currentFuncVars)
-				variables.push_back(*var);
-		else
-			for (auto var: m_currentGlobalVars)
-				variables.push_back(*var);
-
-		auto refVar = [](vector<string>& _var, unsigned _rand, bool _comma = true) -> string
-			{
-				auto index = _rand % _var.size();
-				string ref = _var[index];
-				_var.erase(_var.begin() + index);
-				if (_comma)
-					ref += ", ";
-				return ref;
-			};
-
-		// Convert LHS of multi assignment
-		// We reverse the order of out param visits since the order does not matter.
-		// This helps reduce the size of this switch statement.
-		switch (numOutParams)
-		{
-		case 4:
-			m_output << refVar(variables, _x.out_param4().varnum());
-			[[fallthrough]];
-		case 3:
-			m_output << refVar(variables, _x.out_param3().varnum());
-			[[fallthrough]];
-		case 2:
-			m_output << refVar(variables, _x.out_param2().varnum());
-			m_output << refVar(variables, _x.out_param1().varnum(), false);
-			break;
-		default:
-			yulAssert(false, "Proto fuzzer: Function call with too many or too few input parameters.");
-			break;
-		}
-		m_output << " := ";
-
-		// Convert RHS of multi assignment
-		convertFunctionCall(_x, funcName, numInParams);
-		break;
 	}
 }
 
@@ -1463,10 +1358,13 @@ void ProtoConverter::visit(Statement const& _x)
 		visit(_x.terminatestmt());
 		break;
 	case Statement::kFunctioncall:
-		// Return early if a function call cannot be created
+		if (!m_functionSigMap.empty())
-		if (functionCallNotPossible(_x.functioncall().ret()))
+		{
-			return;
+			unsigned index = counter() % m_functionSigMap.size();
-		visit(_x.functioncall());
+			auto iter = m_functionSigMap.begin();
+			advance(iter, index);
+			visit(_x.functioncall(), iter->first);
+		}
 		break;
 	case Statement::kFuncdef:
 		if (_x.funcdef().block().statements_size() > 0)
@@ -1490,7 +1388,7 @@ void ProtoConverter::visit(Statement const& _x)
 
 void ProtoConverter::openBlockScope()
 {
-	m_scopeFuncs.emplace_back(vector<string>{});
+	m_scopeFuncs.emplace_back();
 
 	// Create new block scope inside current function scope
 	if (m_inFunctionDef)
@@ -1511,9 +1409,9 @@ void ProtoConverter::openBlockScope()
 	}
 	else
 	{
-		m_globalVars.emplace_back(vector<string>{});
+		m_globalVars.emplace_back();
 		if (m_inForInitScope && m_forInitScopeExtEnabled)
-			m_globalForLoopInitVars.emplace_back(vector<string>{});
+			m_globalForLoopInitVars.emplace_back();
 	}
 }
 

test/tools/ossfuzz/protoToYul.h

@@ -102,7 +102,15 @@ private:
 	void visit(RetRevStmt const&);
 	void visit(SelfDestructStmt const&);
 	void visit(TerminatingStmt const&);
-	void visit(FunctionCall const&);
+	/// @param _f is the function call to be visited.
+	/// @param _name is the name of the function called.
+	/// @param _expression is a flag that is true if the function is called
+	/// as a single-value expression, false otherwise.
+	void visit(
+		FunctionCall const& _f,
+		std::string const& _name,
+		bool _expression = false
+	);
 	void visit(FunctionDef const&);
 	void visit(PopStmt const&);
 	void visit(LeaveStmt const&);
@@ -125,8 +133,6 @@ private:
 	void closeFunctionScope();
 	/// Adds @a _vars to current scope
 	void addVarsToScope(std::vector<std::string> const& _vars);
-	/// @returns number of variables that are in scope
-	unsigned numVarsInScope();
 
 	std::string createHex(std::string const& _hexBytes);
 
@@ -140,9 +146,9 @@ private:
 	/// alphabets nor digits from it and returns the said string.
 	static std::string createAlphaNum(std::string const& _strBytes);
 
-	enum class NumFunctionReturns
+	enum class NumFunctionReturns: unsigned
 	{
-		None,
+		None = 0,
 		Single,
 		Multiple
 	};
@@ -176,34 +182,6 @@ private:
 	/// in scope
 	bool varDeclAvailable();
 
-	/// Return true if a function call cannot be made, false otherwise.
-	/// @param _type is an enum denoting the type of function call. It
-	/// can be one of NONE, SINGLE, MULTIDECL, MULTIASSIGN.
-	///		NONE -> Function call does not return a value
-	///		SINGLE -> Function call returns a single value
-	///		MULTIDECL -> Function call returns more than one value
-	///		and it is used to create a multi declaration
-	///		statement
-	///		MULTIASSIGN -> Function call returns more than one value
-	///		and it is used to create a multi assignment
-	///		statement
-	/// @return True if the function call cannot be created for one of the
-	/// following reasons
-	//   - It is a SINGLE function call (we reserve SINGLE functions for
-	//   expressions)
-	//   - It is a MULTIASSIGN function call and we do not have any
-	//   variables available for assignment.
-	bool functionCallNotPossible(FunctionCall_Returns _type);
-
-	/// Checks if function call of type @a _type returns the correct number
-	/// of values.
-	/// @param _type Function call type of the function being checked
-	/// @param _numOutParams Number of values returned by the function
-	/// being checked
-	/// @return true if the function returns the correct number of values,
-	/// false otherwise
-	bool functionValid(FunctionCall_Returns _type, unsigned _numOutParams);
-
 	/// Converts protobuf function call to a Yul function call and appends
 	/// it to output stream.
 	/// @param _x Protobuf function call
@@ -295,6 +273,9 @@ private:
 	/// enum of type Program_Version
 	static solidity::langutil::EVMVersion evmVersionMapping(Program_Version const& _x);
 
+	/// @returns name of Yul function with return type of @param _numReturns.
+	std::optional<std::string> functionExists(NumFunctionReturns _numReturns);
+
 	/// Returns a monotonically increasing counter that starts from zero.
 	unsigned counter()
 	{

test/tools/ossfuzz/protomutators/YulProtoMutator.cpp

@@ -107,9 +107,7 @@ template <typename T>
 T YPM::EnumTypeConverter<T>::validEnum(unsigned _seed)
 {
 	auto ret = static_cast<T>(_seed % (enumMax() - enumMin() + 1) + enumMin());
-	if constexpr (std::is_same_v<std::decay_t<T>, FunctionCall_Returns>)
+	if constexpr (std::is_same_v<std::decay_t<T>, StoreFunc_Storage>)
-		yulAssert(FunctionCall_Returns_IsValid(ret), "Yul proto mutator: Invalid enum");
-	else if constexpr (std::is_same_v<std::decay_t<T>, StoreFunc_Storage>)
 		yulAssert(StoreFunc_Storage_IsValid(ret), "Yul proto mutator: Invalid enum");
 	else if constexpr (std::is_same_v<std::decay_t<T>, NullaryOp_NOp>)
 		yulAssert(NullaryOp_NOp_IsValid(ret), "Yul proto mutator: Invalid enum");
@@ -131,9 +129,7 @@ T YPM::EnumTypeConverter<T>::validEnum(unsigned _seed)
 template <typename T>
 unsigned YPM::EnumTypeConverter<T>::enumMax()
 {
-	if constexpr (std::is_same_v<std::decay_t<T>, FunctionCall_Returns>)
+	if constexpr (std::is_same_v<std::decay_t<T>, StoreFunc_Storage>)
-		return FunctionCall_Returns_Returns_MAX;
-	else if constexpr (std::is_same_v<std::decay_t<T>, StoreFunc_Storage>)
 		return StoreFunc_Storage_Storage_MAX;
 	else if constexpr (std::is_same_v<std::decay_t<T>, NullaryOp_NOp>)
 		return NullaryOp_NOp_NOp_MAX;
@@ -154,9 +150,7 @@ unsigned YPM::EnumTypeConverter<T>::enumMax()
 template <typename T>
 unsigned YPM::EnumTypeConverter<T>::enumMin()
 {
-	if constexpr (std::is_same_v<std::decay_t<T>, FunctionCall_Returns>)
+	if constexpr (std::is_same_v<std::decay_t<T>, StoreFunc_Storage>)
-		return FunctionCall_Returns_Returns_MIN;
-	else if constexpr (std::is_same_v<std::decay_t<T>, StoreFunc_Storage>)
 		return StoreFunc_Storage_Storage_MIN;
 	else if constexpr (std::is_same_v<std::decay_t<T>, NullaryOp_NOp>)
 		return NullaryOp_NOp_NOp_MIN;

test/tools/ossfuzz/yulProto.proto

@@ -58,23 +58,10 @@ message Create {
 }
 
 message FunctionCall {
-  enum Returns {
+  required Expression in_param1 = 1;
-    ZERO = 1;
+  required Expression in_param2 = 2;
-    SINGLE = 2;
+  required Expression in_param3 = 3;
-    MULTIDECL = 3;
+  required Expression in_param4 = 4;
-    MULTIASSIGN = 4;
-  }
-  required Returns ret = 1;
-  // Indexes an existing function
-  required uint32 func_index = 2;
-  required Expression in_param1 = 3;
-  required Expression in_param2 = 4;
-  required Expression in_param3 = 5;
-  required Expression in_param4 = 6;
-  required VarRef out_param1 = 7;
-  required VarRef out_param2 = 8;
-  required VarRef out_param3 = 9;
-  required VarRef out_param4 = 10;
 }
 
 message TypedVarDecl {