Adjustments to Solidity compiler code for Function Hash

7 files changed, 37 insertions, 40 deletions

diff --git a/AST.cpp b/AST.cpp
index fb71e900..fd4cc57c 100644
--- a/AST.cpp
+++ b/AST.cpp
@@ -59,7 +59,9 @@ map<FixedHash<4>, FunctionDefinition const*> ContractDefinition::getInterfaceFun
 		if (f->isPublic() && f->getName() != getName())
 		{
 			FixedHash<4> hash(dev::sha3(f->getCanonicalSignature()));
-			exportedFunctions[hash] = f.get();
+			auto res = exportedFunctions.insert(std::make_pair(hash,f.get()));
+			if (!res.second)
+				solAssert(false, "Hash collision at Function Definition Hash calculation");
 		}
 
 	return exportedFunctions;
diff --git a/Compiler.cpp b/Compiler.cpp
index d49807be..4e5b7f55 100644
--- a/Compiler.cpp
+++ b/Compiler.cpp
@@ -100,7 +100,7 @@ void Compiler::appendConstructorCall(FunctionDefinition const& _constructor)
 	{
 		m_context << u256(argumentSize);
 		m_context.appendProgramSize();
-		m_context << u256(g_functionIdentifierOffset); // copy it to byte four as expected for ABI calls
+		m_context << u256(CompilerUtils::dataStartOffset); // copy it to byte four as expected for ABI calls
 		m_context << eth::Instruction::CODECOPY;
 		appendCalldataUnpacker(_constructor, true);
 	}
@@ -119,30 +119,26 @@ set<FunctionDefinition const*> Compiler::getFunctionsNeededByConstructor(Functio
 void Compiler::appendFunctionSelector(ContractDefinition const& _contract)
 {
 	map<FixedHash<4>, FunctionDefinition const*> interfaceFunctions = _contract.getInterfaceFunctions();
-	vector<eth::AssemblyItem> callDataUnpackerEntryPoints;
-
-	if (interfaceFunctions.size() > 4294967295) // 2 ** 32
-		BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("More than 4294967295 public functions for contract."));
+	map<FixedHash<4>, const eth::AssemblyItem> callDataUnpackerEntryPoints;
 
 	// retrieve the function signature hash from the calldata
-	m_context << u256(1) << u256(0) << (u256(1) << 224) // some constants
-			  << eth::dupInstruction(2) << eth::Instruction::CALLDATALOAD
-			  << eth::Instruction::DIV;
+	m_context << u256(1) << u256(0);
+	CompilerUtils(m_context).loadFromMemory(0, 4, false, true);
 
 	// stack now is: 1 0 <funhash>
-	for (auto it = interfaceFunctions.cbegin(); it != interfaceFunctions.cend(); ++it)
+	// for (auto it = interfaceFunctions.cbegin(); it != interfaceFunctions.cend(); ++it)
+	for (auto const& it: interfaceFunctions)
 	{
-		callDataUnpackerEntryPoints.push_back(m_context.newTag());
-		m_context << eth::dupInstruction(1) << u256(FixedHash<4>::Arith(it->first)) << eth::Instruction::EQ;
-		m_context.appendConditionalJumpTo(callDataUnpackerEntryPoints.back());
+		callDataUnpackerEntryPoints.insert(std::make_pair(it.first, m_context.newTag()));
+		m_context << eth::dupInstruction(1) << u256(FixedHash<4>::Arith(it.first)) << eth::Instruction::EQ;
+		m_context.appendConditionalJumpTo(callDataUnpackerEntryPoints.at(it.first));
 	}
 	m_context << eth::Instruction::STOP; // function not found
 
-	unsigned funid = 0;
-	for (auto it = interfaceFunctions.cbegin(); it != interfaceFunctions.cend(); ++it, ++funid)
+	for (auto const& it: interfaceFunctions)
 	{
-		FunctionDefinition const& function = *it->second;
-		m_context << callDataUnpackerEntryPoints[funid];
+		FunctionDefinition const& function = *it.second;
+		m_context << callDataUnpackerEntryPoints.at(it.first);
 		eth::AssemblyItem returnTag = m_context.pushNewTag();
 		appendCalldataUnpacker(function);
 		m_context.appendJumpTo(m_context.getFunctionEntryLabel(function));
@@ -154,7 +150,7 @@ void Compiler::appendFunctionSelector(ContractDefinition const& _contract)
 unsigned Compiler::appendCalldataUnpacker(FunctionDefinition const& _function, bool _fromMemory)
 {
 	// We do not check the calldata size, everything is zero-padded.
-	unsigned dataOffset = g_functionIdentifierOffset; // the 4 bytes of the function hash signature
+	unsigned dataOffset = CompilerUtils::dataStartOffset; // the 4 bytes of the function hash signature
 	//@todo this can be done more efficiently, saving some CALLDATALOAD calls
 	for (ASTPointer<VariableDeclaration> const& var: _function.getParameters())
 	{
diff --git a/CompilerUtils.cpp b/CompilerUtils.cpp
index 680e9190..a5254b42 100644
--- a/CompilerUtils.cpp
+++ b/CompilerUtils.cpp
@@ -31,6 +31,8 @@ namespace dev
 namespace solidity
 {
 
+const unsigned int CompilerUtils::dataStartOffset = 4;
+
 void CompilerUtils::loadFromMemory(unsigned _offset, unsigned _bytes, bool _leftAligned, bool _fromCalldata)
 {
 	if (_bytes == 0)
diff --git a/CompilerUtils.h b/CompilerUtils.h
index bffd6f49..6bd8d315 100644
--- a/CompilerUtils.h
+++ b/CompilerUtils.h
@@ -30,9 +30,6 @@ namespace solidity {
 
 class Type; // forward
 
-/// The size in bytes of the function (hash) identifier
-static const unsigned int g_functionIdentifierOffset = 4;
-
 class CompilerUtils
 {
 public:
@@ -61,10 +58,14 @@ public:
 	static unsigned getSizeOnStack(std::vector<T> const& _variables);
 	static unsigned getSizeOnStack(std::vector<std::shared_ptr<Type const>> const& _variableTypes);
 
+	/// Bytes we need to the start of call data.
+	///  - The size in bytes of the function (hash) identifier.
+	static const unsigned int dataStartOffset;
 private:
 	CompilerContext& m_context;
 };
 
+
 template <class T>
 unsigned CompilerUtils::getSizeOnStack(std::vector<T> const& _variables)
 {
diff --git a/ExpressionCompiler.cpp b/ExpressionCompiler.cpp
index 1b10c854..44a16d15 100644
--- a/ExpressionCompiler.cpp
+++ b/ExpressionCompiler.cpp
@@ -592,9 +592,9 @@ void ExpressionCompiler::appendExternalFunctionCall(FunctionType const& _functio
 
 	_options.obtainAddress();
 	if (!_options.bare)
-		CompilerUtils(m_context).storeInMemory(0, g_functionIdentifierOffset);
+		CompilerUtils(m_context).storeInMemory(0, CompilerUtils::dataStartOffset);
 
-	unsigned dataOffset = _options.bare ? 0 : g_functionIdentifierOffset; // reserve 4 bytes for the function's hash identifier
+	unsigned dataOffset = _options.bare ? 0 : CompilerUtils::dataStartOffset; // reserve 4 bytes for the function's hash identifier
 	for (unsigned i = 0; i < _arguments.size(); ++i)
 	{
 		_arguments[i]->accept(*this);
diff --git a/InterfaceHandler.cpp b/InterfaceHandler.cpp
index 0843c363..45395127 100644
--- a/InterfaceHandler.cpp
+++ b/InterfaceHandler.cpp
@@ -35,9 +35,8 @@ std::unique_ptr<std::string> InterfaceHandler::getDocumentation(ContractDefiniti
 std::unique_ptr<std::string> InterfaceHandler::getABIInterface(ContractDefinition const& _contractDef)
 {
 	Json::Value methods(Json::arrayValue);
-	auto interfaceFunctions = _contractDef.getInterfaceFunctions();
 
-	for (auto it = interfaceFunctions.cbegin(); it != interfaceFunctions.cend(); ++it)
+	for (auto const& it: _contractDef.getInterfaceFunctions())
 	{
 		Json::Value method;
 		Json::Value inputs(Json::arrayValue);
@@ -56,10 +55,10 @@ std::unique_ptr<std::string> InterfaceHandler::getABIInterface(ContractDefinitio
 			return params;
 		};
 
-		method["name"] = it->second->getName();
-		method["constant"] = it->second->isDeclaredConst();
-		method["inputs"] = populateParameters(it->second->getParameters());
-		method["outputs"] = populateParameters(it->second->getReturnParameters());
+		method["name"] = it.second->getName();
+		method["constant"] = it.second->isDeclaredConst();
+		method["inputs"] = populateParameters(it.second->getParameters());
+		method["outputs"] = populateParameters(it.second->getReturnParameters());
 		methods.append(method);
 	}
 	return std::unique_ptr<std::string>(new std::string(m_writer.write(methods)));
@@ -69,12 +68,11 @@ std::unique_ptr<std::string> InterfaceHandler::getUserDocumentation(ContractDefi
 {
 	Json::Value doc;
 	Json::Value methods(Json::objectValue);
-	auto interfaceFunctions = _contractDef.getInterfaceFunctions();
 
-	for (auto it = interfaceFunctions.cbegin(); it != interfaceFunctions.cend(); ++it)
+	for (auto const& it: _contractDef.getInterfaceFunctions())
 	{
 		Json::Value user;
-		auto strPtr = it->second->getDocumentation();
+		auto strPtr = it.second->getDocumentation();
 		if (strPtr)
 		{
 			resetUser();
@@ -82,7 +80,7 @@ std::unique_ptr<std::string> InterfaceHandler::getUserDocumentation(ContractDefi
 			if (!m_notice.empty())
 			{// since @notice is the only user tag if missing function should not appear
 				user["notice"] = Json::Value(m_notice);
-				methods[it->second->getName()] = user;
+				methods[it.second->getName()] = user;
 			}
 		}
 	}
@@ -112,11 +110,10 @@ std::unique_ptr<std::string> InterfaceHandler::getDevDocumentation(ContractDefin
 			doc["title"] = m_title;
 	}
 
-	auto interfaceFunctions = _contractDef.getInterfaceFunctions();
-	for (auto it = interfaceFunctions.cbegin(); it != interfaceFunctions.cend(); ++it)
+	for (auto const& it: _contractDef.getInterfaceFunctions())
 	{
 		Json::Value method;
-		auto strPtr = it->second->getDocumentation();
+		auto strPtr = it.second->getDocumentation();
 		if (strPtr)
 		{
 			resetDev();
@@ -139,7 +136,7 @@ std::unique_ptr<std::string> InterfaceHandler::getDevDocumentation(ContractDefin
 				method["return"] = m_return;
 
 			if (!method.empty()) // add the function, only if we have any documentation to add
-				methods[it->second->getName()] = method;
+				methods[it.second->getName()] = method;
 		}
 	}
 	doc["methods"] = methods;
diff --git a/Types.cpp b/Types.cpp
index 494bbd26..01fa7734 100644
--- a/Types.cpp
+++ b/Types.cpp
@@ -333,10 +333,9 @@ MemberList const& ContractType::getMembers() const
 	// We need to lazy-initialize it because of recursive references.
 	if (!m_members)
 	{
-		auto interfaceFunctions = m_contract.getInterfaceFunctions();
 		map<string, shared_ptr<Type const>> members;
-		for (auto it = interfaceFunctions.cbegin(); it != interfaceFunctions.cend(); ++it)
-			members[it->second->getName()] = make_shared<FunctionType>(*it->second, false);
+		for (auto const& it: m_contract.getInterfaceFunctions())
+			members[it.second->getName()] = make_shared<FunctionType>(*it.second, false);
 		m_members.reset(new MemberList(members));
 	}
 	return *m_members;