Merge pull request #7765 from ethereum/fix-7626

yul proto fuzzer: permit variable declarations inside for-init block

test/tools/ossfuzz/protoToYul.cpp

@@ -95,18 +95,32 @@ string ProtoConverter::visit(Literal const& _x)
 void ProtoConverter::consolidateVarDeclsInFunctionDef()
 {
 	m_currentFuncVars.clear();
+	yulAssert(!m_funcVars.empty(), "Proto fuzzer: Invalid operation");
+
 	auto const& scopes = m_funcVars.back();
 	for (auto const& s: scopes)
 		for (auto const& var: s)
 			m_currentFuncVars.push_back(&var);
+	yulAssert(!m_funcForLoopInitVars.empty(), "Proto fuzzer: Invalid operation");
+	auto const& forinitscopes = m_funcForLoopInitVars.back();
+	for (auto const& s: forinitscopes)
+		for (auto const& var: s)
+			m_currentFuncVars.push_back(&var);
 }
 
 void ProtoConverter::consolidateGlobalVarDecls()
 {
 	m_currentGlobalVars.clear();
+	// Place pointers to all global variables that are in scope
+	// into a single vector
 	for (auto const& scope: m_globalVars)
 		for (auto const& var: scope)
 			m_currentGlobalVars.push_back(&var);
+	// Place pointers to all variables declared in for-init blocks
+	// that are still live into the same vector
+	for (auto const& init: m_globalForLoopInitVars)
+		for (auto const& var: init)
+			m_currentGlobalVars.push_back(&var);
 }
 
 bool ProtoConverter::varDeclAvailable()
@@ -283,10 +297,54 @@ void ProtoConverter::visit(VarDecl const& _x)
 	m_output << "let " << varName << " := ";
 	visit(_x.expr());
 	m_output << "\n";
+	// If we are inside a for-init block, there are two places
+	// where the visited vardecl may have been defined:
+	// - directly inside the for-init block
+	// - inside a block within the for-init block
+	// In the latter case, we don't scope extend.
 	if (m_inFunctionDef)
-		m_funcVars.back().back().push_back(varName);
+	{
+		// Variables declared directly in for-init block
+		// are tracked separately because their scope
+		// extends beyond the block they are defined in
+		// to the rest of the for-loop statement.
+		if (m_inForInitScope && m_forInitScopeExtEnabled)
+		{
+			yulAssert(
+				!m_funcForLoopInitVars.empty() && !m_funcForLoopInitVars.back().empty(),
+				"Proto fuzzer: Invalid operation"
+			);
+			m_funcForLoopInitVars.back().back().push_back(varName);
+		}
+		else
+		{
+			yulAssert(
+				!m_funcVars.empty() && !m_funcVars.back().empty(),
+				"Proto fuzzer: Invalid operation"
+			);
+			m_funcVars.back().back().push_back(varName);
+		}
+
+	}
 	else
-		m_globalVars.back().push_back(varName);
+	{
+		if (m_inForInitScope && m_forInitScopeExtEnabled)
+		{
+			yulAssert(
+				!m_globalForLoopInitVars.empty(),
+				"Proto fuzzer: Invalid operation"
+			);
+			m_globalForLoopInitVars.back().push_back(varName);
+		}
+		else
+		{
+			yulAssert(
+				!m_globalVars.empty(),
+				"Proto fuzzer: Invalid operation"
+			);
+			m_globalVars.back().push_back(varName);
+		}
+	}
 }
 
 void ProtoConverter::visit(TypedVarDecl const& _x)
@@ -351,10 +409,53 @@ void ProtoConverter::visit(TypedVarDecl const& _x)
 		m_output << " : u256\n";
 		break;
 	}
+	// If we are inside a for-init block, there are two places
+	// where the visited vardecl may have been defined:
+	// - directly inside the for-init block
+	// - inside a block within the for-init block
+	// In the latter case, we don't scope extend.
 	if (m_inFunctionDef)
-		m_funcVars.back().back().push_back(varName);
+	{
+		// Variables declared directly in for-init block
+		// are tracked separately because their scope
+		// extends beyond the block they are defined in
+		// to the rest of the for-loop statement.
+		if (m_inForInitScope && m_forInitScopeExtEnabled)
+		{
+			yulAssert(
+				!m_funcForLoopInitVars.empty() && !m_funcForLoopInitVars.back().empty(),
+				"Proto fuzzer: Invalid operation"
+			);
+			m_funcForLoopInitVars.back().back().push_back(varName);
+		}
+		else
+		{
+			yulAssert(
+				!m_funcVars.empty() && !m_funcVars.back().empty(),
+				"Proto fuzzer: Invalid operation"
+			);
+			m_funcVars.back().back().push_back(varName);
+		}
+	}
 	else
-		m_globalVars.back().push_back(varName);
+	{
+		if (m_inForInitScope && m_forInitScopeExtEnabled)
+		{
+			yulAssert(
+				!m_globalForLoopInitVars.empty(),
+				"Proto fuzzer: Invalid operation"
+			);
+			m_globalForLoopInitVars.back().push_back(varName);
+		}
+		else
+		{
+			yulAssert(
+				!m_globalVars.empty(),
+				"Proto fuzzer: Invalid operation"
+			);
+			m_globalVars.back().push_back(varName);
+		}
+	}
 }
 
 void ProtoConverter::visit(UnaryOp const& _x)
@@ -713,30 +814,27 @@ void ProtoConverter::visit(FunctionCall const& _x)
 		convertFunctionCall(_x, funcName, numInParams, /*newLine=*/false);
 		break;
 	case FunctionCall::MULTIDECL:
-		// Hack: Disallow (multi) variable declarations until scope extension
+	{
-		// is implemented for "for-init"
+		// Ensure that the chosen function returns at most 4 values
-		if (!m_inForInitScope)
+		yulAssert(
-		{
+			numOutParams <= 4,
-			// Ensure that the chosen function returns at most 4 values
+			"Proto fuzzer: Function call with too many output params encountered."
-			yulAssert(
+		);
-				numOutParams <= 4,
-				"Proto fuzzer: Function call with too many output params encountered."
-			);
 
-			// Obtain variable name suffix
+		// Obtain variable name suffix
-			unsigned startIdx = counter();
+		unsigned startIdx = counter();
-			vector<string> varsVec = createVarDecls(
+		vector<string> varsVec = createVarDecls(
-				startIdx,
+			startIdx,
-				startIdx + numOutParams,
+			startIdx + numOutParams,
-				/*isAssignment=*/true
+			/*isAssignment=*/true
-			);
+		);
 
-			// Create RHS of multi var decl
+		// Create RHS of multi var decl
-			convertFunctionCall(_x, funcName, numInParams);
+		convertFunctionCall(_x, funcName, numInParams);
-			// Add newly minted vars in the multidecl statement to current scope
+		// Add newly minted vars in the multidecl statement to current scope
-			addVarsToScope(varsVec);
+		addVarsToScope(varsVec);
-		}
 		break;
+	}
 	case FunctionCall::MULTIASSIGN:
 		// Ensure that the chosen function returns at most 4 values
 		yulAssert(
@@ -749,22 +847,22 @@ void ProtoConverter::visit(FunctionCall const& _x)
 		// This helps reduce the size of this switch statement.
 		switch (numOutParams)
 		{
-			case 4:
+		case 4:
-				visit(_x.out_param4());
+			visit(_x.out_param4());
-				m_output << ", ";
+			m_output << ", ";
-				BOOST_FALLTHROUGH;
+			BOOST_FALLTHROUGH;
-			case 3:
+		case 3:
-				visit(_x.out_param3());
+			visit(_x.out_param3());
-				m_output << ", ";
+			m_output << ", ";
-				BOOST_FALLTHROUGH;
+			BOOST_FALLTHROUGH;
-			case 2:
+		case 2:
-				visit(_x.out_param2());
+			visit(_x.out_param2());
-				m_output << ", ";
+			m_output << ", ";
-				visit(_x.out_param1());
+			visit(_x.out_param1());
-				break;
+			break;
-			default:
+		default:
-				yulAssert(false, "Proto fuzzer: Function call with too many or too few input parameters.");
+			yulAssert(false, "Proto fuzzer: Function call with too many or too few input parameters.");
-				break;
+			break;
 		}
 		m_output << " := ";
 
@@ -868,17 +966,33 @@ void ProtoConverter::visit(ForStmt const& _x)
 {
 	bool wasInForBody = m_inForBodyScope;
 	bool wasInForInit = m_inForInitScope;
+	bool wasForInitScopeExtEnabled = m_forInitScopeExtEnabled;
 	m_inForBodyScope = false;
 	m_inForInitScope = true;
+	m_forInitScopeExtEnabled = true;
 	m_output << "for ";
 	visit(_x.for_init());
 	m_inForInitScope = false;
+	m_forInitScopeExtEnabled = wasForInitScopeExtEnabled;
 	visit(_x.for_cond());
 	visit(_x.for_post());
 	m_inForBodyScope = true;
 	visit(_x.for_body());
 	m_inForBodyScope = wasInForBody;
 	m_inForInitScope = wasInForInit;
+	if (m_inFunctionDef)
+	{
+		yulAssert(
+			!m_funcForLoopInitVars.empty() && !m_funcForLoopInitVars.back().empty(),
+			"Proto fuzzer: Invalid data structure");
+		// Remove variables in for-init
+		m_funcForLoopInitVars.back().pop_back();
+	}
+	else
+	{
+		yulAssert(!m_globalForLoopInitVars.empty(), "Proto fuzzer: Invalid data structure");
+		m_globalForLoopInitVars.pop_back();
+	}
 }
 
 void ProtoConverter::visit(BoundedForStmt const& _x)
@@ -1065,9 +1179,7 @@ void ProtoConverter::visit(Statement const& _x)
 	switch (_x.stmt_oneof_case())
 	{
 	case Statement::kDecl:
-		// Hack: Disallow (multi) variable declarations until scope extension is implemented for "for-init"
+		visit(_x.decl());
-		if (!m_inForInitScope)
-			visit(_x.decl());
 		break;
 	case Statement::kAssignment:
 		// Create an assignment statement only if there is at least one variable
@@ -1138,16 +1250,36 @@ void ProtoConverter::visit(Statement const& _x)
 void ProtoConverter::openBlockScope()
 {
 	m_scopeFuncs.push_back({});
+
 	// Create new block scope inside current function scope
 	if (m_inFunctionDef)
+	{
+		yulAssert(
+			!m_funcVars.empty(),
+			"Proto fuzzer: Invalid data structure"
+		);
 		m_funcVars.back().push_back(vector<string>{});
+		if (m_inForInitScope && m_forInitScopeExtEnabled)
+		{
+			yulAssert(
+				!m_funcForLoopInitVars.empty(),
+				"Proto fuzzer: Invalid data structure"
+			);
+			m_funcForLoopInitVars.back().push_back(vector<string>{});
+		}
+	}
 	else
-		m_globalVars.push_back(vector<string>{});
+	{
+		m_globalVars.push_back({});
+		if (m_inForInitScope && m_forInitScopeExtEnabled)
+			m_globalForLoopInitVars.push_back(vector<string>{});
+	}
 }
 
 void ProtoConverter::openFunctionScope(vector<string> const& _funcParams)
 {
 	m_funcVars.push_back(vector<vector<string>>({_funcParams}));
+	m_funcForLoopInitVars.push_back(vector<vector<string>>({}));
 }
 
 void ProtoConverter::updateFunctionMaps(string const& _var)
@@ -1166,6 +1298,8 @@ void ProtoConverter::updateFunctionMaps(string const& _var)
 
 void ProtoConverter::closeBlockScope()
 {
+	// Remove functions declared in the block that is going
+	// out of scope from the global function map.
 	for (auto const& f: m_scopeFuncs.back())
 	{
 		unsigned numFuncsRemoved = m_functions.size();
@@ -1177,35 +1311,95 @@ void ProtoConverter::closeBlockScope()
 		);
 		updateFunctionMaps(f);
 	}
+	// Pop back the vector of scoped functions.
 	if (!m_scopeFuncs.empty())
 		m_scopeFuncs.pop_back();
-	if (!m_inFunctionDef)
+
+	// If block belongs to function body, then remove
+	// local variables in function body that are going out of scope.
+	if (m_inFunctionDef)
 	{
-		if (!m_globalVars.empty())
+		yulAssert(!m_funcVars.empty(), "Proto fuzzer: Invalid data structure");
-			m_globalVars.pop_back();
+		if (!m_funcVars.back().empty())
+			m_funcVars.back().pop_back();
 	}
+	// Remove variables declared in vanilla block from current
+	// global scope.
 	else
 	{
-		// Variables that have been declared in a
+		yulAssert(!m_globalVars.empty(), "Proto fuzzer: Invalid data structure");
-		// function block, go out of scope
+		m_globalVars.pop_back();
-		if (!m_funcVars.empty())
-			if (!m_funcVars.back().empty())
-				m_funcVars.back().pop_back();
 	}
 }
 
 void ProtoConverter::closeFunctionScope()
 {
-	if (!m_funcVars.empty())
+	yulAssert(!m_funcVars.empty(), "Proto fuzzer: Invalid data structure");
-		m_funcVars.pop_back();
+	m_funcVars.pop_back();
+	yulAssert(!m_funcForLoopInitVars.empty(), "Proto fuzzer: Invalid data structure");
+	m_funcForLoopInitVars.pop_back();
 }
 
 void ProtoConverter::addVarsToScope(vector<string> const& _vars)
 {
+	// If we are in function definition, add the new vars to current function scope
 	if (m_inFunctionDef)
-		m_funcVars.back().back().insert(m_funcVars.back().back().end(), _vars.begin(), _vars.end());
+	{
+		// If we are directly in for-init block, add the newly created vars to the
+		// stack of for-init variables.
+		if (m_inForInitScope && m_forInitScopeExtEnabled)
+		{
+			yulAssert(
+				!m_funcForLoopInitVars.empty() && !m_funcForLoopInitVars.back().empty(),
+				"Proto fuzzer: Invalid data structure"
+			);
+			m_funcForLoopInitVars.back().back().insert(
+				m_funcForLoopInitVars.back().back().end(),
+				_vars.begin(),
+				_vars.end()
+			);
+		}
+		else
+		{
+			yulAssert(
+				!m_funcVars.empty() && !m_funcVars.back().empty(),
+				"Proto fuzzer: Invalid data structure"
+			);
+			m_funcVars.back().back().insert(
+				m_funcVars.back().back().end(),
+				_vars.begin(),
+				_vars.end()
+			);
+		}
+	}
+	// If we are in a vanilla block, add the new vars to current global scope
 	else
-		m_globalVars.back().insert(m_globalVars.back().end(), _vars.begin(), _vars.end());
+	{
+		if (m_inForInitScope && m_forInitScopeExtEnabled)
+		{
+			yulAssert(
+				!m_globalForLoopInitVars.empty(),
+				"Proto fuzzer: Invalid data structure"
+			);
+			m_globalForLoopInitVars.back().insert(
+				m_globalForLoopInitVars.back().end(),
+				_vars.begin(),
+				_vars.end()
+			);
+		}
+		else
+		{
+			yulAssert(
+				!m_globalVars.empty(),
+				"Proto fuzzer: Invalid data structure"
+			);
+			m_globalVars.back().insert(
+				m_globalVars.back().end(),
+				_vars.begin(),
+				_vars.end()
+			);
+		}
+	}
 }
 
 void ProtoConverter::visit(Block const& _x)
@@ -1222,8 +1416,20 @@ void ProtoConverter::visit(Block const& _x)
 	if (_x.statements_size() > 0)
 	{
 		m_output << "{\n";
+		bool wasForInitScopeExtEnabled = m_forInitScopeExtEnabled;
 		for (auto const& st: _x.statements())
+		{
+			// If statement is block or introduces one and we are in for-init block
+			// then temporarily disable scope extension if it is not already disabled.
+			if (
+				(st.has_blockstmt() || st.has_switchstmt() || st.has_ifstmt()) &&
+				m_inForInitScope &&
+				m_forInitScopeExtEnabled
+			)
+				m_forInitScopeExtEnabled = false;
 			visit(st);
+			m_forInitScopeExtEnabled = wasForInitScopeExtEnabled;
+		}
 		m_output << "}\n";
 	}
 	else

test/tools/ossfuzz/protoToYul.h

@@ -48,6 +48,7 @@ public:
 		m_inFunctionDef = false;
 		m_objectId = 0;
 		m_isObject = false;
+		m_forInitScopeExtEnabled = true;
 	}
 	ProtoConverter(ProtoConverter const&) = delete;
 	ProtoConverter(ProtoConverter&&) = delete;
@@ -314,9 +315,13 @@ private:
 	std::vector<std::string const*> m_currentGlobalVars;
 	/// Functions in current scope
 	std::vector<std::vector<std::string>> m_scopeFuncs;
-	/// Variables
+	/// Global variables
 	std::vector<std::vector<std::string>> m_globalVars;
-	/// Functions
+	/// Variables declared in for loop init block that is in global scope
+	std::vector<std::vector<std::string>> m_globalForLoopInitVars;
+	/// Variables declared in for loop init block that is in function scope
+	std::vector<std::vector<std::vector<std::string>>> m_funcForLoopInitVars;
+	/// Vector of function names
 	std::vector<std::string> m_functions;
 	/// Maps FunctionDef object to its name
 	std::map<FunctionDef const*, std::string> m_functionDefMap;
@@ -331,7 +336,7 @@ private:
 	static unsigned constexpr s_modOutputParams = 5;
 	/// Hard-coded identifier for a Yul object's data block
 	static auto constexpr s_dataIdentifier = "datablock";
-	/// Predicate to keep track of for body scope. If true, break/continue
+	/// Predicate to keep track of for body scope. If false, break/continue
 	/// statements can not be created.
 	bool m_inForBodyScope;
 	// Index used for naming loop variable of bounded for loops
@@ -350,5 +355,8 @@ private:
 	/// Flag to track whether program is an object (true) or a statement block
 	/// (false: default value)
 	bool m_isObject;
+	/// Flag to track whether scope extension of variables defined in for-init
+	/// block is enabled.
+	bool m_forInitScopeExtEnabled;
 };
 }