Minor refactoring

test/tools/ossfuzz/protoToAbiV2.cpp

@@ -4,7 +4,7 @@ using namespace std;
 using namespace dev;
 using namespace dev::test::abiv2fuzzer;
 
-string ProtoConverter::appendVarDeclToOutput(
+string ProtoConverter::getVarDecl(
 	string const& _type,
 	string const& _varName,
 	string const& _qualifier
@@ -114,9 +114,7 @@ template <typename T>
 pair<string, string> ProtoConverter::processType(T const& _type, bool _isValueType)
 {
 	ostringstream local, global;
-	auto varNames = newVarNames(getNextVarCounter());
-	string varName = varNames.first;
-	string paramName = varNames.second;
+	auto [varName, paramName] = newVarNames(getNextVarCounter());
 	string location{};
 	if (!m_isStateVar && !_isValueType)
 		location = "memory";
@@ -160,9 +158,9 @@ pair<string, string> ProtoConverter::varDecl(
 
 	// variable declaration
 	if (m_isStateVar)
-		global << appendVarDeclToOutput(typeStr, _varName, _location);
+		global << getVarDecl(typeStr, _varName, _location);
 	else
-		local << appendVarDeclToOutput(typeStr, _varName, _location);
+		local << getVarDecl(typeStr, _varName, _location);
 
 	// Add typed params for calling public and external functions with said type
 	appendTypedParams(
@@ -312,7 +310,7 @@ std::string ProtoConverter::typedParametersAsString(CalleeType _calleeType)
 	}
 }
 
-/// Test function to be called externally.
+// Test function to be called externally.
 string ProtoConverter::visit(TestFunction const& _x, string const& _storageVarDefs)
 {
 	// TODO: Support more than one but less than N local variables
@@ -491,10 +489,10 @@ pragma experimental ABIEncoderV2;)";
 	string testFunction = visit(_x.testfunction(), storageVarDefs);
 	/* Structure of contract body
 	 * - Storage variable declarations
-	 * - Struct type declarations
+	 * - Struct definitions
 	 * - Test function
-     *     - Storage variable definitions
-	 *     - Local variable definitions
+	 *     - Storage variable assignments
+	 *     - Local variable definitions and assignments
 	 *     - Test code proper (calls public and external functions)
 	 * - Helper functions
 	 */
@@ -557,8 +555,13 @@ string TypeVisitor::visit(ArrayType const& _type)
 	                     string("]") :
 	                     string("[]");
 	m_baseType += arrayBraces;
-	if (!_type.is_static())
-		m_isLastDynParamRightPadded = true;
+
+	// If we don't know yet if the array will be dynamically encoded,
+	// check again. If we already know that it will be, there's no
+	// need to do anything.
+	if (!m_isLastDynParamRightPadded)
+		m_isLastDynParamRightPadded = DynParamVisitor().visit(_type);
+
 	return baseType + arrayBraces;
 }
 

test/tools/ossfuzz/protoToAbiV2.h

@@ -97,16 +97,18 @@
  */
 
 
-namespace dev {
-namespace test {
-namespace abiv2fuzzer {
-
+namespace dev
+{
+namespace test
+{
+namespace abiv2fuzzer
+{
 /// Converts a protobuf input into a Solidity program that tests
 /// abi coding.
 class ProtoConverter
 {
 public:
-	ProtoConverter() :
+	ProtoConverter():
 		m_isStateVar(true),
 		m_counter(0),
 		m_varCounter(0),
@@ -125,33 +127,31 @@ private:
 		ADD,
 		SKIP
 	};
+	/// Enum of possible function types that decode abi
+	/// encoded parameters.
 	enum class CalleeType
 	{
 		PUBLIC,
 		EXTERNAL
 	};
-	std::pair<std::string, std::string> visit(VarDecl const&);
-	std::string visit(TestFunction const&, std::string const&);
+
+	/// Visitors for various Protobuf types
+	/// Visit top-level contract specification
 	void visit(Contract const&);
 
-	template <typename T>
-	std::pair<std::string, std::string> processType(T const& _type, bool _isValueType);
+	/// Convert test function specification into Solidity test
+	/// function
+	/// @param _testSpec: Protobuf test function specification
+	/// @param _storageDefs: String containing Solidity assignment
+	/// statements to be placed inside the scope of the test function.
+	std::string visit(TestFunction const& _testSpec, std::string const& _storageDefs);
 
-	template <typename T>
-	std::pair<std::string, std::string> assignChecker(
-		std::string const& _varName,
-		std::string const& _paramName,
-		T _type
-	);
-
-	template <typename T>
-	std::pair<std::string, std::string> varDecl(
-		std::string const& _varName,
-		std::string const& _paramName,
-		T _type,
-		bool _isValueType,
-		std::string const& _location
-	);
+	/// Visitors for the remaining protobuf types. They convert
+	/// the input protobuf specification type into Solidity code.
+	/// @return A pair of strings, first of which contains Solidity
+	/// code to be placed inside contract scope, second of which contains
+	/// Solidity code to be placed inside test function scope.
+	std::pair<std::string, std::string> visit(VarDecl const&);
 	std::pair<std::string, std::string> visit(Type const&);
 	std::pair<std::string, std::string> visit(ValueType const&);
 	std::pair<std::string, std::string> visit(NonValueType const&);
@@ -163,7 +163,59 @@ private:
 	std::pair<std::string, std::string> visit(ArrayType const&);
 	std::pair<std::string, std::string> visit(StructType const&);
 
-	// Utility functions
+	/// Convert a protobuf type @a _T into Solidity code to be placed
+	/// inside contract and test function scopes.
+	/// @param: _type (of parameterized type protobuf type T) is the type
+	/// of protobuf input to be converted.
+	/// @param: _isValueType is true if _type is a Solidity value type e.g., uint
+	/// and false otherwise e.g., string
+	/// @return: A pair of strings, first of which contains Solidity
+	/// code to be placed inside contract scope, second of which contains
+	/// Solidity code to be placed inside test function scope.
+	template <typename T>
+	std::pair<std::string, std::string> processType(T const& _type, bool _isValueType);
+
+	/// Convert a protobuf type @a _T into Solidity variable assignment and check
+	/// statements to be placed inside contract and test function scopes.
+	/// @param: _varName is the name of the Solidity variable
+	/// @param: _paramName is the name of the Solidity parameter that is passed
+	/// to the test function
+	/// @param: _type (of parameterized type protobuf type T) is the type
+	/// of protobuf input to be converted.
+	/// @return: A pair of strings, first of which contains Solidity
+	/// statements to be placed inside contract scope, second of which contains
+	/// Solidity statements to be placed inside test function scope.
+	template <typename T>
+	std::pair<std::string, std::string> assignChecker(
+		std::string const& _varName,
+		std::string const& _paramName,
+		T _type
+	);
+
+	/// Convert a protobuf type @a _T into Solidity variable declaration statement.
+	/// @param: _varName is the name of the Solidity variable
+	/// @param: _paramName is the name of the Solidity parameter that is passed
+	/// to the test function
+	/// @param: _type (of parameterized type protobuf type T) is the type
+	/// of protobuf input to be converted.
+	/// @param: _isValueType is a boolean that is true if _type is a
+	/// Solidity value type e.g., uint and false otherwise e.g., string
+	/// @param: _location is the Solidity location qualifier string to
+	/// be used inside variable declaration statements
+	/// @return: A pair of strings, first of which contains Solidity
+	/// variable declaration statement to be placed inside contract scope,
+	/// second of which contains Solidity variable declaration statement
+	/// to be placed inside test function scope.
+	template <typename T>
+	std::pair<std::string, std::string> varDecl(
+		std::string const& _varName,
+		std::string const& _paramName,
+		T _type,
+		bool _isValueType,
+		std::string const& _location
+	);
+
+	/// Appends a function parameter to the function parameter stream.
 	void appendTypedParams(
 		CalleeType _calleeType,
 		bool _isValueType,
@@ -172,6 +224,8 @@ private:
 		Delimiter _delimiter
 	);
 
+	/// Appends a function parameter to the public test function's
+	/// parameter stream.
 	void appendTypedParamsPublic(
 		bool _isValueType,
 		std::string const& _typeString,
@@ -179,6 +233,8 @@ private:
 		Delimiter _delimiter = Delimiter::ADD
 	);
 
+	/// Appends a function parameter to the external test function's
+	/// parameter stream.
 	void appendTypedParamsExternal(
 		bool _isValueType,
 		std::string const& _typeString,
@@ -186,32 +242,41 @@ private:
 		Delimiter _delimiter = Delimiter::ADD
 	);
 
-	std::string appendVarDeclToOutput(
+	/// Returns a Solidity variable declaration statement
+	/// @param _type: string containing Solidity type of the
+	/// variable to be declared.
+	/// @param _varName: string containing Solidity variable
+	/// name
+	/// @param _qualifier: string containing location where
+	/// the variable will be placed
+	std::string getVarDecl(
 		std::string const& _type,
 		std::string const& _varName,
 		std::string const& _qualifier
 	);
 
+	/// Return checks that are encoded as Solidity if statements
+	/// as string
 	std::string equalityChecksAsString();
 
+	/// Return comma separated typed function parameters as string
 	std::string typedParametersAsString(CalleeType _calleeType);
 
+	/// Return Solidity helper functions as string
 	std::string helperFunctions();
 
+	/// Return top-level test code as string
 	std::string testCode(unsigned _invalidLength);
 
-	std::pair<std::string, unsigned> structDecl(StructType const& _type);
-
-	// Function definitions
-	// m_varCounter is used to derive declared and parameterized variable names
+	/// Return the next variable count that is used for
+	/// variable naming.
 	unsigned getNextVarCounter()
 	{
 		return m_varCounter++;
 	}
 
-	// Accepts an unsigned counter and returns a pair of strings
-	// First string is declared name (s_varNamePrefix<varcounter_value>)
-	// Second string is parameterized name (s_paramPrefix<varcounter_value>)
+	/// Return a pair of names for Solidity variable and the same variable when
+	/// passed as a function parameter.
 	static std::pair<std::string, std::string> newVarNames(unsigned _varCounter)
 	{
 		return std::make_pair(
@@ -220,11 +285,14 @@ private:
 		);
 	}
 
+	/// Checks if the last dynamically encoded Solidity type is right
+	/// padded, returning true if it is and false otherwise.
 	bool isLastDynParamRightPadded()
 	{
 		return m_isLastDynParamRightPadded;
 	}
 
+	/// Convert delimter to a comma or null string.
 	static std::string delimiterToString(Delimiter _delimiter);
 
 	/// Contains the test program
@@ -290,10 +358,7 @@ public:
 	};
 
 	/// Prefixes for declared and parameterized variable names
-	static auto constexpr s_varNamePrefix = "x_";
-	static auto constexpr s_paramNamePrefix = "c_";
 	static auto constexpr s_structNamePrefix = "S";
-	static auto constexpr s_structFieldPrefix = "m";
 
 	// Static function definitions
 	static bool isValueType(DataType _dataType)
@@ -386,7 +451,8 @@ public:
 
 	static std::string bytesArrayTypeAsString(DynamicByteArrayType const& _x)
 	{
-		switch (_x.type()) {
+		switch (_x.type())
+		{
 		case DynamicByteArrayType::BYTES:
 			return "bytes";
 		case DynamicByteArrayType::STRING:
@@ -699,6 +765,12 @@ public:
 
 	bool visit(ArrayType const& _type) override
 	{
+		// Mark array type as invalid in one of the following is true
+		//  - contains more than s_maxArrayDimensions dimensions
+		//  - contains an invalid base type, which happens in the
+		//  following cases
+		//    - array base type is invalid
+		//    - array base type is empty
 		m_arrayDimensions++;
 		if (m_arrayDimensions > s_maxArrayDimensions)
 			return false;
@@ -707,6 +779,9 @@ public:
 
 	bool visit(StructType const& _type) override
 	{
+		// A struct is marked invalid only if all of its fields
+		// are invalid. This is done to prevent an empty struct
+		// being defined (which is a Solidity error).
 		for (auto const& t: _type.t())
 			if (visit(t))
 				return true;