Refactor subtype check.

This commit is contained in:
chriseth 2020-07-14 12:12:16 +02:00
parent 526e7b878b
commit 656dd5d5f4
14 changed files with 91 additions and 123 deletions

View File

@ -362,10 +362,7 @@ void DeclarationTypeChecker::endVisit(VariableDeclaration const& _variable)
solAssert(varLoc == Location::Unspecified, "");
typeLoc = (_variable.isConstant() || _variable.immutable()) ? DataLocation::Memory : DataLocation::Storage;
}
else if (
dynamic_cast<StructDefinition const*>(_variable.scope()) ||
dynamic_cast<EnumDefinition const*>(_variable.scope())
)
else if (_variable.isStructMember() || _variable.isEnumMember())
// The actual location will later be changed depending on how the type is used.
typeLoc = DataLocation::Storage;
else

View File

@ -32,6 +32,60 @@ using namespace solidity;
using namespace solidity::langutil;
using namespace solidity::frontend;
namespace
{
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>;
class OversizedTypeChecker
{
public:
OversizedTypeChecker(Type const& _type)
{
visit(_type, true);
}
vector<Type const*> oversizedTypes()
{
return util::convertContainer<vector<Type const*>>(std::move(m_oversizedTypes));
}
private:
void visit(Type const& _type, bool _needsSizeCheck)
{
if (_needsSizeCheck && _type.storageSizeUpperBound() >= bigint(1) << 64)
m_oversizedTypes.insert(&_type);
if (ArrayType const* array = dynamic_cast<ArrayType const*>(&_type))
visit(*array->baseType(), array->isDynamicallySized());
else if (StructType const* structType = dynamic_cast<StructType const*>(&_type))
{
if (m_structsSeen.insert(&structType->structDefinition()).second)
{
for (auto const& member: structType->members(nullptr))
visit(*member.type, false);
m_structsSeen.erase(&structType->structDefinition());
}
}
else if (MappingType const* mappingType = dynamic_cast<MappingType const*>(&_type))
visit(*mappingType->valueType(), true);
}
set<StructDefinition const*> m_structsSeen;
TypeSet m_oversizedTypes;
};
}
/**
* Helper class that determines whether a contract's constructor uses inline assembly.
*/
@ -73,6 +127,7 @@ private:
map<ContractDefinition const*, bool> m_usesAssembly;
};
StaticAnalyzer::StaticAnalyzer(ErrorReporter& _errorReporter):
m_errorReporter(_errorReporter)
{
@ -155,6 +210,19 @@ bool StaticAnalyzer::visit(VariableDeclaration const& _variable)
// This is not a no-op, the entry might pre-exist.
m_localVarUseCount[make_pair(_variable.id(), &_variable)] += 0;
}
// This excludes struct members on purpose.
if (_variable.isStateVariable() || _variable.referenceLocation() == VariableDeclaration::Location::Storage)
for (Type const* subtype: OversizedTypeChecker{*_variable.annotation().type}.oversizedTypes())
m_errorReporter.warning(
7325_error,
_variable.typeName()->location(),
"Type " +
util::escapeAndQuoteString(subtype->canonicalName()) +
" 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;
}

View File

@ -526,10 +526,7 @@ bool TypeChecker::visit(VariableDeclaration const& _variable)
m_errorReporter.typeError(6744_error, _variable.location(), "Internal or recursive type is not allowed for public state variables.");
}
bool isStructMemberDeclaration = dynamic_cast<StructDefinition const*>(_variable.scope()) != nullptr;
if (isStructMemberDeclaration)
return false;
if (!_variable.isStructMember() && !_variable.isEnumMember())
if (auto referenceType = dynamic_cast<ReferenceType const*>(varType))
{
auto result = referenceType->validForLocation(referenceType->location());
@ -543,33 +540,6 @@ bool TypeChecker::visit(VariableDeclaration const& _variable)
}
}
if (varType->dataStoredIn(DataLocation::Storage))
{
auto collisionMessage = [&](string const& variableOrType, bool isVariable) -> string {
return
(isVariable ? "Variable " : "Type ") +
util::escapeAndQuoteString(variableOrType) +
" 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.";
};
if (varType->storageSizeUpperBound() >= bigint(1) << 64)
{
if (_variable.isStateVariable())
m_errorReporter.warning(3408_error, _variable.location(), collisionMessage(_variable.name(), true));
else
m_errorReporter.warning(
2332_error,
_variable.typeName() ? _variable.typeName()->location() : _variable.location(),
collisionMessage(varType->canonicalName(), false)
);
}
vector<Type const*> oversizedSubtypes = frontend::oversizedSubtypes(*varType);
for (Type const* subtype: oversizedSubtypes)
m_errorReporter.warning(7325_error, _variable.typeName()->location(), collisionMessage(subtype->canonicalName(), false));
}
return false;
}

View File

@ -940,6 +940,8 @@ public:
/// Can only be called after reference resolution.
bool hasReferenceOrMappingType() const;
bool isStateVariable() const { return m_isStateVariable; }
bool isStructMember() const { solAssert(annotation().scope, ""); return dynamic_cast<StructDefinition const*>(annotation().scope); }
bool isEnumMember() const { solAssert(annotation().scope, ""); return dynamic_cast<EnumDefinition const*>(annotation().scope); }
bool isIndexed() const { return m_isIndexed; }
Mutability mutability() const { return m_mutability; }
bool isConstant() const { return m_mutability == Mutability::Constant; }

View File

@ -53,57 +53,6 @@ using namespace solidity::frontend;
namespace
{
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>;
void oversizedSubtypesInner(
Type const& _type,
bool _includeType,
set<StructDefinition const*>& _structsSeen,
TypeSet& _oversizedSubtypes
)
{
switch (_type.category())
{
case Type::Category::Array:
{
auto const& t = dynamic_cast<ArrayType const&>(_type);
if (_includeType && t.storageSizeUpperBound() >= bigint(1) << 64)
_oversizedSubtypes.insert(&t);
oversizedSubtypesInner(*t.baseType(), t.isDynamicallySized(), _structsSeen, _oversizedSubtypes);
break;
}
case Type::Category::Struct:
{
auto const& t = dynamic_cast<StructType const&>(_type);
if (_structsSeen.count(&t.structDefinition()))
return;
if (_includeType && t.storageSizeUpperBound() >= bigint(1) << 64)
_oversizedSubtypes.insert(&t);
_structsSeen.insert(&t.structDefinition());
for (auto const& m: t.members(nullptr))
oversizedSubtypesInner(*m.type, false, _structsSeen, _oversizedSubtypes);
_structsSeen.erase(&t.structDefinition());
break;
}
case Type::Category::Mapping:
{
auto const* valueType = dynamic_cast<MappingType const&>(_type).valueType();
oversizedSubtypesInner(*valueType, true, _structsSeen, _oversizedSubtypes);
break;
}
default:
break;
}
}
/// Check whether (_base ** _exp) fits into 4096 bits.
bool fitsPrecisionExp(bigint const& _base, bigint const& _exp)
{
@ -200,16 +149,6 @@ util::Result<TypePointers> transformParametersToExternal(TypePointers const& _pa
}
vector<frontend::Type const*> solidity::frontend::oversizedSubtypes(frontend::Type const& _type)
{
set<StructDefinition const*> structsSeen;
TypeSet oversized;
oversizedSubtypesInner(_type, false, structsSeen, oversized);
vector<frontend::Type const*> res;
copy(oversized.cbegin(), oversized.cend(), back_inserter(res));
return res;
}
void Type::clearCache() const
{
m_members.clear();

View File

@ -59,8 +59,6 @@ using BoolResult = util::Result<bool>;
namespace solidity::frontend
{
std::vector<frontend::Type const*> oversizedSubtypes(frontend::Type const& _type);
inline rational makeRational(bigint const& _numerator, bigint const& _denominator)
{
solAssert(_denominator != 0, "division by zero");

View File

@ -10,6 +10,4 @@ contract b {
}
// ----
// Warning 2519: (105-110): This declaration shadows an existing declaration.
// Warning 3408: (66-69): Variable "d" 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 2332: (105-110): Type "b.c" 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.
// SyntaxError 1719: (105-114): Use of the "var" keyword is disallowed. Use explicit declaration `struct b.c storage pointer d = ...´ instead.

View File

@ -2,4 +2,4 @@ contract C {
uint[2**64] x;
}
// ----
// Warning 3408: (17-30): Variable "x" 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: (17-28): Type "uint256[18446744073709551616]" 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.

View File

@ -3,4 +3,4 @@ contract C {
S[2**20] x;
}
// ----
// Warning 3408: (64-74): Variable "x" 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: (64-72): Type "C.S[1048576]" 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.

View File

@ -55,14 +55,14 @@ contract C {
Q4 q4;
}
// ----
// Warning 3408: (106-111): Variable "s0" 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: (171-176): Variable "s1" 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: (106-108): Type "C.S0" 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: (171-173): Type "C.S1" 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: (341-343): Type "C.P[103]" 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: (341-343): Type "C.P[104]" 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: (505-510): Variable "q0" 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: (505-507): Type "uint256[100000000000000000002]" 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: (505-507): Type "uint256[100000000000000000004]" 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: (576-581): Variable "q1" 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: (505-507): Type "C.Q0" 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: (576-578): Type "C.Q1" 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: (647-649): Type "uint256[100000000000000000006]" 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: (715-720): Variable "q3" 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: (715-717): Type "C.Q3" 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: (783-785): Type "uint256[100000000000000000008]" 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.

View File

@ -6,5 +6,3 @@ contract C {
}
// ----
// TypeError 7676: (60-114): Contract too large for storage.
// Warning 3408: (77-91): Variable "a" 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: (97-111): Variable "b" 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.

View File

@ -8,5 +8,3 @@ contract D is C {
}
// ----
// TypeError 7676: (95-134): Contract too large for storage.
// Warning 3408: (77-91): Variable "a" 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: (117-131): Variable "b" 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.

View File

@ -3,4 +3,4 @@ contract C {
function f(S storage) internal {}
}
// ----
// Warning 2332: (64-65): Type "C.S" 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: (64-65): Type "C.S" 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.

View File

@ -2,4 +2,4 @@ contract c {
uint[2**253] data;
}
// ----
// Warning 3408: (17-34): Variable "data" 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: (17-29): Type "uint256[14474011154664524427946373126085988481658748083205070504932198000989141204992]" 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.