diff options
23 files changed, 184 insertions, 183 deletions
diff --git a/Changelog.md b/Changelog.md index c21f0ae9..e5306dc0 100644 --- a/Changelog.md +++ b/Changelog.md @@ -17,6 +17,7 @@ Breaking Changes: * Commandline interface: Remove obsolete ``--formal`` option. * Commandline interface: Rename the ``--julia`` option to ``--yul``. * Commandline interface: Require ``-`` if standard input is used as source. + * Control Flow Analyzer: Turn warning about returning uninitialized storage pointers into an error. * General: ``continue`` in a ``do...while`` loop jumps to the condition (it used to jump to the loop body). Warning: this may silently change the semantics of existing code. * General: Disallow declaring empty structs. * General: Disallow raw ``callcode`` (was already deprecated in 0.4.12). It is still possible to use it via inline assembly. diff --git a/libsolidity/analysis/ControlFlowAnalyzer.cpp b/libsolidity/analysis/ControlFlowAnalyzer.cpp index 6edf7986..483d08c8 100644 --- a/libsolidity/analysis/ControlFlowAnalyzer.cpp +++ b/libsolidity/analysis/ControlFlowAnalyzer.cpp @@ -144,12 +144,12 @@ void ControlFlowAnalyzer::checkUnassignedStorageReturnValues( ssl.append("Problematic end of function:", _function.location()); } - m_errorReporter.warning( + m_errorReporter.typeError( returnVal->location(), - "This variable is of storage pointer type and might be returned without assignment. " - "This can cause storage corruption. Assign the variable (potentially from itself) " - "to remove this warning.", - ssl + ssl, + "This variable is of storage pointer type and might be returned without assignment and " + "could be used uninitialized. Assign the variable (potentially from itself) " + "to fix this error." ); } } diff --git a/test/libsolidity/SolidityABIJSON.cpp b/test/libsolidity/SolidityABIJSON.cpp index 14413ca0..f2d6d66f 100644 --- a/test/libsolidity/SolidityABIJSON.cpp +++ b/test/libsolidity/SolidityABIJSON.cpp @@ -756,7 +756,7 @@ BOOST_AUTO_TEST_CASE(library_function) char const* sourceCode = R"( library test { struct StructType { uint a; } - function f(StructType storage b, uint[] storage c, test d) public returns (uint[] memory e, StructType storage f) {} + function f(StructType storage b, uint[] storage c, test d) public returns (uint[] memory e, StructType storage f) { f = f; } } )"; diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_err.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_err.sol new file mode 100644 index 00000000..5fde497c --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_err.sol @@ -0,0 +1,10 @@ +contract C { + struct S { bool f; } + S s; + function f() internal pure returns (S storage) { + assembly { + } + } +} +// ---- +// TypeError: (87-88): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_fine.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_fine.sol index 65902cc8..0d3db856 100644 --- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_fine.sol +++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_fine.sol @@ -8,7 +8,7 @@ contract C { } function g(bool flag) internal returns (S storage c) { // control flow in assembly will not be analyzed for now, - // so this will not issue a warning + // so this will not issue an error assembly { if flag { sstore(c_slot, sload(s_slot)) @@ -17,7 +17,7 @@ contract C { } function h() internal returns (S storage c) { // any reference from assembly will be sufficient for now, - // so this will not issue a warning + // so this will not issue an error assembly { sstore(s_slot, sload(c_slot)) } diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_warn.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_warn.sol deleted file mode 100644 index 09c13847..00000000 --- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_warn.sol +++ /dev/null @@ -1,10 +0,0 @@ -contract C { - struct S { bool f; } - S s; - function f() internal pure returns (S storage) { - assembly { - } - } -} -// ---- -// Warning: (87-88): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_err.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_err.sol new file mode 100644 index 00000000..eb574c96 --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_err.sol @@ -0,0 +1,52 @@ +contract C { + struct S { bool f; } + S s; + function f() internal view returns (S storage c) { + do { + break; + c = s; + } while(false); + } + function g() internal view returns (S storage c) { + do { + if (s.f) { + continue; + c = s; + } + else { + } + } while(false); + } + function h() internal view returns (S storage c) { + do { + if (s.f) { + break; + } + else { + c = s; + } + } while(false); + } + function i() internal view returns (S storage c) { + do { + if (s.f) { + continue; + } + else { + c = s; + } + } while(false); + } + function j() internal view returns (S storage c) { + do { + continue; + c = s; + } while(false); + } +} +// ---- +// TypeError: (87-98): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error. +// TypeError: (223-234): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error. +// TypeError: (440-451): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error. +// TypeError: (654-665): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error. +// TypeError: (871-882): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_warn.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_warn.sol deleted file mode 100644 index 7d001c19..00000000 --- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_warn.sol +++ /dev/null @@ -1,52 +0,0 @@ -contract C { - struct S { bool f; } - S s; - function f() internal view returns (S storage c) { - do { - break; - c = s; - } while(false); - } - function g() internal view returns (S storage c) { - do { - if (s.f) { - continue; - c = s; - } - else { - } - } while(false); - } - function h() internal view returns (S storage c) { - do { - if (s.f) { - break; - } - else { - c = s; - } - } while(false); - } - function i() internal view returns (S storage c) { - do { - if (s.f) { - continue; - } - else { - c = s; - } - } while(false); - } - function j() internal view returns (S storage c) { - do { - continue; - c = s; - } while(false); - } -} -// ---- -// Warning: (87-98): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning. -// Warning: (223-234): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning. -// Warning: (440-451): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning. -// Warning: (654-665): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning. -// Warning: (871-882): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_err.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_err.sol new file mode 100644 index 00000000..0d266ccf --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_err.sol @@ -0,0 +1,14 @@ +contract C { + struct S { bool f; } + S s; + function f() internal pure returns (S storage) { return; } + function g() internal view returns (S storage c, S storage) { c = s; return; } + function h() internal view returns (S storage, S storage d) { d = s; return; } + function i() internal pure returns (S storage, S storage) { return; } +} +// ---- +// TypeError: (87-88): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error. +// TypeError: (163-164): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error. +// TypeError: (233-234): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error. +// TypeError: (316-317): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error. +// TypeError: (327-328): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_fine.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_fine.sol index 3a0a30ea..6d72e4ef 100644 --- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_fine.sol +++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_fine.sol @@ -2,5 +2,6 @@ contract C { struct S { bool f; } S s; function f() internal view returns (S storage c, S storage d) { c = s; d = s; return; } + function g() internal view returns (S storage, S storage) { return (s,s); } } // ---- diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_warn.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_warn.sol deleted file mode 100644 index 0a5b2fbf..00000000 --- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_warn.sol +++ /dev/null @@ -1,15 +0,0 @@ -contract C { - struct S { bool f; } - S s; - function f() internal pure returns (S storage) { return; } - function g() internal view returns (S storage c, S storage) { c = s; return; } - function h() internal view returns (S storage, S storage d) { d = s; return; } - function i() internal pure returns (S storage, S storage) { return; } - function j() internal view returns (S storage, S storage) { return (s,s); } -} -// ---- -// Warning: (87-88): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning. -// Warning: (163-164): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning. -// Warning: (233-234): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning. -// Warning: (316-317): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning. -// Warning: (327-328): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/for_err.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/for_err.sol new file mode 100644 index 00000000..9aa580a4 --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/for_err.sol @@ -0,0 +1,16 @@ +contract C { + struct S { bool f; } + S s; + function f() internal view returns (S storage c) { + for(;; c = s) { + } + } + function g() internal view returns (S storage c) { + for(;;) { + c = s; + } + } +} +// ---- +// TypeError: (87-98): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error. +// TypeError: (182-193): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/for_warn.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/for_warn.sol deleted file mode 100644 index ba9a2440..00000000 --- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/for_warn.sol +++ /dev/null @@ -1,16 +0,0 @@ -contract C { - struct S { bool f; } - S s; - function f() internal view returns (S storage c) { - for(;; c = s) { - } - } - function g() internal view returns (S storage c) { - for(;;) { - c = s; - } - } -} -// ---- -// Warning: (87-98): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning. -// Warning: (182-193): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/if_err.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/if_err.sol new file mode 100644 index 00000000..f3e55318 --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/if_err.sol @@ -0,0 +1,18 @@ +contract C { + struct S { bool f; } + S s; + function f(bool flag) internal view returns (S storage c) { + if (flag) c = s; + } + function g(bool flag) internal returns (S storage c) { + if (flag) c = s; + else + { + if (!flag) c = s; + else s.f = true; + } + } +} +// ---- +// TypeError: (96-107): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error. +// TypeError: (186-197): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/if_warn.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/if_warn.sol deleted file mode 100644 index c257c252..00000000 --- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/if_warn.sol +++ /dev/null @@ -1,18 +0,0 @@ -contract C { - struct S { bool f; } - S s; - function f(bool flag) internal view returns (S storage c) { - if (flag) c = s; - } - function g(bool flag) internal returns (S storage c) { - if (flag) c = s; - else - { - if (!flag) c = s; - else s.f = true; - } - } -} -// ---- -// Warning: (96-107): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning. -// Warning: (186-197): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/modifier_err.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/modifier_err.sol new file mode 100644 index 00000000..a0047782 --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/modifier_err.sol @@ -0,0 +1,22 @@ +contract C { + modifier revertIfNoReturn() { + _; + revert(); + } + modifier ifFlag(bool flag) { + if (flag) + _; + } + struct S { uint a; } + S s; + function f(bool flag) ifFlag(flag) internal view returns(S storage) { + return s; + } + + function g(bool flag) ifFlag(flag) revertIfNoReturn() internal view returns(S storage) { + return s; + } +} +// ---- +// TypeError: (249-250): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error. +// TypeError: (367-368): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/modifier_warn.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/modifier_warn.sol deleted file mode 100644 index 50c6dd99..00000000 --- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/modifier_warn.sol +++ /dev/null @@ -1,22 +0,0 @@ -contract C { - modifier revertIfNoReturn() { - _; - revert(); - } - modifier ifFlag(bool flag) { - if (flag) - _; - } - struct S { uint a; } - S s; - function f(bool flag) ifFlag(flag) internal view returns(S storage) { - return s; - } - - function g(bool flag) ifFlag(flag) revertIfNoReturn() internal view returns(S storage) { - return s; - } -} -// ---- -// Warning: (249-250): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning. -// Warning: (367-368): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/short_circuit_err.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/short_circuit_err.sol new file mode 100644 index 00000000..d0ad8245 --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/short_circuit_err.sol @@ -0,0 +1,18 @@ +contract C { + struct S { bool f; } + S s; + function f() internal view returns (S storage c) { + false && (c = s).f; + } + function g() internal view returns (S storage c) { + true || (c = s).f; + } + function h() internal view returns (S storage c) { + // expect error, although this is always fine + true && (false || (c = s).f); + } +} +// ---- +// TypeError: (87-98): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error. +// TypeError: (176-187): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error. +// TypeError: (264-275): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/short_circuit_warn.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/short_circuit_warn.sol deleted file mode 100644 index 9f660f11..00000000 --- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/short_circuit_warn.sol +++ /dev/null @@ -1,18 +0,0 @@ -contract C { - struct S { bool f; } - S s; - function f() internal view returns (S storage c) { - false && (c = s).f; - } - function g() internal view returns (S storage c) { - true || (c = s).f; - } - function h() internal view returns (S storage c) { - // expect warning, although this is always fine - true && (false || (c = s).f); - } -} -// ---- -// Warning: (87-98): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning. -// Warning: (176-187): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning. -// Warning: (264-275): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/ternary_err.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/ternary_err.sol new file mode 100644 index 00000000..6d10287b --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/ternary_err.sol @@ -0,0 +1,13 @@ +contract C { + struct S { bool f; } + S s; + function f(bool flag) internal view returns (S storage c) { + flag ? (c = s).f : false; + } + function g(bool flag) internal view returns (S storage c) { + flag ? false : (c = s).f; + } +} +// ---- +// TypeError: (96-107): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error. +// TypeError: (200-211): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/ternary_warn.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/ternary_warn.sol deleted file mode 100644 index 57561fbb..00000000 --- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/ternary_warn.sol +++ /dev/null @@ -1,13 +0,0 @@ -contract C { - struct S { bool f; } - S s; - function f(bool flag) internal view returns (S storage c) { - flag ? (c = s).f : false; - } - function g(bool flag) internal view returns (S storage c) { - flag ? false : (c = s).f; - } -} -// ---- -// Warning: (96-107): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning. -// Warning: (200-211): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/while_err.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/while_err.sol new file mode 100644 index 00000000..e7b4fae7 --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/while_err.sol @@ -0,0 +1,11 @@ +contract C { + struct S { bool f; } + S s; + function f() internal view returns (S storage c) { + while(false) { + c = s; + } + } +} +// ---- +// TypeError: (87-98): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/while_warn.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/while_warn.sol deleted file mode 100644 index 26db892f..00000000 --- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/while_warn.sol +++ /dev/null @@ -1,11 +0,0 @@ -contract C { - struct S { bool f; } - S s; - function f() internal view returns (S storage c) { - while(false) { - c = s; - } - } -} -// ---- -// Warning: (87-98): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning. |