Merge pull request #9187 from a3d4/fix-storagesizeestimate

Fixed recursive check in structureSizeEstimate

libsolidity/analysis/StaticAnalyzer.cpp

@@ -158,7 +158,8 @@ bool StaticAnalyzer::visit(VariableDeclaration const& _variable)
 	else if (_variable.isStateVariable())
 	{
 		set<StructDefinition const*> structsSeen;
-		if (structureSizeEstimate(*_variable.type(), structsSeen) >= bigint(1) << 64)
+		TypeSet oversizedSubTypes;
+		if (structureSizeEstimate(*_variable.type(), structsSeen, oversizedSubTypes) >= bigint(1) << 64)
 			m_errorReporter.warning(
 				3408_error,
 				_variable.location(),
@@ -166,6 +167,14 @@ bool StaticAnalyzer::visit(VariableDeclaration const& _variable)
 				"Either use mappings or dynamic arrays and allow their size to be increased only "
 				"in small quantities per transaction."
 			);
+		for (Type const* type: oversizedSubTypes)
+			m_errorReporter.warning(
+				7325_error,
+				_variable.location(),
+				"Type " + type->canonicalName() + " has large size and thus makes collisions likely. "
+				"Either use mappings or dynamic arrays and allow their size to be increased only "
+				"in small quantities per transaction."
+			);
 	}
 	return true;
 }
@@ -339,30 +348,43 @@ bool StaticAnalyzer::visit(FunctionCall const& _functionCall)
 	return true;
 }
 
-bigint StaticAnalyzer::structureSizeEstimate(Type const& _type, set<StructDefinition const*>& _structsSeen)
+bigint StaticAnalyzer::structureSizeEstimate(
+	Type const& _type,
+	set<StructDefinition const*>& _structsSeen,
+	TypeSet& _oversizedSubTypes
+)
 {
 	switch (_type.category())
 	{
 	case Type::Category::Array:
 	{
 		auto const& t = dynamic_cast<ArrayType const&>(_type);
-		return structureSizeEstimate(*t.baseType(), _structsSeen) * (t.isDynamicallySized() ? 1 : t.length());
+		bigint baseTypeSize = structureSizeEstimate(*t.baseType(), _structsSeen, _oversizedSubTypes);
+		if (baseTypeSize >= bigint(1) << 64)
+			_oversizedSubTypes.insert(t.baseType());
+		if (!t.isDynamicallySized())
+			return structureSizeEstimate(*t.baseType(), _structsSeen, _oversizedSubTypes) * t.length();
+		break;
 	}
 	case Type::Category::Struct:
 	{
 		auto const& t = dynamic_cast<StructType const&>(_type);
 		bigint size = 1;
-		if (!_structsSeen.count(&t.structDefinition()))
-		{
-			_structsSeen.insert(&t.structDefinition());
-			for (auto const& m: t.members(nullptr))
-				size += structureSizeEstimate(*m.type, _structsSeen);
-		}
+		if (_structsSeen.count(&t.structDefinition()))
+			return size;
+		_structsSeen.insert(&t.structDefinition());
+		for (auto const& m: t.members(nullptr))
+			size += structureSizeEstimate(*m.type, _structsSeen, _oversizedSubTypes);
+		_structsSeen.erase(&t.structDefinition());
 		return size;
 	}
 	case Type::Category::Mapping:
 	{
-		return structureSizeEstimate(*dynamic_cast<MappingType const&>(_type).valueType(), _structsSeen);
+		auto const* valueType = dynamic_cast<MappingType const&>(_type).valueType();
+		bigint valueTypeSize = structureSizeEstimate(*valueType, _structsSeen, _oversizedSubTypes);
+		if (valueTypeSize >= bigint(1) << 64)
+			_oversizedSubTypes.insert(valueType);
+		break;
 	}
 	default:
 		break;

libsolidity/analysis/StaticAnalyzer.h

@@ -73,8 +73,22 @@ private:
 	bool visit(BinaryOperation const& _operation) override;
 	bool visit(FunctionCall const& _functionCall) override;
 
+	struct TypeComp
+	{
+		bool operator()(Type const* lhs, Type const* rhs) const
+		{
+			solAssert(lhs && rhs, "");
+			return lhs->richIdentifier() < rhs->richIdentifier();
+		}
+	};
+	using TypeSet = std::set<Type const*, TypeComp>;
+
 	/// @returns the size of this type in storage, including all sub-types.
-	static bigint structureSizeEstimate(Type const& _type, std::set<StructDefinition const*>& _structsSeen);
+	static bigint structureSizeEstimate(
+		Type const& _type,
+		std::set<StructDefinition const*>& _structsSeen,
+		TypeSet& _oversizedSubTypes
+	);
 
 	langutil::ErrorReporter& m_errorReporter;
 

test/libsolidity/syntaxTests/largeTypes/large_storage_array_mapping.sol

@@ -0,0 +1,5 @@
+contract C {
+    mapping(uint => uint[2**100]) x;
+}
+// ----
+// Warning 7325: (17-48): Type uint256[1267650600228229401496703205376] has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.

test/libsolidity/syntaxTests/largeTypes/large_storage_arrays_combined.sol

@@ -0,0 +1,4 @@
+contract C {
+    uint[200][200][2**30][][2**30] x;
+}
+// ----

test/libsolidity/syntaxTests/largeTypes/large_storage_structs.sol

@@ -0,0 +1,65 @@
+contract C {
+    struct P { uint256[2**63] x; }
+
+    struct S0 {
+        P[2**62] x;
+        P y;
+    }
+    S0 s0;
+
+    struct S1 {
+        P x;
+        P[2**62] y;
+    }
+    S1 s1;
+
+    struct S2 {
+        mapping(uint => P[2**62]) x;
+        mapping(uint => P[2**62]) y;
+        mapping(uint => S2) z;
+    }
+    S2 s2;
+
+    struct Q0
+    {
+        uint[1][][2**65] x;
+        uint[2**65][][1] y;
+        uint[][2**65] z;
+        uint[2**65][] t;
+    }
+    Q0 q0;
+
+    struct Q1
+    {
+        uint[1][][2**65] x;
+    }
+    Q1 q1;
+
+    struct Q2
+    {
+        uint[2**65][][1] y;
+    }
+    Q2 q2;
+
+    struct Q3
+    {
+        uint[][2**65] z;
+    }
+    Q3 q3;
+
+    struct Q4
+    {
+        uint[2**65][] t;
+    }
+    Q4 q4;
+}
+// ----
+// Warning 3408: (108-113): Variable covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
+// Warning 3408: (175-180): Variable covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
+// Warning 7325: (314-319): Type C.P[4611686018427387904] has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
+// Warning 3408: (458-463): Variable covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
+// Warning 7325: (458-463): Type uint256[36893488147419103232] has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
+// Warning 3408: (524-529): Variable covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
+// Warning 7325: (590-595): Type uint256[36893488147419103232] has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
+// Warning 3408: (653-658): Variable covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
+// Warning 7325: (716-721): Type uint256[36893488147419103232] has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.

test/libsolidity/syntaxTests/nameAndTypeResolution/508_large_storage_arrays_combined.sol

@@ -1,5 +0,0 @@
-contract C {
-    uint[200][200][2**30][][2**30] x;
-}
-// ----
-// Warning 3408: (17-49): Variable covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.

test/libsolidity/syntaxTests/nameAndTypeResolution/510_large_storage_array_mapping.sol

@@ -1,5 +0,0 @@
-contract C {
-    mapping(uint => uint[2**100]) x;
-}
-// ----
-// Warning 3408: (17-48): Variable covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.