diff --git a/Changelog.md b/Changelog.md index f9f9bf9d1fb5..89d306c5ada8 100644 --- a/Changelog.md +++ b/Changelog.md @@ -16,6 +16,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: Consider mappings as well when checking for uninitialized return values. * 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. diff --git a/libsolidity/analysis/ControlFlowAnalyzer.cpp b/libsolidity/analysis/ControlFlowAnalyzer.cpp index 483d08c829b4..a69a8abd326c 100644 --- a/libsolidity/analysis/ControlFlowAnalyzer.cpp +++ b/libsolidity/analysis/ControlFlowAnalyzer.cpp @@ -75,7 +75,10 @@ void ControlFlowAnalyzer::checkUnassignedStorageReturnValues( { auto& unassignedAtFunctionEntry = unassigned[_functionEntry]; for (auto const& returnParameter: _function.returnParameterList()->parameters()) - if (returnParameter->type()->dataStoredIn(DataLocation::Storage)) + if ( + returnParameter->type()->dataStoredIn(DataLocation::Storage) || + returnParameter->type()->category() == Type::Category::Mapping + ) unassignedAtFunctionEntry.insert(returnParameter.get()); } @@ -147,7 +150,9 @@ void ControlFlowAnalyzer::checkUnassignedStorageReturnValues( m_errorReporter.typeError( returnVal->location(), ssl, - "This variable is of storage pointer type and might be returned without assignment and " + string("This variable is of ") + + (returnVal->type()->category() == Type::Category::Mapping ? "mapping" : "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/mappingReturn/named_err.sol b/test/libsolidity/syntaxTests/controlFlow/mappingReturn/named_err.sol new file mode 100644 index 000000000000..1120d950f106 --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/mappingReturn/named_err.sol @@ -0,0 +1,5 @@ +contract C { + function f() internal pure returns (mapping(uint=>uint) storage r) { } +} +// ---- +// TypeError: (53-82): This variable is of mapping 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/mappingReturn/named_fine.sol b/test/libsolidity/syntaxTests/controlFlow/mappingReturn/named_fine.sol new file mode 100644 index 000000000000..4146192f3097 --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/mappingReturn/named_fine.sol @@ -0,0 +1,5 @@ +contract C { + mapping(uint=>uint) m; + function f() internal view returns (mapping(uint=>uint) storage r) { r = m; } +} +// ---- diff --git a/test/libsolidity/syntaxTests/controlFlow/mappingReturn/unnamed_err.sol b/test/libsolidity/syntaxTests/controlFlow/mappingReturn/unnamed_err.sol new file mode 100644 index 000000000000..e82b50aaf957 --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/mappingReturn/unnamed_err.sol @@ -0,0 +1,5 @@ +contract C { + function f() internal pure returns (mapping(uint=>uint) storage) {} +} +// ---- +// TypeError: (53-72): This variable is of mapping 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/mappingReturn/unnamed_fine.sol b/test/libsolidity/syntaxTests/controlFlow/mappingReturn/unnamed_fine.sol new file mode 100644 index 000000000000..9c5e3149da5b --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/mappingReturn/unnamed_fine.sol @@ -0,0 +1,5 @@ +contract C { + mapping(uint=>uint) m; + function f() internal view returns (mapping(uint=>uint) storage) { return m; } +} +// ----