Merge pull request #9569 from ethereum/yul-dialect-literalkind

Include literal kind of Dialect for builtins

Changelog.md

@@ -4,7 +4,7 @@ Language Features:
 
 
 Compiler Features:
-
+ * Yul: Report error when using non-string literals for ``datasize()``, ``dataoffset()``, ``linkersymbol()``, ``loadimmutable()``, ``setimmutable()``.
 
 Bugfixes:
  * Optimizer: Keep side-effects of ``x`` in ``byte(a, shr(b, x))`` even if the constants ``a`` and ``b`` would make the expression zero unconditionally. This optimizer rule is very hard if not impossible to trigger in a way that it can result in invalid code, though.

libyul/AsmAnalysis.cpp

@@ -44,6 +44,20 @@ using namespace solidity::yul;
 using namespace solidity::util;
 using namespace solidity::langutil;
 
+namespace
+{
+inline string to_string(LiteralKind _kind)
+{
+	switch (_kind)
+	{
+	case LiteralKind::Number: return "number";
+	case LiteralKind::Boolean: return "boolean";
+	case LiteralKind::String: return "string";
+	default: yulAssert(false, "");
+	}
+}
+}
+
 bool AsmAnalyzer::analyze(Block const& _block)
 {
 	auto watcher = m_errorReporter.errorWatcher();
@@ -272,14 +286,14 @@ vector<YulString> AsmAnalyzer::operator()(FunctionCall const& _funCall)
 	auto watcher = m_errorReporter.errorWatcher();
 	vector<YulString> const* parameterTypes = nullptr;
 	vector<YulString> const* returnTypes = nullptr;
-	vector<bool> const* needsLiteralArguments = nullptr;
+	vector<optional<LiteralKind>> const* literalArguments = nullptr;
 
 	if (BuiltinFunction const* f = m_dialect.builtin(_funCall.functionName.name))
 	{
 		parameterTypes = &f->parameters;
 		returnTypes = &f->returns;
-		if (f->literalArguments)
-			needsLiteralArguments = &f->literalArguments.value();
+		if (!f->literalArguments.empty())
+			literalArguments = &f->literalArguments;
 
 		validateInstructions(_funCall);
 	}
@@ -318,15 +332,7 @@ vector<YulString> AsmAnalyzer::operator()(FunctionCall const& _funCall)
 	for (size_t i = _funCall.arguments.size(); i > 0; i--)
 	{
 		Expression const& arg = _funCall.arguments[i - 1];
-		bool isLiteralArgument = needsLiteralArguments && (*needsLiteralArguments)[i - 1];
-		bool isStringLiteral = holds_alternative<Literal>(arg) && get<Literal>(arg).kind == LiteralKind::String;
-
-		if (isLiteralArgument && isStringLiteral)
-			argTypes.emplace_back(expectUnlimitedStringLiteral(get<Literal>(arg)));
-		else
-			argTypes.emplace_back(expectExpression(arg));
-
-		if (isLiteralArgument)
+		if (auto literalArgumentKind = literalArguments ? literalArguments->at(i - 1) : std::nullopt)
 		{
 			if (!holds_alternative<Literal>(arg))
 				m_errorReporter.typeError(
@@ -334,17 +340,29 @@ vector<YulString> AsmAnalyzer::operator()(FunctionCall const& _funCall)
 					_funCall.functionName.location,
 					"Function expects direct literals as arguments."
 				);
-			else if (
-				_funCall.functionName.name.str() == "datasize" ||
-				_funCall.functionName.name.str() == "dataoffset"
-			)
-				if (!m_dataNames.count(std::get<Literal>(arg).value))
-					m_errorReporter.typeError(
-						3517_error,
-						_funCall.functionName.location,
-						"Unknown data object \"" + std::get<Literal>(arg).value.str() + "\"."
-					);
+			else if (*literalArgumentKind != get<Literal>(arg).kind)
+				m_errorReporter.typeError(
+					5859_error,
+					get<Literal>(arg).location,
+					"Function expects " + to_string(*literalArgumentKind) + " literal."
+				);
+			else if (*literalArgumentKind == LiteralKind::String)
+			{
+				if (
+					_funCall.functionName.name.str() == "datasize" ||
+					_funCall.functionName.name.str() == "dataoffset"
+				)
+					if (!m_dataNames.count(get<Literal>(arg).value))
+						m_errorReporter.typeError(
+							3517_error,
+							get<Literal>(arg).location,
+							"Unknown data object \"" + std::get<Literal>(arg).value.str() + "\"."
+						);
+				argTypes.emplace_back(expectUnlimitedStringLiteral(get<Literal>(arg)));
+				continue;
+			}
 		}
+		argTypes.emplace_back(expectExpression(arg));
 	}
 	std::reverse(argTypes.begin(), argTypes.end());
 

libyul/Dialect.h

@@ -48,8 +48,13 @@ struct BuiltinFunction
 	ControlFlowSideEffects controlFlowSideEffects;
 	/// If true, this is the msize instruction.
 	bool isMSize = false;
-	/// If set, same length as the arguments, if true at index i, the i'th argument has to be a literal which means it can't be moved to variables.
-	std::optional<std::vector<bool>> literalArguments;
+	/// Must be empty or the same length as the arguments.
+	/// If set at index i, the i'th argument has to be a literal which means it can't be moved to variables.
+	std::vector<std::optional<LiteralKind>> literalArguments{};
+	std::optional<LiteralKind> literalArgument(size_t i) const
+	{
+		return literalArguments.empty() ? std::nullopt : literalArguments.at(i);
+	}
 };
 
 struct Dialect: boost::noncopyable

libyul/backends/evm/EVMDialect.cpp

@@ -70,7 +70,7 @@ pair<YulString, BuiltinFunctionForEVM> createEVMFunction(
 	f.controlFlowSideEffects.terminates = evmasm::SemanticInformation::terminatesControlFlow(_instruction);
 	f.controlFlowSideEffects.reverts = evmasm::SemanticInformation::reverts(_instruction);
 	f.isMSize = _instruction == evmasm::Instruction::MSIZE;
-	f.literalArguments.reset();
+	f.literalArguments.clear();
 	f.instruction = _instruction;
 	f.generateCode = [_instruction](
 		FunctionCall const& _call,
@@ -90,7 +90,7 @@ pair<YulString, BuiltinFunctionForEVM> createFunction(
 	size_t _params,
 	size_t _returns,
 	SideEffects _sideEffects,
-	vector<bool> _literalArguments,
+	vector<optional<LiteralKind>> _literalArguments,
 	std::function<void(FunctionCall const&, AbstractAssembly&, BuiltinContext&, std::function<void(Expression const&)>)> _generateCode
 )
 {
@@ -102,10 +102,7 @@ pair<YulString, BuiltinFunctionForEVM> createFunction(
 	f.parameters.resize(_params);
 	f.returns.resize(_returns);
 	f.sideEffects = std::move(_sideEffects);
-	if (!_literalArguments.empty())
-		f.literalArguments = std::move(_literalArguments);
-	else
-		f.literalArguments.reset();
+	f.literalArguments = std::move(_literalArguments);
 	f.isMSize = false;
 	f.instruction = {};
 	f.generateCode = std::move(_generateCode);
@@ -135,7 +132,7 @@ map<YulString, BuiltinFunctionForEVM> createBuiltins(langutil::EVMVersion _evmVe
 
 	if (_objectAccess)
 	{
-		builtins.emplace(createFunction("linkersymbol", 1, 1, SideEffects{}, {true}, [](
+		builtins.emplace(createFunction("linkersymbol", 1, 1, SideEffects{}, {LiteralKind::String}, [](
 			FunctionCall const& _call,
 			AbstractAssembly& _assembly,
 			BuiltinContext&,
@@ -145,7 +142,7 @@ map<YulString, BuiltinFunctionForEVM> createBuiltins(langutil::EVMVersion _evmVe
 			Expression const& arg = _call.arguments.front();
 			_assembly.appendLinkerSymbol(std::get<Literal>(arg).value.str());
 		}));
-		builtins.emplace(createFunction("datasize", 1, 1, SideEffects{}, {true}, [](
+		builtins.emplace(createFunction("datasize", 1, 1, SideEffects{}, {LiteralKind::String}, [](
 			FunctionCall const& _call,
 			AbstractAssembly& _assembly,
 			BuiltinContext& _context,
@@ -167,7 +164,7 @@ map<YulString, BuiltinFunctionForEVM> createBuiltins(langutil::EVMVersion _evmVe
 				_assembly.appendDataSize(subIdPath);
 			}
 		}));
-		builtins.emplace(createFunction("dataoffset", 1, 1, SideEffects{}, {true}, [](
+		builtins.emplace(createFunction("dataoffset", 1, 1, SideEffects{}, {LiteralKind::String}, [](
 			FunctionCall const& _call,
 			AbstractAssembly& _assembly,
 			BuiltinContext& _context,
@@ -210,7 +207,7 @@ map<YulString, BuiltinFunctionForEVM> createBuiltins(langutil::EVMVersion _evmVe
 			2,
 			0,
 			SideEffects{false, false, false, false, true},
-			{true, false},
+			{LiteralKind::String, std::nullopt},
 			[](
 				FunctionCall const& _call,
 				AbstractAssembly& _assembly,
@@ -230,7 +227,7 @@ map<YulString, BuiltinFunctionForEVM> createBuiltins(langutil::EVMVersion _evmVe
 			1,
 			1,
 			SideEffects{},
-			{true},
+			{LiteralKind::String},
 			[](
 				FunctionCall const& _call,
 				AbstractAssembly& _assembly,

libyul/backends/evm/NoOutputAssembly.cpp

@@ -170,7 +170,7 @@ NoOutputEVMDialect::NoOutputEVMDialect(EVMDialect const& _copyFrom):
 			for (size_t j = 0; j < _call.arguments.size(); j++)
 			{
 				size_t const i = _call.arguments.size() - j - 1;
-				if (!(fun.second.literalArguments && (*fun.second.literalArguments)[i]))
+				if (!fun.second.literalArgument(i))
 				{
 					_visitExpression(_call.arguments[i]);
 					visited++;

libyul/backends/wasm/WasmCodeTransform.cpp

@@ -140,22 +140,20 @@ wasm::Expression WasmCodeTransform::operator()(FunctionCall const& _call)
 				m_functionsToImport[builtin->name] = std::move(imp);
 			}
 		}
-		else if (builtin->literalArguments && contains(builtin->literalArguments.value(), true))
-		{
-			vector<wasm::Expression> literals;
-			for (size_t i = 0; i < _call.arguments.size(); i++)
-				if (builtin->literalArguments.value()[i])
-					literals.emplace_back(wasm::StringLiteral{std::get<Literal>(_call.arguments[i]).value.str()});
-				else
-					literals.emplace_back(visitReturnByValue(_call.arguments[i]));
-
-			return wasm::BuiltinCall{_call.functionName.name.str(), std::move(literals)};
-		}
 		else
-			return wasm::BuiltinCall{
-				_call.functionName.name.str(),
-				visit(_call.arguments)
-			};
+		{
+			vector<wasm::Expression> arguments;
+			for (size_t i = 0; i < _call.arguments.size(); i++)
+				if (builtin->literalArgument(i))
+				{
+					yulAssert(builtin->literalArgument(i) == LiteralKind::String, "");
+					arguments.emplace_back(wasm::StringLiteral{std::get<Literal>(_call.arguments[i]).value.str()});
+				}
+				else
+					arguments.emplace_back(visitReturnByValue(_call.arguments[i]));
+
+			return wasm::BuiltinCall{_call.functionName.name.str(), std::move(arguments)};
+		}
 	}
 
 	// If this function returns multiple values, then the first one will

libyul/backends/wasm/WasmDialect.cpp

@@ -21,6 +21,7 @@
 
 #include <libyul/backends/wasm/WasmDialect.h>
 
+#include <libyul/AsmData.h>
 #include <libyul/Exceptions.h>
 
 using namespace std;
@@ -119,8 +120,8 @@ WasmDialect::WasmDialect()
 	m_functions["unreachable"_yulstring].controlFlowSideEffects.terminates = true;
 	m_functions["unreachable"_yulstring].controlFlowSideEffects.reverts = true;
 
-	addFunction("datasize", {i64}, {i64}, true, {true});
-	addFunction("dataoffset", {i64}, {i64}, true, {true});
+	addFunction("datasize", {i64}, {i64}, true, {LiteralKind::String});
+	addFunction("dataoffset", {i64}, {i64}, true, {LiteralKind::String});
 
 	addEthereumExternals();
 }
@@ -221,7 +222,7 @@ void WasmDialect::addEthereumExternals()
 		f.controlFlowSideEffects = ext.controlFlowSideEffects;
 		f.isMSize = false;
 		f.sideEffects.invalidatesStorage = (ext.name == "storageStore");
-		f.literalArguments.reset();
+		f.literalArguments.clear();
 	}
 }
 
@@ -230,7 +231,7 @@ void WasmDialect::addFunction(
 	vector<YulString> _params,
 	vector<YulString> _returns,
 	bool _movable,
-	std::vector<bool> _literalArguments
+	vector<optional<LiteralKind>> _literalArguments
 )
 {
 	YulString name{move(_name)};
@@ -241,8 +242,5 @@ void WasmDialect::addFunction(
 	f.returns = std::move(_returns);
 	f.sideEffects = _movable ? SideEffects{} : SideEffects::worst();
 	f.isMSize = false;
-	if (!_literalArguments.empty())
-		f.literalArguments = std::move(_literalArguments);
-	else
-		f.literalArguments.reset();
+	f.literalArguments = std::move(_literalArguments);
 }

libyul/backends/wasm/WasmDialect.h

@@ -62,7 +62,7 @@ private:
 		std::vector<YulString> _params,
 		std::vector<YulString> _returns,
 		bool _movable = true,
-		std::vector<bool> _literalArguments = std::vector<bool>{}
+		std::vector<std::optional<LiteralKind>> _literalArguments = std::vector<std::optional<LiteralKind>>{}
 	);
 
 	std::map<YulString, BuiltinFunction> m_functions;

libyul/backends/wasm/WordSizeTransform.cpp

@@ -42,16 +42,16 @@ void WordSizeTransform::operator()(FunctionDefinition& _fd)
 
 void WordSizeTransform::operator()(FunctionCall& _fc)
 {
-	vector<bool> const* literalArguments = nullptr;
+	vector<optional<LiteralKind>> const* literalArguments = nullptr;
 
 	if (BuiltinFunction const* fun = m_inputDialect.builtin(_fc.functionName.name))
-		if (fun->literalArguments)
-			literalArguments = &fun->literalArguments.value();
+		if (!fun->literalArguments.empty())
+			literalArguments = &fun->literalArguments;
 
 	vector<Expression> newArgs;
 
 	for (size_t i = 0; i < _fc.arguments.size(); i++)
-		if (!literalArguments || !(*literalArguments)[i])
+		if (!literalArguments || !(*literalArguments)[i].has_value())
 			newArgs += expandValueToVector(_fc.arguments[i]);
 		else
 		{
@@ -109,7 +109,8 @@ void WordSizeTransform::operator()(Block& _block)
 					if (BuiltinFunction const* f = m_inputDialect.builtin(std::get<FunctionCall>(*varDecl.value).functionName.name))
 						if (f->name == "datasize"_yulstring || f->name == "dataoffset"_yulstring)
 						{
-							yulAssert(f->literalArguments && f->literalArguments.value()[0], "");
+							yulAssert(f->literalArguments.size() == 1, "");
+							yulAssert(f->literalArguments.at(0) == LiteralKind::String, "");
 							yulAssert(varDecl.variables.size() == 1, "");
 							auto newLhs = generateU64IdentifierNames(varDecl.variables[0].name);
 							vector<Statement> ret;
@@ -169,7 +170,8 @@ void WordSizeTransform::operator()(Block& _block)
 					if (BuiltinFunction const* f = m_inputDialect.builtin(std::get<FunctionCall>(*assignment.value).functionName.name))
 						if (f->name == "datasize"_yulstring || f->name == "dataoffset"_yulstring)
 						{
-							yulAssert(f->literalArguments && f->literalArguments.value()[0], "");
+							yulAssert(f->literalArguments.size() == 1, "");
+							yulAssert(f->literalArguments[0] == LiteralKind::String, "");
 							yulAssert(assignment.variableNames.size() == 1, "");
 							auto newLhs = generateU64IdentifierNames(assignment.variableNames[0].name);
 							vector<Statement> ret;

libyul/optimiser/CommonSubexpressionEliminator.cpp

@@ -67,7 +67,7 @@ void CommonSubexpressionEliminator::visit(Expression& _e)
 				// We should not modify function arguments that have to be literals
 				// Note that replacing the function call entirely is fine,
 				// if the function call is movable.
-				if (!builtin->literalArguments || !builtin->literalArguments.value()[i - 1])
+				if (!builtin->literalArgument(i - 1))
 					visit(funCall.arguments[i - 1]);
 
 			descend = false;

libyul/optimiser/ExpressionSplitter.cpp

@@ -46,14 +46,10 @@ void ExpressionSplitter::run(OptimiserStepContext& _context, Block& _ast)
 
 void ExpressionSplitter::operator()(FunctionCall& _funCall)
 {
-	vector<bool> const* literalArgs = nullptr;
-
-	if (BuiltinFunction const* builtin = m_dialect.builtin(_funCall.functionName.name))
-		if (builtin->literalArguments)
-			literalArgs = &builtin->literalArguments.value();
+	BuiltinFunction const* builtin = m_dialect.builtin(_funCall.functionName.name);
 
 	for (size_t i = _funCall.arguments.size(); i > 0; i--)
-		if (!literalArgs || !(*literalArgs)[i - 1])
+		if (!builtin || !builtin->literalArgument(i - 1))
 			outlineExpression(_funCall.arguments[i - 1]);
 }
 

scripts/error_codes.py

@@ -171,12 +171,13 @@ def examine_id_coverage(top_dir, source_id_to_file_names, new_ids_only=False):
     test_sub_dirs = [
         path.join("test", "libsolidity", "errorRecoveryTests"),
         path.join("test", "libsolidity", "smtCheckerTests"),
-        path.join("test", "libsolidity", "syntaxTests")
+        path.join("test", "libsolidity", "syntaxTests"),
+        path.join("test", "libyul", "yulSyntaxTests")
     ]
     test_file_names = find_files(
         top_dir,
         test_sub_dirs,
-        [".sol"]
+        [".sol", ".yul"]
     )
     source_ids = source_id_to_file_names.keys()
     test_ids = find_ids_in_test_files(test_file_names)

test/cmdlineTests/standard_yul_object_invalid_sub/output.json

@@ -1,4 +1,4 @@
-{"errors":[{"component":"general","formattedMessage":"A:1:40: TypeError: Unknown data object \"NamedObject.\".
+{"errors":[{"component":"general","formattedMessage":"A:1:51: TypeError: Unknown data object \"NamedObject.\".
 object \"NamedObject\" { code { let x := dataoffset(\"NamedObject.\") sstore(add(x, 0), 0) } object \"OtherObject\" { code { revert(0, 0) } } }
-                                       ^--------^
-","message":"Unknown data object \"NamedObject.\".","severity":"error","sourceLocation":{"end":49,"file":"A","start":39},"type":"TypeError"}]}
+                                                  ^------------^
+","message":"Unknown data object \"NamedObject.\".","severity":"error","sourceLocation":{"end":64,"file":"A","start":50},"type":"TypeError"}]}

test/libyul/ObjectParser.cpp

@@ -258,6 +258,16 @@ BOOST_AUTO_TEST_CASE(arg_to_dataoffset_must_be_literal)
 	CHECK_ERROR(code, TypeError, "Function expects direct literals as arguments.");
 }
 
+BOOST_AUTO_TEST_CASE(arg_to_dataoffset_must_be_string_literal)
+{
+	string code = R"(
+		object "outer" {
+			code { let y := dataoffset(0) }
+		}
+	)";
+	CHECK_ERROR(code, TypeError, "Function expects string literal.");
+}
+
 BOOST_AUTO_TEST_CASE(arg_to_datasize_must_be_literal)
 {
 	string code = R"(
@@ -268,6 +278,16 @@ BOOST_AUTO_TEST_CASE(arg_to_datasize_must_be_literal)
 	CHECK_ERROR(code, TypeError, "Function expects direct literals as arguments.");
 }
 
+BOOST_AUTO_TEST_CASE(arg_to_datasize_must_be_string_literal)
+{
+	string code = R"(
+		object "outer" {
+			code { let y := datasize(0) }
+		}
+	)";
+	CHECK_ERROR(code, TypeError, "Function expects string literal.");
+}
+
 BOOST_AUTO_TEST_CASE(args_to_datacopy_are_arbitrary)
 {
 	string code = R"(

test/libyul/yulSyntaxTests/datacopy.yul

@@ -0,0 +1,12 @@
+{
+    datacopy(0, 1, 2)
+    datasize("")
+    datasize(0) // This should not be valid.
+    dataoffset("")
+    dataoffset(0) // This should not be valid.
+}
+// ----
+// TypeError 3517: (37-39): Unknown data object "".
+// TypeError 5859: (54-55): Function expects string literal.
+// TypeError 3517: (101-103): Unknown data object "".
+// TypeError 5859: (120-121): Function expects string literal.

test/libyul/yulSyntaxTests/linkersymbol_bad_literal.yul

@@ -0,0 +1,11 @@
+{
+    pop(linkersymbol(0))
+    pop(linkersymbol(true))
+    pop(linkersymbol(false))
+}
+// ====
+// dialect: evm
+// ----
+// TypeError 5859: (23-24): Function expects string literal.
+// TypeError 5859: (48-52): Function expects string literal.
+// TypeError 5859: (76-81): Function expects string literal.

test/libyul/yulSyntaxTests/loadimmutable_bad_literal.yul

@@ -0,0 +1,11 @@
+{
+    pop(loadimmutable(0))
+    pop(loadimmutable(true))
+    pop(loadimmutable(false))
+}
+// ====
+// dialect: evm
+// ----
+// TypeError 5859: (24-25): Function expects string literal.
+// TypeError 5859: (50-54): Function expects string literal.
+// TypeError 5859: (79-84): Function expects string literal.

test/libyul/yulSyntaxTests/setimmutable_bad_literal.yul

@@ -0,0 +1,11 @@
+{
+    setimmutable(0, 0x1234567890123456789012345678901234567890)
+    setimmutable(true, 0x1234567890123456789012345678901234567890)
+    setimmutable(false, 0x1234567890123456789012345678901234567890)
+}
+// ====
+// dialect: evm
+// ----
+// TypeError 5859: (19-20): Function expects string literal.
+// TypeError 5859: (83-87): Function expects string literal.
+// TypeError 5859: (150-155): Function expects string literal.