Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type checking for contract explicit storage base location #15528

Draft
wants to merge 10 commits into
base: storageLocationsParserSupport
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 80 additions & 0 deletions libsolidity/analysis/TypeChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
*/

#include <libsolidity/analysis/TypeChecker.h>
#include <libsolidity/analysis/ConstantEvaluator.h>
#include <libsolidity/ast/AST.h>
#include <libsolidity/ast/ASTUtils.h>
#include <libsolidity/ast/UserDefinableOperators.h>
Expand All @@ -46,6 +47,7 @@
#include <range/v3/algorithm/count_if.hpp>
#include <range/v3/view/drop_exactly.hpp>
#include <range/v3/view/enumerate.hpp>
#include <range/v3/view/reverse.hpp>
#include <range/v3/view/zip.hpp>

#include <memory>
Expand Down Expand Up @@ -231,6 +233,84 @@ bool TypeChecker::visit(ImportDirective const&)
return false;
}

void TypeChecker::endVisit(ContractDefinition const& _contract)
{
for (auto const* ancestorContract: _contract.annotation().linearizedBaseContracts | ranges::views::reverse)
if (*ancestorContract != _contract && ancestorContract->storageBaseLocationExpression())
{
m_errorReporter.typeError(
8894_error,
_contract.location(),
SecondarySourceLocation().append(
"Storage base location was already specified here.",
ancestorContract->storageBaseLocationExpression()->location()
),
"Storage base location can only be specified in the most derived contract."
);
return;
}
if (ASTPointer<Expression> const baseLocation = _contract.storageBaseLocationExpression())
{
if (_contract.isLibrary() || _contract.abstract())
m_errorReporter.typeError(
7587_error,
baseLocation->location(),
"Storage base location cannot be specified for abstract contracts or libraries"
);

if (!*baseLocation->annotation().isPure)
{
// TODO: handle erc7201 as a builtin function ?
if (auto functionCall = dynamic_cast<FunctionCall const*>(baseLocation.get()))
if (
auto const* identifier = dynamic_cast<Identifier const*>(&functionCall->expression());
identifier && identifier->name() == "erc7201"
)
return;

m_errorReporter.typeError(
1139_error,
baseLocation->location(),
"The contract base location must be an expression that can be evaluated at compilation time."
);
}

auto const* expressionType = type(*baseLocation);
BoolResult result = expressionType->isImplicitlyConvertibleTo(*TypeProvider::uint256());
if (!result)
{
std::string errorMessage = "Contract storage base location ";
if (
auto const* rationalType = dynamic_cast<RationalNumberType const*>(expressionType);
rationalType && rationalType->isFractional()
)
errorMessage += "cannot be specified by a fractional number.";
else
errorMessage += "must be in range of type uint256. Current type is " + expressionType->humanReadableName();

m_errorReporter.typeErrorConcatenateDescriptions(
1763_error,
baseLocation->location(),
errorMessage,
result.message()
);
return;
}

if (auto const* rationalType = dynamic_cast<RationalNumberType const*>(expressionType))
{
solAssert(!rationalType->isFractional());
_contract.annotation().storageBaseLocationValue = u256(rationalType->value().numerator());
}
else
m_errorReporter.typeError(
6396_error,
baseLocation->location(),
"Only number literals are accepted in the expression specifying the contract base storage location."
);
}
}

void TypeChecker::endVisit(InheritanceSpecifier const& _inheritance)
{
auto base = dynamic_cast<ContractDefinition const*>(&dereference(_inheritance.name()));
Expand Down
1 change: 1 addition & 0 deletions libsolidity/analysis/TypeChecker.h
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ class TypeChecker: private ASTConstVisitor
void endVisit(ElementaryTypeNameExpression const& _expr) override;
void endVisit(Literal const& _literal) override;
void endVisit(UsingForDirective const& _usingForDirective) override;
void endVisit(ContractDefinition const& _contract) override;

void checkErrorAndEventParameters(CallableDeclaration const& _callable);

Expand Down
4 changes: 4 additions & 0 deletions libsolidity/ast/AST.h
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,10 @@ class ContractDefinition: public Declaration, public StructurallyDocumented, pub
/// Returns the ether receiver function or nullptr if no receive function was specified.
FunctionDefinition const* receiveFunction() const;

ASTPointer<Expression> const& storageBaseLocationExpression() const { return m_storageBaseLocationExpression; }

u256 storageBaseLocationValue() const { return annotation().storageBaseLocationValue; }

std::string fullyQualifiedName() const { return sourceUnitName() + ":" + name(); }

Type const* type() const override;
Expand Down
3 changes: 3 additions & 0 deletions libsolidity/ast/ASTAnnotations.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <libsolidity/ast/ASTEnums.h>
#include <libsolidity/ast/ExperimentalFeatures.h>

#include <libsolutil/Numeric.h>
#include <libsolutil/SetOnce.h>

#include <map>
Expand Down Expand Up @@ -171,6 +172,8 @@ struct ContractDefinitionAnnotation: TypeDeclarationAnnotation, StructurallyDocu

// Per-contract map from function AST IDs to internal dispatch function IDs.
std::map<FunctionDefinition const*, uint64_t> internalFunctionIDs;

solidity::u256 storageBaseLocationValue;
};

struct CallableDeclarationAnnotation: DeclarationAnnotation
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
abstract contract C layout at 0x123 {
}
// ----
// TypeError 7587: (30-35): Storage base location cannot be specified for abstract contracts or libraries
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
contract A layout at 0x1234 {}

contract B is A {}

contract C is B layout at 0xABCD {}
// ----
// TypeError 8894: (32-50): Storage base location can only be specified in the most derived contract.
// TypeError 8894: (52-87): Storage base location can only be specified in the most derived contract.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
contract A layout at 3/2 {}
// ----
// TypeError 1763: (21-24): Contract storage base location cannot be specified by a fractional number.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
contract A layout at 0x1234 {}

contract B is A {}

contract C is B {}
// ----
// TypeError 8894: (32-50): Storage base location can only be specified in the most derived contract.
// TypeError 8894: (52-70): Storage base location can only be specified in the most derived contract.
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
uint constant X = 42;
contract C layout at 0xffff * (50 - X) { }
// ----
// TypeError 6396: (43-60): Only number literals are accepted in the expression specifying the contract base storage location.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function f(uint x) returns (uint) { return x + 1; }
contract A layout at f(2) {}
// ----
// TypeError 1139: (73-77): The contract base location must be an expression that can be evaluated at compilation time.
// TypeError 6396: (73-77): Only number literals are accepted in the expression specifying the contract base storage location.
5 changes: 5 additions & 0 deletions test/libsolidity/syntaxTests/contractBaseLocation/library.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
library L layout at 0x123 {
function f() public pure { }
}
// ----
// TypeError 7587: (20-25): Storage base location cannot be specified for abstract contracts or libraries
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
contract A layout at 0x1234 {}
contract B layout at 1024 {}
contract C layout at "C" {}
contract C layout at 0 {}
// ----