Merge pull request #2703 from ethereum/warnAboutLargeStorageArrays

Warn about large storage structures.
This commit is contained in:
chriseth 2017-08-14 11:37:02 +02:00 committed by GitHub
commit 4d9790b6d5
4 changed files with 97 additions and 0 deletions

View File

@ -3,6 +3,7 @@
Features:
* Parser: Display previous visibility specifier in error if multiple are found.
* Syntax Checker: Support ``pragma experimental <feature>;`` to turn on experimental features.
* Static Analyzer: Warn about large storage structures.
* Metadata: Store experimental flag in metadata CBOR.
* Type Checker: More detailed error message for invalid overrides.

View File

@ -92,6 +92,17 @@ bool StaticAnalyzer::visit(VariableDeclaration const& _variable)
// This is not a no-op, the entry might pre-exist.
m_localVarUseCount[&_variable] += 0;
}
else if (_variable.isStateVariable())
{
set<StructDefinition const*> structsSeen;
if (structureSizeEstimate(*_variable.type(), structsSeen) >= bigint(1) << 64)
m_errorReporter.warning(
_variable.location(),
"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."
);
}
return true;
}
@ -160,3 +171,34 @@ bool StaticAnalyzer::visit(InlineAssembly const& _inlineAssembly)
return true;
}
bigint StaticAnalyzer::structureSizeEstimate(Type const& _type, set<StructDefinition const*>& _structsSeen)
{
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());
}
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);
}
return size;
}
case Type::Category::Mapping:
{
return structureSizeEstimate(*dynamic_cast<MappingType const&>(_type).valueType(), _structsSeen);
}
default:
break;
}
return bigint(1);
}

View File

@ -65,6 +65,9 @@ private:
virtual bool visit(MemberAccess const& _memberAccess) override;
virtual bool visit(InlineAssembly const& _inlineAssembly) override;
/// @returns the size of this type in storage, including all sub-types.
static bigint structureSizeEstimate(Type const& _type, std::set<StructDefinition const*>& _structsSeen);
ErrorReporter& m_errorReporter;
/// Flag that indicates whether the current contract definition is a library.

View File

@ -6545,6 +6545,57 @@ BOOST_AUTO_TEST_CASE(constructor_without_implementation)
CHECK_ERROR(text, TypeError, "Constructor must be implemented if declared.");
}
BOOST_AUTO_TEST_CASE(large_storage_array_fine)
{
char const* text = R"(
contract C {
uint[2**64 - 1] x;
}
)";
CHECK_SUCCESS_NO_WARNINGS(text);
}
BOOST_AUTO_TEST_CASE(large_storage_array_simple)
{
char const* text = R"(
contract C {
uint[2**64] x;
}
)";
CHECK_WARNING(text, "covers a large part of storage and thus makes collisions likely");
}
BOOST_AUTO_TEST_CASE(large_storage_arrays_combined)
{
char const* text = R"(
contract C {
uint[200][200][2**30][][2**30] x;
}
)";
CHECK_WARNING(text, "covers a large part of storage and thus makes collisions likely");
}
BOOST_AUTO_TEST_CASE(large_storage_arrays_struct)
{
char const* text = R"(
contract C {
struct S { uint[2**30] x; uint[2**50] y; }
S[2**20] x;
}
)";
CHECK_WARNING(text, "covers a large part of storage and thus makes collisions likely");
}
BOOST_AUTO_TEST_CASE(large_storage_array_mapping)
{
char const* text = R"(
contract C {
mapping(uint => uint[2**100]) x;
}
)";
CHECK_WARNING(text, "covers a large part of storage and thus makes collisions likely");
}
BOOST_AUTO_TEST_CASE(library_function_without_implementation)
{
char const* text = R"(