aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--Changelog.md3
-rw-r--r--libsolidity/analysis/TypeChecker.cpp31
-rw-r--r--libsolidity/analysis/TypeChecker.h3
-rw-r--r--libsolidity/ast/Types.cpp10
-rw-r--r--libsolidity/ast/Types.h9
-rw-r--r--libsolidity/codegen/CompilerUtils.cpp41
-rw-r--r--libsolidity/codegen/CompilerUtils.h2
-rw-r--r--libsolidity/codegen/ExpressionCompiler.cpp1
-rw-r--r--test/libsolidity/SolidityEndToEndTest.cpp32
-rw-r--r--test/libsolidity/SolidityNameAndTypeResolution.cpp119
10 files changed, 223 insertions, 28 deletions
diff --git a/Changelog.md b/Changelog.md
index 5f4ec10b..3d8701ca 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -11,6 +11,7 @@ Features:
* Inline Assembly: introduce ``keccak256`` as an opcode. ``sha3`` is still a valid alias.
* Inline Assembly: ``for`` and ``switch`` statements.
* Inline Assembly: function definitions and function calls.
+ * Type Checker: Warn about copies in storage that might overwrite unexpectedly.
* Code Generator: Added the Whiskers template system.
* Remove obsolete Why3 output.
* Type Checker: Enforce strict UTF-8 validation.
@@ -19,6 +20,8 @@ Bugfixes:
* Code generator: Use ``REVERT`` instead of ``INVALID`` for generated input validation routines.
* Type Checker: Fix address literals not being treated as compile-time constants.
* Type Checker: Disallow invoking the same modifier multiple times.
+ * Type Checker: Make UTF8-validation a bit more sloppy to include more valid sequences.
+ * Type Checker: Disallow comparisons between mapping and non-internal function types.
* Type Checker: Do not treat strings that look like addresses as addresses.
* Type Checker: Support valid, but incorrectly rejected UTF-8 sequences.
* Fixed crash concerning non-callable types.
diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index 40ff59f6..ef8a9345 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -364,6 +364,35 @@ void TypeChecker::checkLibraryRequirements(ContractDefinition const& _contract)
m_errorReporter.typeError(var->location(), "Library cannot have non-constant state variables");
}
+void TypeChecker::checkDoubleStorageAssignment(Assignment const& _assignment)
+{
+ TupleType const& lhs = dynamic_cast<TupleType const&>(*type(_assignment.leftHandSide()));
+ TupleType const& rhs = dynamic_cast<TupleType const&>(*type(_assignment.rightHandSide()));
+
+ bool fillRight = !lhs.components().empty() && (!lhs.components().back() || lhs.components().front());
+ size_t storageToStorageCopies = 0;
+ size_t toStorageCopies = 0;
+ for (size_t i = 0; i < lhs.components().size(); ++i)
+ {
+ ReferenceType const* ref = dynamic_cast<ReferenceType const*>(lhs.components()[i].get());
+ if (!ref || !ref->dataStoredIn(DataLocation::Storage) || ref->isPointer())
+ continue;
+ size_t rhsPos = fillRight ? i : rhs.components().size() - (lhs.components().size() - i);
+ solAssert(rhsPos < rhs.components().size(), "");
+ toStorageCopies++;
+ if (rhs.components()[rhsPos]->dataStoredIn(DataLocation::Storage))
+ storageToStorageCopies++;
+ }
+ if (storageToStorageCopies >= 1 && toStorageCopies >= 2)
+ m_errorReporter.warning(
+ _assignment.location(),
+ "This assignment performs two copies to storage. Since storage copies do not first "
+ "copy to a temporary location, one of them might be overwritten before the second "
+ "is executed and thus may have unexpected effects. It is safer to perform the copies "
+ "separately or assign to storage pointers first."
+ );
+}
+
void TypeChecker::endVisit(InheritanceSpecifier const& _inheritance)
{
auto base = dynamic_cast<ContractDefinition const*>(&dereference(_inheritance.name()));
@@ -1047,6 +1076,8 @@ bool TypeChecker::visit(Assignment const& _assignment)
// Sequenced assignments of tuples is not valid, make the result a "void" type.
_assignment.annotation().type = make_shared<TupleType>();
expectType(_assignment.rightHandSide(), *tupleType);
+
+ checkDoubleStorageAssignment(_assignment);
}
else if (t->category() == Type::Category::Mapping)
{
diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h
index 2fa66f97..ee43d13a 100644
--- a/libsolidity/analysis/TypeChecker.h
+++ b/libsolidity/analysis/TypeChecker.h
@@ -69,6 +69,9 @@ private:
void checkContractExternalTypeClashes(ContractDefinition const& _contract);
/// Checks that all requirements for a library are fulfilled if this is a library.
void checkLibraryRequirements(ContractDefinition const& _contract);
+ /// Checks (and warns) if a tuple assignment might cause unexpected overwrites in storage.
+ /// Should only be called if the left hand side is tuple-typed.
+ void checkDoubleStorageAssignment(Assignment const& _assignment);
virtual void endVisit(InheritanceSpecifier const& _inheritance) override;
virtual void endVisit(UsingForDirective const& _usingFor) override;
diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp
index bd3346f9..7dc6c4a6 100644
--- a/libsolidity/ast/Types.cpp
+++ b/libsolidity/ast/Types.cpp
@@ -2248,6 +2248,16 @@ TypePointer FunctionType::unaryOperatorResult(Token::Value _operator) const
return TypePointer();
}
+TypePointer FunctionType::binaryOperatorResult(Token::Value _operator, TypePointer const& _other) const
+{
+ if (_other->category() != category() || !(_operator == Token::Equal || _operator == Token::NotEqual))
+ return TypePointer();
+ FunctionType const& other = dynamic_cast<FunctionType const&>(*_other);
+ if (kind() == Kind::Internal && other.kind() == Kind::Internal && sizeOnStack() == 1 && other.sizeOnStack() == 1)
+ return commonType(shared_from_this(), _other);
+ return TypePointer();
+}
+
string FunctionType::canonicalName(bool) const
{
solAssert(m_kind == Kind::External, "");
diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h
index c4ffc44c..f7a73ab5 100644
--- a/libsolidity/ast/Types.h
+++ b/libsolidity/ast/Types.h
@@ -933,6 +933,7 @@ public:
virtual bool operator==(Type const& _other) const override;
virtual bool isExplicitlyConvertibleTo(Type const& _convertTo) const override;
virtual TypePointer unaryOperatorResult(Token::Value _operator) const override;
+ virtual TypePointer binaryOperatorResult(Token::Value, TypePointer const&) const override;
virtual std::string canonicalName(bool /*_addDataLocation*/) const override;
virtual std::string toString(bool _short) const override;
virtual unsigned calldataEncodedSize(bool _padded) const override;
@@ -1038,6 +1039,7 @@ public:
virtual std::string toString(bool _short) const override;
virtual std::string canonicalName(bool _addDataLocation) const override;
virtual bool canLiveOutsideStorage() const override { return false; }
+ virtual TypePointer binaryOperatorResult(Token::Value, TypePointer const&) const override { return TypePointer(); }
virtual TypePointer encodingType() const override
{
return std::make_shared<IntegerType>(256);
@@ -1116,11 +1118,7 @@ public:
explicit ModuleType(SourceUnit const& _source): m_sourceUnit(_source) {}
- virtual TypePointer binaryOperatorResult(Token::Value, TypePointer const&) const override
- {
- return TypePointer();
- }
-
+ virtual TypePointer binaryOperatorResult(Token::Value, TypePointer const&) const override { return TypePointer(); }
virtual std::string identifier() const override;
virtual bool operator==(Type const& _other) const override;
virtual bool canBeStored() const override { return false; }
@@ -1178,6 +1176,7 @@ public:
virtual std::string identifier() const override { return "t_inaccessible"; }
virtual bool isImplicitlyConvertibleTo(Type const&) const override { return false; }
virtual bool isExplicitlyConvertibleTo(Type const&) const override { return false; }
+ virtual TypePointer binaryOperatorResult(Token::Value, TypePointer const&) const override { return TypePointer(); }
virtual unsigned calldataEncodedSize(bool _padded) const override { (void)_padded; return 32; }
virtual bool canBeStored() const override { return false; }
virtual bool canLiveOutsideStorage() const override { return false; }
diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp
index 7fed1975..4edec155 100644
--- a/libsolidity/codegen/CompilerUtils.cpp
+++ b/libsolidity/codegen/CompilerUtils.cpp
@@ -305,15 +305,9 @@ void CompilerUtils::memoryCopy32()
m_context.appendInlineAssembly(R"(
{
- jumpi(end, eq(len, 0))
- start:
- mstore(dst, mload(src))
- jumpi(end, iszero(gt(len, 32)))
- dst := add(dst, 32)
- src := add(src, 32)
- len := sub(len, 32)
- jump(start)
- end:
+ for { let i := 0 } lt(i, len) { i := add(i, 32) } {
+ mstore(add(dst, i), mload(add(src, i)))
+ }
}
)",
{ "len", "dst", "src" }
@@ -327,21 +321,22 @@ void CompilerUtils::memoryCopy()
m_context.appendInlineAssembly(R"(
{
- // copy 32 bytes at once
- start32:
- jumpi(end32, lt(len, 32))
- mstore(dst, mload(src))
- dst := add(dst, 32)
- src := add(src, 32)
- len := sub(len, 32)
- jump(start32)
- end32:
+ // copy 32 bytes at once
+ for
+ {}
+ iszero(lt(len, 32))
+ {
+ dst := add(dst, 32)
+ src := add(src, 32)
+ len := sub(len, 32)
+ }
+ { mstore(dst, mload(src)) }
- // copy the remainder (0 < len < 32)
- let mask := sub(exp(256, sub(32, len)), 1)
- let srcpart := and(mload(src), not(mask))
- let dstpart := and(mload(dst), mask)
- mstore(dst, or(srcpart, dstpart))
+ // copy the remainder (0 < len < 32)
+ let mask := sub(exp(256, sub(32, len)), 1)
+ let srcpart := and(mload(src), not(mask))
+ let dstpart := and(mload(dst), mask)
+ mstore(dst, or(srcpart, dstpart))
}
)",
{ "len", "dst", "src" }
diff --git a/libsolidity/codegen/CompilerUtils.h b/libsolidity/codegen/CompilerUtils.h
index caf2cdc2..0ee053a9 100644
--- a/libsolidity/codegen/CompilerUtils.h
+++ b/libsolidity/codegen/CompilerUtils.h
@@ -110,10 +110,12 @@ public:
void zeroInitialiseMemoryArray(ArrayType const& _type);
/// Copies full 32 byte words in memory (regions cannot overlap), i.e. may copy more than length.
+ /// Length can be zero, in this case, it copies nothing.
/// Stack pre: <size> <target> <source>
/// Stack post:
void memoryCopy32();
/// Copies data in memory (regions cannot overlap).
+ /// Length can be zero, in this case, it copies nothing.
/// Stack pre: <size> <target> <source>
/// Stack post:
void memoryCopy();
diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp
index a7cfe4dc..a65549fd 100644
--- a/libsolidity/codegen/ExpressionCompiler.cpp
+++ b/libsolidity/codegen/ExpressionCompiler.cpp
@@ -1366,6 +1366,7 @@ void ExpressionCompiler::appendAndOrOperatorCode(BinaryOperation const& _binaryO
void ExpressionCompiler::appendCompareOperatorCode(Token::Value _operator, Type const& _type)
{
+ solAssert(_type.sizeOnStack() == 1, "Comparison of multi-slot types.");
if (_operator == Token::Equal || _operator == Token::NotEqual)
{
if (FunctionType const* funType = dynamic_cast<decltype(funType)>(&_type))
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index 823a8eda..ffee5e36 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -4483,6 +4483,38 @@ BOOST_AUTO_TEST_CASE(array_copy_including_mapping)
BOOST_CHECK(storageEmpty(m_contractAddress));
}
+BOOST_AUTO_TEST_CASE(swap_in_storage_overwrite)
+{
+ // This tests a swap in storage which does not work as one
+ // might expect because we do not have temporary storage.
+ // (x, y) = (y, x) is the same as
+ // y = x;
+ // x = y;
+ char const* sourceCode = R"(
+ contract c {
+ struct S { uint a; uint b; }
+ S public x;
+ S public y;
+ function set() {
+ x.a = 1; x.b = 2;
+ y.a = 3; y.b = 4;
+ }
+ function swap() {
+ (x, y) = (y, x);
+ }
+ }
+ )";
+ compileAndRun(sourceCode);
+ BOOST_CHECK(callContractFunction("x()") == encodeArgs(u256(0), u256(0)));
+ BOOST_CHECK(callContractFunction("y()") == encodeArgs(u256(0), u256(0)));
+ BOOST_CHECK(callContractFunction("set()") == encodeArgs());
+ BOOST_CHECK(callContractFunction("x()") == encodeArgs(u256(1), u256(2)));
+ BOOST_CHECK(callContractFunction("y()") == encodeArgs(u256(3), u256(4)));
+ BOOST_CHECK(callContractFunction("swap()") == encodeArgs());
+ BOOST_CHECK(callContractFunction("x()") == encodeArgs(u256(1), u256(2)));
+ BOOST_CHECK(callContractFunction("y()") == encodeArgs(u256(1), u256(2)));
+}
+
BOOST_AUTO_TEST_CASE(pass_dynamic_arguments_to_the_base)
{
char const* sourceCode = R"(
diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp
index a6027812..b489be22 100644
--- a/test/libsolidity/SolidityNameAndTypeResolution.cpp
+++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp
@@ -554,6 +554,51 @@ BOOST_AUTO_TEST_CASE(comparison_bitop_precedence)
CHECK_SUCCESS(text);
}
+BOOST_AUTO_TEST_CASE(comparison_of_function_types)
+{
+ char const* text = R"(
+ contract C {
+ function f() returns (bool ret) {
+ return this.f < this.f;
+ }
+ }
+ )";
+ CHECK_ERROR(text, TypeError, "Operator < not compatible");
+ text = R"(
+ contract C {
+ function f() returns (bool ret) {
+ return f < f;
+ }
+ }
+ )";
+ CHECK_ERROR(text, TypeError, "Operator < not compatible");
+ text = R"(
+ contract C {
+ function f() returns (bool ret) {
+ return f == f;
+ }
+ function g() returns (bool ret) {
+ return f != f;
+ }
+ }
+ )";
+ CHECK_SUCCESS(text);
+}
+
+BOOST_AUTO_TEST_CASE(comparison_of_mapping_types)
+{
+ char const* text = R"(
+ contract C {
+ mapping(uint => uint) x;
+ function f() returns (bool ret) {
+ var y = x;
+ return x == y;
+ }
+ }
+ )";
+ CHECK_ERROR(text, TypeError, "Operator == not compatible");
+}
+
BOOST_AUTO_TEST_CASE(function_no_implementation)
{
ASTPointer<SourceUnit> sourceUnit;
@@ -5771,6 +5816,80 @@ BOOST_AUTO_TEST_CASE(pure_statement_check_for_regular_for_loop)
success(text);
}
+BOOST_AUTO_TEST_CASE(warn_multiple_storage_storage_copies)
+{
+ char const* text = R"(
+ contract C {
+ struct S { uint a; uint b; }
+ S x; S y;
+ function f() {
+ (x, y) = (y, x);
+ }
+ }
+ )";
+ CHECK_WARNING(text, "This assignment performs two copies to storage.");
+}
+
+BOOST_AUTO_TEST_CASE(warn_multiple_storage_storage_copies_fill_right)
+{
+ char const* text = R"(
+ contract C {
+ struct S { uint a; uint b; }
+ S x; S y;
+ function f() {
+ (x, y, ) = (y, x, 1, 2);
+ }
+ }
+ )";
+ CHECK_WARNING(text, "This assignment performs two copies to storage.");
+}
+
+BOOST_AUTO_TEST_CASE(warn_multiple_storage_storage_copies_fill_left)
+{
+ char const* text = R"(
+ contract C {
+ struct S { uint a; uint b; }
+ S x; S y;
+ function f() {
+ (,x, y) = (1, 2, y, x);
+ }
+ }
+ )";
+ CHECK_WARNING(text, "This assignment performs two copies to storage.");
+}
+
+BOOST_AUTO_TEST_CASE(nowarn_swap_memory)
+{
+ char const* text = R"(
+ contract C {
+ struct S { uint a; uint b; }
+ function f() {
+ S memory x;
+ S memory y;
+ (x, y) = (y, x);
+ }
+ }
+ )";
+ CHECK_SUCCESS_NO_WARNINGS(text);
+}
+
+BOOST_AUTO_TEST_CASE(nowarn_swap_storage_pointers)
+{
+ char const* text = R"(
+ contract C {
+ struct S { uint a; uint b; }
+ S x; S y;
+ function f() {
+ S storage x_local = x;
+ S storage y_local = y;
+ S storage z_local = x;
+ (x, y_local, x_local, z_local) = (y, x_local, y_local, y);
+ }
+ }
+ )";
+ CHECK_SUCCESS_NO_WARNINGS(text);
+}
+
BOOST_AUTO_TEST_CASE(warn_unused_local)
{
char const* text = R"(
generated by cgit v1.2.3 (git 2.25.1) at 2024-12-31 07:07:44 +0800