From 09485058d89b0187e7919cdcc7e39f586d03109d Mon Sep 17 00:00:00 2001 From: Christian Parpart Date: Wed, 13 May 2020 13:51:49 +0200 Subject: [PATCH 01/36] Adds ``--base-path` to solc when compiling in `--standard-json` mode for resolving relative paths. --- Changelog.md | 1 + docs/using-the-compiler.rst | 8 ++++++++ solc/CommandLineInterface.cpp | 23 ++++++++++++++++++++++- solc/CommandLineInterface.h | 2 ++ 4 files changed, 33 insertions(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index ad568e6ae..cba4521a0 100644 --- a/Changelog.md +++ b/Changelog.md @@ -6,6 +6,7 @@ Language Features: Compiler Features: * Build system: Update the soljson.js build to emscripten 1.39.15 and boost 1.73.0 and include Z3 for integrated SMTChecker support without the callback mechanism. + * Commandline Interface: Adds new option ``--base-path PATH`` to use the given path as the root of the source tree instead of the root of the filesystem. * SMTChecker: Support array ``length``. * SMTChecker: Support array ``push`` and ``pop``. * Add support for natspec comments on state variables. diff --git a/docs/using-the-compiler.rst b/docs/using-the-compiler.rst index 9009fac01..3f0252884 100644 --- a/docs/using-the-compiler.rst +++ b/docs/using-the-compiler.rst @@ -44,8 +44,15 @@ An empty remapping prefix is not allowed. If there are multiple matches due to remappings, the one with the longest common prefix is selected. +When accessing the filesystem to search for imports, all paths are treated as if they were fully qualified paths. +This behaviour can be customized by adding the command line option ``--base-path`` with a path to be prepended +before each filesystem access for imports is performed. Furthermore, the part added via ``--base-path`` +will not appear in the contract metadata. + For security reasons the compiler has restrictions what directories it can access. Paths (and their subdirectories) of source files specified on the commandline and paths defined by remappings are allowed for import statements, but everything else is rejected. Additional paths (and their subdirectories) can be allowed via the ``--allow-paths /sample/path,/another/sample/path`` switch. +Everything inside the path specified via ``--base-path`` is always allowed. + If your contracts use :ref:`libraries `, you will notice that the bytecode contains substrings of the form ``__$53aea86b7d70b31448b230b20ae141a537$__``. These are placeholders for the actual library addresses. The placeholder is a 34 character prefix of the hex encoding of the keccak256 hash of the fully qualified library name. The bytecode file will also contain lines of the form ``// -> `` at the end to help @@ -58,6 +65,7 @@ Either add ``--libraries "file.sol:Math:0x12345678901234567890123456789012345678 If ``solc`` is called with the option ``--link``, all input files are interpreted to be unlinked binaries (hex-encoded) in the ``__$53aea86b7d70b31448b230b20ae141a537$__``-format given above and are linked in-place (if the input is read from stdin, it is written to stdout). All options except ``--libraries`` are ignored (including ``-o``) in this case. If ``solc`` is called with the option ``--standard-json``, it will expect a JSON input (as explained below) on the standard input, and return a JSON output on the standard output. This is the recommended interface for more complex and especially automated uses. The process will always terminate in a "success" state and report any errors via the JSON output. +The option ``--base-path`` is also processed in standard-json mode. .. note:: The library placeholder used to be the fully qualified name of the library itself diff --git a/solc/CommandLineInterface.cpp b/solc/CommandLineInterface.cpp index cd792d9c0..a8e5cc5b7 100644 --- a/solc/CommandLineInterface.cpp +++ b/solc/CommandLineInterface.cpp @@ -107,6 +107,7 @@ std::ostream& serr(bool _used = true) static string const g_stdinFileNameStr = ""; static string const g_strAbi = "abi"; static string const g_strAllowPaths = "allow-paths"; +static string const g_strBasePath = "base-path"; static string const g_strAsm = "asm"; static string const g_strAsmJson = "asm-json"; static string const g_strAssemble = "assemble"; @@ -181,6 +182,7 @@ static string const g_strOldReporter = "old-reporter"; static string const g_argAbi = g_strAbi; static string const g_argPrettyJson = g_strPrettyJson; static string const g_argAllowPaths = g_strAllowPaths; +static string const g_argBasePath = g_strBasePath; static string const g_argAsm = g_strAsm; static string const g_argAsmJson = g_strAsmJson; static string const g_argAssemble = g_strAssemble; @@ -830,6 +832,11 @@ Allowed options)").c_str(), po::value()->value_name("path(s)"), "Allow a given path for imports. A list of paths can be supplied by separating them with a comma." ) + ( + g_argBasePath.c_str(), + po::value()->value_name("path"), + "Use the given path as the root of the source tree instead of the root of the filesystem." + ) (g_argColor.c_str(), "Force colored output.") (g_argNoColor.c_str(), "Explicitly disable colored output, disabling terminal auto-detection.") (g_argOldReporter.c_str(), "Enables old diagnostics reporter.") @@ -965,7 +972,8 @@ bool CommandLineInterface::processInput() string validPath = _path; if (validPath.find("file://") == 0) validPath.erase(0, 7); - auto path = boost::filesystem::path(validPath); + + auto const path = m_basePath / validPath; auto canonicalPath = boost::filesystem::weakly_canonical(path); bool isAllowed = false; for (auto const& allowedDir: m_allowedDirectories) @@ -1003,6 +1011,19 @@ bool CommandLineInterface::processInput() } }; + if (m_args.count(g_argBasePath)) + { + boost::filesystem::path const fspath{m_args[g_argBasePath].as()}; + if (!boost::filesystem::is_directory(fspath)) + { + serr() << "Base path must be a directory: \"" << fspath << "\"\n"; + return false; + } + m_basePath = fspath; + if (!contains(m_allowedDirectories, fspath)) + m_allowedDirectories.push_back(fspath); + } + if (m_args.count(g_argAllowPaths)) { vector paths; diff --git a/solc/CommandLineInterface.h b/solc/CommandLineInterface.h index 446ea8224..00b9181ec 100644 --- a/solc/CommandLineInterface.h +++ b/solc/CommandLineInterface.h @@ -117,6 +117,8 @@ private: std::vector m_remappings; /// list of allowed directories to read files from std::vector m_allowedDirectories; + /// Base path, used for resolving relative paths in imports. + boost::filesystem::path m_basePath; /// map of library names to addresses std::map m_libraries; /// Solidity compiler stack From 7548441b4fb394012d0f14c5058bc8da8413fe56 Mon Sep 17 00:00:00 2001 From: Mathias Baumann Date: Wed, 27 May 2020 12:13:37 +0200 Subject: [PATCH 02/36] Check for wrong error codes in the CI --- .circleci/config.yml | 10 ++++++++++ scripts/correct_error_ids.py | 38 ++++++++++++++++++++++++------------ 2 files changed, 36 insertions(+), 12 deletions(-) mode change 100644 => 100755 scripts/correct_error_ids.py diff --git a/.circleci/config.yml b/.circleci/config.yml index a34649d74..7f2b46a0c 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -316,6 +316,15 @@ jobs: name: checking shell scripts command: ./scripts/chk_shellscripts/chk_shellscripts.sh + chk_errorcodes: + docker: + - image: circleci/python:3.6 + steps: + - checkout + - run: + name: Check for error codes + command: ./scripts/correct_error_ids.py --noconfirm + chk_pylint: docker: - image: buildpack-deps:eoan @@ -845,6 +854,7 @@ workflows: - chk_buglist: *workflow_trigger_on_tags - chk_proofs: *workflow_trigger_on_tags - chk_pylint: *workflow_trigger_on_tags + - chk_errorcodes: *workflow_trigger_on_tags - chk_antlr_grammar: *workflow_trigger_on_tags - chk_docs_pragma_min_version: *workflow_trigger_on_tags diff --git a/scripts/correct_error_ids.py b/scripts/correct_error_ids.py old mode 100644 new mode 100755 index cce8285de..cbbad8264 --- a/scripts/correct_error_ids.py +++ b/scripts/correct_error_ids.py @@ -1,6 +1,9 @@ +#! /usr/bin/env python3 import random import re import os +import getopt +import sys from os import path ENCODING = "utf-8" @@ -115,19 +118,28 @@ def find_source_files(top_dir): return source_file_names -def main(): +def main(argv): + noconfirm = False + opts, args = getopt.getopt(argv, "", ["noconfirm"]) + + for opt, arg in opts: + if opt == '--noconfirm': + noconfirm = True + random.seed() cwd = os.getcwd() - answer = input( - f"This script checks and corrects *_error literals in .h and .cpp files\n" - f"in {cwd}, recursively.\n\n" - f"Please commit current changes first, and review the results when the script finishes.\n\n" - f"Do you want to start [Y/N]? " - ) - while len(answer) == 0 or answer not in "YNyn": - answer = input("[Y/N]? ") - if answer not in "yY": - return + + if not noconfirm: + answer = input( + f"This script checks and corrects *_error literals in .h and .cpp files\n" + f"in {cwd}, recursively.\n\n" + f"Please commit current changes first, and review the results when the script finishes.\n\n" + f"Do you want to start [Y/N]? " + ) + while len(answer) == 0 or answer not in "YNyn": + answer = input("[Y/N]? ") + if answer not in "yY": + exit(0) source_file_names = find_source_files(cwd) @@ -147,10 +159,12 @@ def main(): if ok: print("No incorrect IDs found") + exit(0) else: fix_ids(used_ids, source_file_names) print("Fixing completed") + exit(1) if __name__ == "__main__": - main() + main(sys.argv[1:]) From eb923af09cd0b1d77d63cd242641612969bffeab Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 26 May 2020 17:24:52 +0200 Subject: [PATCH 03/36] Do not set source locations for small internal assembly routines. --- Changelog.md | 1 + libevmasm/Assembly.h | 1 + liblangutil/ParserBase.h | 2 +- libsolidity/codegen/CompilerContext.cpp | 7 +++- libyul/AsmParser.h | 17 ++++++++-- .../optimizer_BlockDeDuplicator/output | 16 --------- test/cmdlineTests/optimizer_user_yul/output | 3 -- .../standard_immutable_references/output.json | 2 +- test/libsolidity/Assembly.cpp | 28 +++------------ test/libsolidity/StandardCompiler.cpp | 34 +++++++++---------- 10 files changed, 46 insertions(+), 65 deletions(-) diff --git a/Changelog.md b/Changelog.md index d19d08be2..ca31a9700 100644 --- a/Changelog.md +++ b/Changelog.md @@ -6,6 +6,7 @@ Language Features: Compiler Features: + * Code Generator: Do not introduce new source references for small internal routines. * Build system: Update the soljson.js build to emscripten 1.39.15 and boost 1.73.0 and include Z3 for integrated SMTChecker support without the callback mechanism. * SMTChecker: Support array ``length``. * SMTChecker: Support array ``push`` and ``pop``. diff --git a/libevmasm/Assembly.h b/libevmasm/Assembly.h index 27d825268..e82fee7dd 100644 --- a/libevmasm/Assembly.h +++ b/libevmasm/Assembly.h @@ -97,6 +97,7 @@ public: /// Changes the source location used for each appended item. void setSourceLocation(langutil::SourceLocation const& _location) { m_currentSourceLocation = _location; } + langutil::SourceLocation const& currentSourceLocation() const { return m_currentSourceLocation; } /// Assembles the assembly into bytecode. The assembly should not be modified after this call, since the assembled version is cached. LinkerObject const& assemble() const; diff --git a/liblangutil/ParserBase.h b/liblangutil/ParserBase.h index bb4123b9f..0f6146005 100644 --- a/liblangutil/ParserBase.h +++ b/liblangutil/ParserBase.h @@ -64,7 +64,7 @@ protected: }; /// Location of the current token - SourceLocation currentLocation() const; + virtual SourceLocation currentLocation() const; ///@{ ///@name Helper functions diff --git a/libsolidity/codegen/CompilerContext.cpp b/libsolidity/codegen/CompilerContext.cpp index 9431ecec6..9e2c0663b 100644 --- a/libsolidity/codegen/CompilerContext.cpp +++ b/libsolidity/codegen/CompilerContext.cpp @@ -422,7 +422,12 @@ void CompilerContext::appendInlineAssembly( ErrorReporter errorReporter(errors); auto scanner = make_shared(langutil::CharStream(_assembly, "--CODEGEN--")); yul::EVMDialect const& dialect = yul::EVMDialect::strictAssemblyForEVM(m_evmVersion); - shared_ptr parserResult = yul::Parser(errorReporter, dialect).parse(scanner, false); + optional locationOverride; + if (!_system) + locationOverride = m_asm->currentSourceLocation(); + shared_ptr parserResult = + yul::Parser(errorReporter, dialect, std::move(locationOverride)) + .parse(scanner, false); #ifdef SOL_OUTPUT_ASM cout << yul::AsmPrinter(&dialect)(*parserResult) << endl; #endif diff --git a/libyul/AsmParser.h b/libyul/AsmParser.h index 54c79a0e7..3c70abd7e 100644 --- a/libyul/AsmParser.h +++ b/libyul/AsmParser.h @@ -46,8 +46,15 @@ public: None, ForLoopPre, ForLoopPost, ForLoopBody }; - explicit Parser(langutil::ErrorReporter& _errorReporter, Dialect const& _dialect): - ParserBase(_errorReporter), m_dialect(_dialect) {} + explicit Parser( + langutil::ErrorReporter& _errorReporter, + Dialect const& _dialect, + std::optional _locationOverride = {} + ): + ParserBase(_errorReporter), + m_dialect(_dialect), + m_locationOverride(std::move(_locationOverride)) + {} /// Parses an inline assembly block starting with `{` and ending with `}`. /// @param _reuseScanner if true, do check for end of input after the `}`. @@ -60,6 +67,11 @@ public: protected: using ElementaryOperation = std::variant; + langutil::SourceLocation currentLocation() const override + { + return m_locationOverride ? *m_locationOverride : ParserBase::currentLocation(); + } + /// Creates an inline assembly node with the current source location. template T createWithLocation() const { @@ -91,6 +103,7 @@ protected: private: Dialect const& m_dialect; + std::optional m_locationOverride; ForLoopComponent m_currentForLoopComponent = ForLoopComponent::None; bool m_insideFunction = false; }; diff --git a/test/cmdlineTests/optimizer_BlockDeDuplicator/output b/test/cmdlineTests/optimizer_BlockDeDuplicator/output index 6f11f97a4..e5c45e53e 100644 --- a/test/cmdlineTests/optimizer_BlockDeDuplicator/output +++ b/test/cmdlineTests/optimizer_BlockDeDuplicator/output @@ -19,20 +19,14 @@ EVM assembly: sstore /* "optimizer_BlockDeDuplicator/input.sol":60:213 contract C {... */ callvalue - /* "--CODEGEN--":2:4 */ dup1 iszero tag_5 jumpi - /* "--CODEGEN--":27:28 */ 0x00 - /* "--CODEGEN--":24:25 */ dup1 - /* "--CODEGEN--":17:29 */ revert - /* "--CODEGEN--":2:4 */ tag_5: - /* "optimizer_BlockDeDuplicator/input.sol":60:213 contract C {... */ pop jump(tag_6) /* "optimizer_BlockDeDuplicator/input.sol":77:103 function fun_x() public {} */ @@ -53,21 +47,14 @@ sub_0: assembly { /* "optimizer_BlockDeDuplicator/input.sol":60:213 contract C {... */ mstore(0x40, 0x80) callvalue - /* "--CODEGEN--":5:14 */ dup1 - /* "--CODEGEN--":2:4 */ iszero tag_1 jumpi - /* "--CODEGEN--":27:28 */ 0x00 - /* "--CODEGEN--":24:25 */ dup1 - /* "--CODEGEN--":17:29 */ revert - /* "--CODEGEN--":2:4 */ tag_1: - /* "optimizer_BlockDeDuplicator/input.sol":60:213 contract C {... */ pop jumpi(tag_2, lt(calldatasize, 0x04)) shr(0xe0, calldataload(0x00)) @@ -87,11 +74,8 @@ sub_0: assembly { tag_3 jumpi tag_2: - /* "--CODEGEN--":12:13 */ 0x00 - /* "--CODEGEN--":9:10 */ dup1 - /* "--CODEGEN--":2:14 */ revert /* "optimizer_BlockDeDuplicator/input.sol":138:174 function f() public { true ? 1 : 3;} */ tag_3: diff --git a/test/cmdlineTests/optimizer_user_yul/output b/test/cmdlineTests/optimizer_user_yul/output index a160c37c4..340d70914 100644 --- a/test/cmdlineTests/optimizer_user_yul/output +++ b/test/cmdlineTests/optimizer_user_yul/output @@ -76,11 +76,8 @@ stop sub_0: assembly { /* "optimizer_user_yul/input.sol":60:525 contract C... */ mstore(0x40, 0x80) - /* "--CODEGEN--":12:13 */ 0x00 - /* "--CODEGEN--":9:10 */ dup1 - /* "--CODEGEN--":2:14 */ revert auxdata: AUXDATA REMOVED diff --git a/test/cmdlineTests/standard_immutable_references/output.json b/test/cmdlineTests/standard_immutable_references/output.json index edb5d74d0..f39a67c07 100644 --- a/test/cmdlineTests/standard_immutable_references/output.json +++ b/test/cmdlineTests/standard_immutable_references/output.json @@ -1,2 +1,2 @@ -{"contracts":{"a.sol":{"A":{"evm":{"deployedBytecode":{"immutableReferences":{"3":[{"length":32,"start":77}]},"linkReferences":{},"object":"bytecode removed","opcodes":"opcodes removed","sourceMap":"36:96:0:-:0;;;;5:9:-1;2:2;;;27:1;24;17:12;2:2;36:96:0;;;;;;;;;;;;;;;;12:1:-1;9;2:12;74:56:0;;;:::i;:::-;;;;;;;;;;;;;;;;;;;;108:7;126:1;119:8;;74:56;:::o"}}}}},"errors":[{"component":"general","formattedMessage":"a.sol: Warning: Source file does not specify required compiler version! +{"contracts":{"a.sol":{"A":{"evm":{"deployedBytecode":{"immutableReferences":{"3":[{"length":32,"start":77}]},"linkReferences":{},"object":"bytecode removed","opcodes":"opcodes removed","sourceMap":"36:96:0:-:0;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;74:56;;;:::i;:::-;;;;;;;;;;;;;;;;;;;;108:7;126:1;119:8;;74:56;:::o"}}}}},"errors":[{"component":"general","formattedMessage":"a.sol: Warning: Source file does not specify required compiler version! ","message":"Source file does not specify required compiler version!","severity":"warning","sourceLocation":{"end":-1,"file":"a.sol","start":-1},"type":"Warning"}],"sources":{"a.sol":{"id":0}}} diff --git a/test/libsolidity/Assembly.cpp b/test/libsolidity/Assembly.cpp index 898de1427..0484bf95c 100644 --- a/test/libsolidity/Assembly.cpp +++ b/test/libsolidity/Assembly.cpp @@ -107,13 +107,13 @@ void printAssemblyLocations(AssemblyItems const& _items) cout << "\t\tvector(" << _repetitions << - ", SourceLocation(" << + ", SourceLocation{" << _loc.start << ", " << _loc.end << ", make_shared(\"" << _loc.source->name() << - "\"))) +" << endl; + "\")}) +" << endl; }; vector locations; @@ -175,33 +175,13 @@ BOOST_AUTO_TEST_CASE(location_test) vector locations; if (solidity::test::CommonOptions::get().optimize) locations = - vector(4, SourceLocation{2, 82, sourceCode}) + - vector(1, SourceLocation{5, 14, codegenCharStream}) + - vector(3, SourceLocation{2, 4, codegenCharStream}) + - vector(1, SourceLocation{27, 28, codegenCharStream}) + - vector(1, SourceLocation{24, 25, codegenCharStream}) + - vector(1, SourceLocation{17, 29, codegenCharStream}) + - vector(1, SourceLocation{2, 4, codegenCharStream}) + - vector(16, SourceLocation{2, 82, sourceCode}) + - vector(1, SourceLocation{12, 13, codegenCharStream}) + - vector(1, SourceLocation{9, 10, codegenCharStream}) + - vector(1, SourceLocation{2, 14, codegenCharStream}) + + vector(31, SourceLocation{2, 82, sourceCode}) + vector(21, SourceLocation{20, 79, sourceCode}) + vector(1, SourceLocation{72, 74, sourceCode}) + vector(2, SourceLocation{20, 79, sourceCode}); else locations = - vector(4, SourceLocation{2, 82, sourceCode}) + - vector(1, SourceLocation{5, 14, codegenCharStream}) + - vector(3, SourceLocation{2, 4, codegenCharStream}) + - vector(1, SourceLocation{27, 28, codegenCharStream}) + - vector(1, SourceLocation{24, 25, codegenCharStream}) + - vector(1, SourceLocation{17, 29, codegenCharStream}) + - vector(1, SourceLocation{2, 4, codegenCharStream}) + - vector(hasShifts ? 16 : 17, SourceLocation{2, 82, sourceCode}) + - vector(1, SourceLocation{12, 13, codegenCharStream}) + - vector(1, SourceLocation{9, 10, codegenCharStream}) + - vector(1, SourceLocation{2, 14, codegenCharStream}) + + vector(hasShifts ? 31 : 32, SourceLocation{2, 82, sourceCode}) + vector(24, SourceLocation{20, 79, sourceCode}) + vector(1, SourceLocation{49, 58, sourceCode}) + vector(1, SourceLocation{72, 74, sourceCode}) + diff --git a/test/libsolidity/StandardCompiler.cpp b/test/libsolidity/StandardCompiler.cpp index 5f4a4e3d4..3b7b1d128 100644 --- a/test/libsolidity/StandardCompiler.cpp +++ b/test/libsolidity/StandardCompiler.cpp @@ -370,15 +370,15 @@ BOOST_AUTO_TEST_CASE(basic_compilation) BOOST_CHECK(contract["evm"]["assembly"].isString()); BOOST_CHECK(contract["evm"]["assembly"].asString().find( " /* \"fileA\":0:14 contract A { } */\n mstore(0x40, 0x80)\n " - "callvalue\n /* \"--CODEGEN--\":5:14 */\n dup1\n " - "/* \"--CODEGEN--\":2:4 */\n iszero\n tag_1\n jumpi\n " - "/* \"--CODEGEN--\":27:28 */\n 0x00\n /* \"--CODEGEN--\":24:25 */\n " - "dup1\n /* \"--CODEGEN--\":17:29 */\n revert\n /* \"--CODEGEN--\":2:4 */\n" - "tag_1:\n /* \"fileA\":0:14 contract A { } */\n pop\n dataSize(sub_0)\n dup1\n " + "callvalue\n dup1\n " + "iszero\n tag_1\n jumpi\n " + "0x00\n " + "dup1\n revert\n" + "tag_1:\n pop\n dataSize(sub_0)\n dup1\n " "dataOffset(sub_0)\n 0x00\n codecopy\n 0x00\n return\nstop\n\nsub_0: assembly {\n " - "/* \"fileA\":0:14 contract A { } */\n mstore(0x40, 0x80)\n " - "/* \"--CODEGEN--\":12:13 */\n 0x00\n /* \"--CODEGEN--\":9:10 */\n " - "dup1\n /* \"--CODEGEN--\":2:14 */\n revert\n\n auxdata: 0xa26469706673582212" + "/* \"fileA\":0:14 contract A { } */\n mstore(0x40, 0x80)\n " + "0x00\n " + "dup1\n revert\n\n auxdata: 0xa26469706673582212" ) == 0); BOOST_CHECK(contract["evm"]["gasEstimates"].isObject()); BOOST_CHECK_EQUAL(contract["evm"]["gasEstimates"].size(), 1); @@ -402,15 +402,15 @@ BOOST_AUTO_TEST_CASE(basic_compilation) "{\"begin\":0,\"end\":14,\"name\":\"PUSH\",\"source\":0,\"value\":\"40\"}," "{\"begin\":0,\"end\":14,\"name\":\"MSTORE\",\"source\":0}," "{\"begin\":0,\"end\":14,\"name\":\"CALLVALUE\",\"source\":0}," - "{\"begin\":5,\"end\":14,\"name\":\"DUP1\",\"source\":-1}," - "{\"begin\":2,\"end\":4,\"name\":\"ISZERO\",\"source\":-1}," - "{\"begin\":2,\"end\":4,\"name\":\"PUSH [tag]\",\"source\":-1,\"value\":\"1\"}," - "{\"begin\":2,\"end\":4,\"name\":\"JUMPI\",\"source\":-1}," - "{\"begin\":27,\"end\":28,\"name\":\"PUSH\",\"source\":-1,\"value\":\"0\"}," - "{\"begin\":24,\"end\":25,\"name\":\"DUP1\",\"source\":-1}," - "{\"begin\":17,\"end\":29,\"name\":\"REVERT\",\"source\":-1}," - "{\"begin\":2,\"end\":4,\"name\":\"tag\",\"source\":-1,\"value\":\"1\"}," - "{\"begin\":2,\"end\":4,\"name\":\"JUMPDEST\",\"source\":-1}," + "{\"begin\":0,\"end\":14,\"name\":\"DUP1\",\"source\":0}," + "{\"begin\":0,\"end\":14,\"name\":\"ISZERO\",\"source\":0}," + "{\"begin\":0,\"end\":14,\"name\":\"PUSH [tag]\",\"source\":0,\"value\":\"1\"}," + "{\"begin\":0,\"end\":14,\"name\":\"JUMPI\",\"source\":0}," + "{\"begin\":0,\"end\":14,\"name\":\"PUSH\",\"source\":0,\"value\":\"0\"}," + "{\"begin\":0,\"end\":14,\"name\":\"DUP1\",\"source\":0}," + "{\"begin\":0,\"end\":14,\"name\":\"REVERT\",\"source\":0}," + "{\"begin\":0,\"end\":14,\"name\":\"tag\",\"source\":0,\"value\":\"1\"}," + "{\"begin\":0,\"end\":14,\"name\":\"JUMPDEST\",\"source\":0}," "{\"begin\":0,\"end\":14,\"name\":\"POP\",\"source\":0}," "{\"begin\":0,\"end\":14,\"name\":\"PUSH #[$]\",\"source\":0,\"value\":\"0000000000000000000000000000000000000000000000000000000000000000\"}," "{\"begin\":0,\"end\":14,\"name\":\"DUP1\",\"source\":0}," From a846c18e67cbd1e5e4bf2d153f01f66e894cbb4e Mon Sep 17 00:00:00 2001 From: Mathias Baumann Date: Wed, 27 May 2020 18:22:04 +0200 Subject: [PATCH 04/36] Fix failure to find overload resolution when overrides are involved --- Changelog.md | 1 + libsolidity/analysis/TypeChecker.cpp | 10 +++++----- .../inheritance/override/no_matching_resolution.sol | 11 +++++++++++ 3 files changed, 17 insertions(+), 5 deletions(-) create mode 100644 test/libsolidity/syntaxTests/inheritance/override/no_matching_resolution.sol diff --git a/Changelog.md b/Changelog.md index d19d08be2..36ac87428 100644 --- a/Changelog.md +++ b/Changelog.md @@ -17,6 +17,7 @@ Bugfixes: * Type Checker: Disallow inline arrays of non-nameable types. * Type Checker: Fix internal compiler error when accessing members of array slices. * Type Checker: Fix internal compiler error when trying to decode too large static arrays. + * Type Checker: Fix wrong compiler error when referencing an overridden function without calling it. * NatSpec: DocString block is terminated when encountering an empty line. * Scanner: Fix bug when two empty NatSpec comments lead to scanning past EOL. * Code Generator: Trigger proper unimplemented errors on certain array copy operations. diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 7681efc18..e6694043c 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -2981,7 +2981,11 @@ bool TypeChecker::visit(Identifier const& _identifier) if (!annotation.referencedDeclaration) { annotation.overloadedDeclarations = cleanOverloadedDeclarations(_identifier, annotation.candidateDeclarations); - if (!annotation.arguments) + if (annotation.overloadedDeclarations.empty()) + m_errorReporter.fatalTypeError(7593_error, _identifier.location(), "No candidates for overload resolution found."); + else if (annotation.overloadedDeclarations.size() == 1) + annotation.referencedDeclaration = *annotation.overloadedDeclarations.begin(); + else if (!annotation.arguments) { // The identifier should be a public state variable shadowing other functions vector candidates; @@ -2998,10 +3002,6 @@ bool TypeChecker::visit(Identifier const& _identifier) else m_errorReporter.fatalTypeError(7589_error, _identifier.location(), "No unique declaration found after variable lookup."); } - else if (annotation.overloadedDeclarations.empty()) - m_errorReporter.fatalTypeError(7593_error, _identifier.location(), "No candidates for overload resolution found."); - else if (annotation.overloadedDeclarations.size() == 1) - annotation.referencedDeclaration = *annotation.overloadedDeclarations.begin(); else { vector candidates; diff --git a/test/libsolidity/syntaxTests/inheritance/override/no_matching_resolution.sol b/test/libsolidity/syntaxTests/inheritance/override/no_matching_resolution.sol new file mode 100644 index 000000000..870f2b736 --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/override/no_matching_resolution.sol @@ -0,0 +1,11 @@ +contract A { + function f() virtual internal {} +} +contract B is A { + function f() virtual override internal {} + function h() pure internal { f; } +} +contract C is B { + function f() override internal {} + function i() pure internal { f; } +} From 9e9f0c52e1f300358efe6df77f7179b7a0188de7 Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Wed, 13 May 2020 13:08:48 +0200 Subject: [PATCH 05/36] [SMTChecker] Support to bitwise --- Changelog.md | 2 + libsmtutil/CVC4Interface.cpp | 45 +++++++++++++++++++ libsmtutil/SMTLib2Interface.cpp | 37 ++++++++++++++- libsmtutil/SolverInterface.h | 34 ++++++++++++++ libsmtutil/Sorts.cpp | 10 ++++- libsmtutil/Sorts.h | 35 ++++++++++++++- libsmtutil/Z3Interface.cpp | 13 ++++++ libsolidity/formal/CHC.cpp | 6 +-- libsolidity/formal/SMTEncoder.cpp | 39 ++++++++++++++++ libsolidity/formal/SMTEncoder.h | 1 + libsolidity/formal/SymbolicState.h | 2 +- libsolidity/formal/SymbolicTypes.cpp | 20 +++++---- libsolidity/formal/SymbolicVariables.cpp | 2 +- .../operators/bitwise_and_int.sol | 18 ++++++++ .../operators/bitwise_and_rational.sol | 12 +++++ .../operators/bitwise_and_uint.sol | 18 ++++++++ 16 files changed, 277 insertions(+), 17 deletions(-) create mode 100644 test/libsolidity/smtCheckerTests/operators/bitwise_and_int.sol create mode 100644 test/libsolidity/smtCheckerTests/operators/bitwise_and_rational.sol create mode 100644 test/libsolidity/smtCheckerTests/operators/bitwise_and_uint.sol diff --git a/Changelog.md b/Changelog.md index d19d08be2..40b4765cd 100644 --- a/Changelog.md +++ b/Changelog.md @@ -9,8 +9,10 @@ Compiler Features: * Build system: Update the soljson.js build to emscripten 1.39.15 and boost 1.73.0 and include Z3 for integrated SMTChecker support without the callback mechanism. * SMTChecker: Support array ``length``. * SMTChecker: Support array ``push`` and ``pop``. + * SMTChecker: General support to BitVectors and the bitwise ``and`` operator. * Add support for natspec comments on state variables. + Bugfixes: * Optimizer: Fixed a bug in BlockDeDuplicator. * Type Checker: Disallow assignments to storage variables of type ``mapping``. diff --git a/libsmtutil/CVC4Interface.cpp b/libsmtutil/CVC4Interface.cpp index 4c1ef378a..87184714f 100644 --- a/libsmtutil/CVC4Interface.cpp +++ b/libsmtutil/CVC4Interface.cpp @@ -19,6 +19,8 @@ #include +#include + using namespace std; using namespace solidity; using namespace solidity::util; @@ -185,6 +187,49 @@ CVC4::Expr CVC4Interface::toCVC4Expr(Expression const& _expr) return m_context.mkExpr(CVC4::kind::INTS_DIVISION_TOTAL, arguments[0], arguments[1]); else if (n == "mod") return m_context.mkExpr(CVC4::kind::INTS_MODULUS, arguments[0], arguments[1]); + else if (n == "bvand") + return m_context.mkExpr(CVC4::kind::BITVECTOR_AND, arguments[0], arguments[1]); + else if (n == "int2bv") + { + size_t size = std::stoi(_expr.arguments[1].name); + auto i2bvOp = m_context.mkConst(CVC4::IntToBitVector(size)); + // CVC4 treats all BVs as unsigned, so we need to manually apply 2's complement if needed. + return m_context.mkExpr( + CVC4::kind::ITE, + m_context.mkExpr(CVC4::kind::GEQ, arguments[0], m_context.mkConst(CVC4::Rational(0))), + m_context.mkExpr(CVC4::kind::INT_TO_BITVECTOR, i2bvOp, arguments[0]), + m_context.mkExpr( + CVC4::kind::BITVECTOR_NEG, + m_context.mkExpr(CVC4::kind::INT_TO_BITVECTOR, i2bvOp, m_context.mkExpr(CVC4::kind::UMINUS, arguments[0])) + ) + ); + } + else if (n == "bv2int") + { + auto intSort = dynamic_pointer_cast(_expr.sort); + smtAssert(intSort, ""); + auto nat = m_context.mkExpr(CVC4::kind::BITVECTOR_TO_NAT, arguments[0]); + if (!intSort->isSigned) + return nat; + + auto type = arguments[0].getType(); + smtAssert(type.isBitVector(), ""); + auto size = CVC4::BitVectorType(type).getSize(); + // CVC4 treats all BVs as unsigned, so we need to manually apply 2's complement if needed. + auto extractOp = m_context.mkConst(CVC4::BitVectorExtract(size - 1, size - 1)); + return m_context.mkExpr(CVC4::kind::ITE, + m_context.mkExpr( + CVC4::kind::EQUAL, + m_context.mkExpr(CVC4::kind::BITVECTOR_EXTRACT, extractOp, arguments[0]), + m_context.mkConst(CVC4::BitVector(1, size_t(0))) + ), + nat, + m_context.mkExpr( + CVC4::kind::UMINUS, + m_context.mkExpr(CVC4::kind::BITVECTOR_TO_NAT, m_context.mkExpr(CVC4::kind::BITVECTOR_NEG, arguments[0])) + ) + ); + } else if (n == "select") return m_context.mkExpr(CVC4::kind::SELECT, arguments[0], arguments[1]); else if (n == "store") diff --git a/libsmtutil/SMTLib2Interface.cpp b/libsmtutil/SMTLib2Interface.cpp index 4e81444fe..bc37ffb6c 100644 --- a/libsmtutil/SMTLib2Interface.cpp +++ b/libsmtutil/SMTLib2Interface.cpp @@ -126,7 +126,7 @@ pair> SMTLib2Interface::check(vector con result = CheckResult::ERROR; vector values; - if (result == CheckResult::SATISFIABLE && result != CheckResult::ERROR) + if (result == CheckResult::SATISFIABLE && !_expressionsToEvaluate.empty()) values = parseValues(find(response.cbegin(), response.cend(), '\n'), response.cend()); return make_pair(result, values); } @@ -137,7 +137,40 @@ string SMTLib2Interface::toSExpr(Expression const& _expr) return _expr.name; std::string sexpr = "("; - if (_expr.name == "const_array") + if (_expr.name == "int2bv") + { + size_t size = std::stoi(_expr.arguments[1].name); + auto arg = toSExpr(_expr.arguments.front()); + auto int2bv = "(_ int2bv " + to_string(size) + ")"; + // Some solvers treat all BVs as unsigned, so we need to manually apply 2's complement if needed. + sexpr += string("ite ") + + "(>= " + arg + " 0) " + + "(" + int2bv + " " + arg + ") " + + "(bvneg (" + int2bv + " (- " + arg + ")))"; + } + else if (_expr.name == "bv2int") + { + auto intSort = dynamic_pointer_cast(_expr.sort); + smtAssert(intSort, ""); + + auto arg = toSExpr(_expr.arguments.front()); + auto nat = "(bv2nat " + arg + ")"; + + if (!intSort->isSigned) + return nat; + + auto bvSort = dynamic_pointer_cast(_expr.arguments.front().sort); + smtAssert(bvSort, ""); + auto size = to_string(bvSort->size); + auto pos = to_string(bvSort->size - 1); + + // Some solvers treat all BVs as unsigned, so we need to manually apply 2's complement if needed. + sexpr += string("ite ") + + "(= ((_ extract " + pos + " " + pos + ")" + arg + ") #b0) " + + nat + " " + + "(- (bvneg " + arg + "))"; + } + else if (_expr.name == "const_array") { smtAssert(_expr.arguments.size() == 2, ""); auto sortSort = std::dynamic_pointer_cast(_expr.arguments.at(0).sort); diff --git a/libsmtutil/SolverInterface.h b/libsmtutil/SolverInterface.h index 13fcae4f2..be12c3a80 100644 --- a/libsmtutil/SolverInterface.h +++ b/libsmtutil/SolverInterface.h @@ -94,6 +94,9 @@ public: {"*", 2}, {"/", 2}, {"mod", 2}, + {"bvand", 2}, + {"int2bv", 2}, + {"bv2int", 1}, {"select", 2}, {"store", 3}, {"const_array", 2}, @@ -195,6 +198,32 @@ public: ); } + static Expression int2bv(Expression _n, size_t _size) + { + smtAssert(_n.sort->kind == Kind::Int, ""); + std::shared_ptr intSort = std::dynamic_pointer_cast(_n.sort); + smtAssert(intSort, ""); + smtAssert(_size <= 256, ""); + return Expression( + "int2bv", + std::vector{std::move(_n), Expression(_size)}, + std::make_shared(_size) + ); + } + + static Expression bv2int(Expression _bv, bool _signed = false) + { + smtAssert(_bv.sort->kind == Kind::BitVector, ""); + std::shared_ptr bvSort = std::dynamic_pointer_cast(_bv.sort); + smtAssert(bvSort, ""); + smtAssert(bvSort->size <= 256, ""); + return Expression( + "bv2int", + std::vector{std::move(_bv)}, + SortProvider::intSort(_signed) + ); + } + friend Expression operator!(Expression _a) { return Expression("not", std::move(_a), Kind::Bool); @@ -251,6 +280,11 @@ public: { return Expression("mod", std::move(_a), std::move(_b), Kind::Int); } + friend Expression operator&(Expression _a, Expression _b) + { + auto bvSort = _a.sort; + return Expression("bvand", {std::move(_a), std::move(_b)}, bvSort); + } Expression operator()(std::vector _arguments) const { smtAssert( diff --git a/libsmtutil/Sorts.cpp b/libsmtutil/Sorts.cpp index 7a45a1746..7624cbb5b 100644 --- a/libsmtutil/Sorts.cpp +++ b/libsmtutil/Sorts.cpp @@ -24,6 +24,14 @@ namespace solidity::smtutil { shared_ptr const SortProvider::boolSort{make_shared(Kind::Bool)}; -shared_ptr const SortProvider::intSort{make_shared(Kind::Int)}; +shared_ptr const SortProvider::uintSort{make_shared(false)}; +shared_ptr const SortProvider::sintSort{make_shared(true)}; + +shared_ptr SortProvider::intSort(bool _signed) +{ + if (_signed) + return sintSort; + return uintSort; +} } diff --git a/libsmtutil/Sorts.h b/libsmtutil/Sorts.h index 228dd057b..20cbf2be5 100644 --- a/libsmtutil/Sorts.h +++ b/libsmtutil/Sorts.h @@ -31,6 +31,7 @@ enum class Kind { Int, Bool, + BitVector, Function, Array, Sort, @@ -48,6 +49,36 @@ struct Sort }; using SortPointer = std::shared_ptr; +struct IntSort: public Sort +{ + IntSort(bool _signed): + Sort(Kind::Int), + isSigned(_signed) + {} + + bool operator==(IntSort const& _other) const + { + return Sort::operator==(_other) && isSigned == _other.isSigned; + } + + bool isSigned; +}; + +struct BitVectorSort: public Sort +{ + BitVectorSort(unsigned _size): + Sort(Kind::BitVector), + size(_size) + {} + + bool operator==(BitVectorSort const& _other) const + { + return Sort::operator==(_other) && size == _other.size; + } + + unsigned size; +}; + struct FunctionSort: public Sort { FunctionSort(std::vector _domain, SortPointer _codomain): @@ -160,7 +191,9 @@ struct TupleSort: public Sort struct SortProvider { static std::shared_ptr const boolSort; - static std::shared_ptr const intSort; + static std::shared_ptr const uintSort; + static std::shared_ptr const sintSort; + static std::shared_ptr intSort(bool _signed = false); }; } diff --git a/libsmtutil/Z3Interface.cpp b/libsmtutil/Z3Interface.cpp index b42bc4765..1337b7b20 100644 --- a/libsmtutil/Z3Interface.cpp +++ b/libsmtutil/Z3Interface.cpp @@ -180,6 +180,19 @@ z3::expr Z3Interface::toZ3Expr(Expression const& _expr) return arguments[0] / arguments[1]; else if (n == "mod") return z3::mod(arguments[0], arguments[1]); + else if (n == "bvand") + return arguments[0] & arguments[1]; + else if (n == "int2bv") + { + size_t size = std::stoi(_expr.arguments[1].name); + return z3::int2bv(size, arguments[0]); + } + else if (n == "bv2int") + { + auto intSort = dynamic_pointer_cast(_expr.sort); + smtAssert(intSort, ""); + return z3::bv2int(arguments[0], intSort->isSigned); + } else if (n == "select") return z3::select(arguments[0], arguments[1]); else if (n == "store") diff --git a/libsolidity/formal/CHC.cpp b/libsolidity/formal/CHC.cpp index fe63c198d..f0ba6d0de 100644 --- a/libsolidity/formal/CHC.cpp +++ b/libsolidity/formal/CHC.cpp @@ -701,7 +701,7 @@ vector CHC::stateSorts(ContractDefinition const& _contract smtutil::SortPointer CHC::constructorSort() { return make_shared( - vector{smtutil::SortProvider::intSort} + m_stateSorts, + vector{smtutil::SortProvider::uintSort} + m_stateSorts, smtutil::SortProvider::boolSort ); } @@ -747,7 +747,7 @@ smtutil::SortPointer CHC::sort(FunctionDefinition const& _function) auto inputSorts = applyMap(_function.parameters(), smtSort); auto outputSorts = applyMap(_function.returnParameters(), smtSort); return make_shared( - vector{smtutil::SortProvider::intSort} + m_stateSorts + inputSorts + m_stateSorts + inputSorts + outputSorts, + vector{smtutil::SortProvider::uintSort} + m_stateSorts + inputSorts + m_stateSorts + inputSorts + outputSorts, smtutil::SortProvider::boolSort ); } @@ -776,7 +776,7 @@ smtutil::SortPointer CHC::summarySort(FunctionDefinition const& _function, Contr auto inputSorts = applyMap(_function.parameters(), smtSort); auto outputSorts = applyMap(_function.returnParameters(), smtSort); return make_shared( - vector{smtutil::SortProvider::intSort} + sorts + inputSorts + sorts + outputSorts, + vector{smtutil::SortProvider::uintSort} + sorts + inputSorts + sorts + outputSorts, smtutil::SortProvider::boolSort ); } diff --git a/libsolidity/formal/SMTEncoder.cpp b/libsolidity/formal/SMTEncoder.cpp index 99aa6195f..258c74bcc 100644 --- a/libsolidity/formal/SMTEncoder.cpp +++ b/libsolidity/formal/SMTEncoder.cpp @@ -574,6 +574,8 @@ void SMTEncoder::endVisit(BinaryOperation const& _op) arithmeticOperation(_op); else if (TokenTraits::isCompareOp(_op.getOperator())) compareOperation(_op); + else if (TokenTraits::isBitOp(_op.getOperator())) + bitwiseOperation(_op); else m_errorReporter.warning( 3876_error, @@ -1346,6 +1348,43 @@ void SMTEncoder::booleanOperation(BinaryOperation const& _op) ); } +void SMTEncoder::bitwiseOperation(BinaryOperation const& _op) +{ + solAssert(TokenTraits::isBitOp(_op.getOperator()), ""); + auto commonType = _op.annotation().commonType; + solAssert(commonType, ""); + + unsigned bvSize = 256; + bool isSigned = false; + if (auto const* intType = dynamic_cast(commonType)) + { + bvSize = intType->numBits(); + isSigned = intType->isSigned(); + } + else if (auto const* fixedType = dynamic_cast(commonType)) + { + bvSize = fixedType->numBits(); + isSigned = fixedType->isSigned(); + } + + auto bvLeft = smtutil::Expression::int2bv(expr(_op.leftExpression()), bvSize); + auto bvRight = smtutil::Expression::int2bv(expr(_op.rightExpression()), bvSize); + + optional result; + if (_op.getOperator() == Token::BitAnd) + result = bvLeft & bvRight; + // TODO implement the other operators + else + m_errorReporter.warning( + 1093_error, + _op.location(), + "Assertion checker does not yet implement this bitwise operator." + ); + + if (result) + defineExpr(_op, smtutil::Expression::bv2int(*result, isSigned)); +} + smtutil::Expression SMTEncoder::division(smtutil::Expression _left, smtutil::Expression _right, IntegerType const& _type) { // Signed division in SMTLIB2 rounds differently for negative division. diff --git a/libsolidity/formal/SMTEncoder.h b/libsolidity/formal/SMTEncoder.h index 63bbd45df..8010ba820 100644 --- a/libsolidity/formal/SMTEncoder.h +++ b/libsolidity/formal/SMTEncoder.h @@ -108,6 +108,7 @@ protected: ); void compareOperation(BinaryOperation const& _op); void booleanOperation(BinaryOperation const& _op); + void bitwiseOperation(BinaryOperation const& _op); void initContract(ContractDefinition const& _contract); void initFunction(FunctionDefinition const& _function); diff --git a/libsolidity/formal/SymbolicState.h b/libsolidity/formal/SymbolicState.h index f33faf9f2..2b234466d 100644 --- a/libsolidity/formal/SymbolicState.h +++ b/libsolidity/formal/SymbolicState.h @@ -63,7 +63,7 @@ private: /// Symbolic balances. SymbolicArrayVariable m_balances{ - std::make_shared(smtutil::SortProvider::intSort, smtutil::SortProvider::intSort), + std::make_shared(smtutil::SortProvider::uintSort, smtutil::SortProvider::uintSort), "balances", m_context }; diff --git a/libsolidity/formal/SymbolicTypes.cpp b/libsolidity/formal/SymbolicTypes.cpp index 38f72cb66..0ed81ee31 100644 --- a/libsolidity/formal/SymbolicTypes.cpp +++ b/libsolidity/formal/SymbolicTypes.cpp @@ -34,7 +34,11 @@ SortPointer smtSort(frontend::Type const& _type) switch (smtKind(_type.category())) { case Kind::Int: - return SortProvider::intSort; + if (auto const* intType = dynamic_cast(&_type)) + return SortProvider::intSort(intType->isSigned()); + if (auto const* fixedType = dynamic_cast(&_type)) + return SortProvider::intSort(fixedType->isSigned()); + return SortProvider::uintSort; case Kind::Bool: return SortProvider::boolSort; case Kind::Function: @@ -50,7 +54,7 @@ SortPointer smtSort(frontend::Type const& _type) returnSort = SortProvider::boolSort; else if (returnTypes.size() > 1) // Abstract sort. - returnSort = SortProvider::intSort; + returnSort = SortProvider::uintSort; else returnSort = smtSort(*returnTypes.front()); return make_shared(parameterSorts, returnSort); @@ -68,7 +72,7 @@ SortPointer smtSort(frontend::Type const& _type) { auto stringLitType = dynamic_cast(&_type); solAssert(stringLitType, ""); - array = make_shared(SortProvider::intSort, SortProvider::intSort); + array = make_shared(SortProvider::uintSort, SortProvider::uintSort); } else { @@ -81,7 +85,7 @@ SortPointer smtSort(frontend::Type const& _type) solAssert(false, ""); solAssert(arrayType, ""); - array = make_shared(SortProvider::intSort, smtSortAbstractFunction(*arrayType->baseType())); + array = make_shared(SortProvider::uintSort, smtSortAbstractFunction(*arrayType->baseType())); } string tupleName; @@ -98,7 +102,7 @@ SortPointer smtSort(frontend::Type const& _type) return make_shared( tupleName, vector{tupleName + "_accessor_array", tupleName + "_accessor_length"}, - vector{array, SortProvider::intSort} + vector{array, SortProvider::uintSort} ); } case Kind::Tuple: @@ -118,7 +122,7 @@ SortPointer smtSort(frontend::Type const& _type) } default: // Abstract case. - return SortProvider::intSort; + return SortProvider::uintSort; } } @@ -133,7 +137,7 @@ vector smtSort(vector const& _types) SortPointer smtSortAbstractFunction(frontend::Type const& _type) { if (isFunction(_type.category())) - return SortProvider::intSort; + return SortProvider::uintSort; return smtSort(_type); } @@ -144,7 +148,7 @@ vector smtSortAbstractFunction(vector const& if (type) sorts.push_back(smtSortAbstractFunction(*type)); else - sorts.push_back(SortProvider::intSort); + sorts.push_back(SortProvider::uintSort); return sorts; } diff --git a/libsolidity/formal/SymbolicVariables.cpp b/libsolidity/formal/SymbolicVariables.cpp index 558e7ccd1..002df6002 100644 --- a/libsolidity/formal/SymbolicVariables.cpp +++ b/libsolidity/formal/SymbolicVariables.cpp @@ -286,7 +286,7 @@ SymbolicArrayVariable::SymbolicArrayVariable( std::make_shared( "array_length_pair", std::vector{"array", "length"}, - std::vector{m_sort, SortProvider::intSort} + std::vector{m_sort, SortProvider::uintSort} ), m_uniqueName + "_array_length_pair", m_context diff --git a/test/libsolidity/smtCheckerTests/operators/bitwise_and_int.sol b/test/libsolidity/smtCheckerTests/operators/bitwise_and_int.sol new file mode 100644 index 000000000..579702632 --- /dev/null +++ b/test/libsolidity/smtCheckerTests/operators/bitwise_and_int.sol @@ -0,0 +1,18 @@ +pragma experimental SMTChecker; + +contract C { + function f() public pure { + int8 x = 1; + int8 y = 0; + assert(x & y != 0); + x = -1; y = 3; + assert(x & y == 3); + y = -1; + int8 z = x & y; + assert(z == -1); + y = 127; + assert(x & y == 127); + } +} +// ---- +// Warning: (104-122): Assertion violation happens here diff --git a/test/libsolidity/smtCheckerTests/operators/bitwise_and_rational.sol b/test/libsolidity/smtCheckerTests/operators/bitwise_and_rational.sol new file mode 100644 index 000000000..4efdafdeb --- /dev/null +++ b/test/libsolidity/smtCheckerTests/operators/bitwise_and_rational.sol @@ -0,0 +1,12 @@ +pragma experimental SMTChecker; + +contract C { + function f() public pure { + assert(1 & 0 != 0); + assert(-1 & 3 == 3); + assert(-1 & -1 == -1); + assert(-1 & 127 == 127); + } +} +// ---- +// Warning: (76-94): Assertion violation happens here diff --git a/test/libsolidity/smtCheckerTests/operators/bitwise_and_uint.sol b/test/libsolidity/smtCheckerTests/operators/bitwise_and_uint.sol new file mode 100644 index 000000000..ce8400dbe --- /dev/null +++ b/test/libsolidity/smtCheckerTests/operators/bitwise_and_uint.sol @@ -0,0 +1,18 @@ +pragma experimental SMTChecker; + +contract C { + function f() public pure { + uint8 x = 1; + uint16 y = 0; + assert(x & y != 0); + x = 0xff; + y = 0xffff; + assert(x & y == 0xff); + assert(x & y == 0xffff); + assert(x & y == 0x0000); + } +} +// ---- +// Warning: (107-125): Assertion violation happens here +// Warning: (180-203): Assertion violation happens here +// Warning: (207-230): Assertion violation happens here From 011f8a462d718f3034e72025b1d3d3916faa474d Mon Sep 17 00:00:00 2001 From: a3d4 Date: Thu, 28 May 2020 02:02:53 +0200 Subject: [PATCH 06/36] Remove AsmAnalyzer class-specific error reporting functions --- libyul/AsmAnalysis.cpp | 84 ++++++++++++++++++++++++------------------ libyul/AsmAnalysis.h | 3 -- 2 files changed, 49 insertions(+), 38 deletions(-) diff --git a/libyul/AsmAnalysis.cpp b/libyul/AsmAnalysis.cpp index fd8e44dfa..7a8037760 100644 --- a/libyul/AsmAnalysis.cpp +++ b/libyul/AsmAnalysis.cpp @@ -81,17 +81,19 @@ vector AsmAnalyzer::operator()(Literal const& _literal) { expectValidType(_literal.type, _literal.location); if (_literal.kind == LiteralKind::String && _literal.value.str().size() > 32) - typeError( + m_errorReporter.typeError( + 3069_error, _literal.location, "String literal too long (" + to_string(_literal.value.str().size()) + " > 32)" ); else if (_literal.kind == LiteralKind::Number && bigint(_literal.value.str()) > u256(-1)) - typeError(_literal.location, "Number literal too large (> 256 bits)"); + m_errorReporter.typeError(6708_error, _literal.location, "Number literal too large (> 256 bits)"); else if (_literal.kind == LiteralKind::Boolean) yulAssert(_literal.value == "true"_yulstring || _literal.value == "false"_yulstring, ""); if (!m_dialect.validTypeForLiteral(_literal.kind, _literal.value, _literal.type)) - typeError( + m_errorReporter.typeError( + 5170_error, _literal.location, "Invalid type \"" + _literal.type.str() + "\" for literal \"" + _literal.value.str() + "\"." ); @@ -110,7 +112,8 @@ vector AsmAnalyzer::operator()(Identifier const& _identifier) [&](Scope::Variable const& _var) { if (!m_activeVariables.count(&_var)) - declarationError( + m_errorReporter.declarationError( + 4990_error, _identifier.location, "Variable " + _identifier.name.str() + " used before it was declared." ); @@ -118,7 +121,8 @@ vector AsmAnalyzer::operator()(Identifier const& _identifier) }, [&](Scope::Function const&) { - typeError( + m_errorReporter.typeError( + 6041_error, _identifier.location, "Function " + _identifier.name.str() + " used without being called." ); @@ -141,7 +145,7 @@ vector AsmAnalyzer::operator()(Identifier const& _identifier) } if (!found && watcher.ok()) // Only add an error message if the callback did not do it. - declarationError(_identifier.location, "Identifier not found."); + m_errorReporter.declarationError(8198_error, _identifier.location, "Identifier not found."); } return {type}; @@ -152,7 +156,9 @@ void AsmAnalyzer::operator()(ExpressionStatement const& _statement) auto watcher = m_errorReporter.errorWatcher(); vector types = std::visit(*this, _statement.expression); if (watcher.ok() && !types.empty()) - typeError(_statement.location, + m_errorReporter.typeError( + 3083_error, + _statement.location, "Top-level expressions are not supposed to return values (this expression returns " + to_string(types.size()) + " value" + @@ -170,7 +176,8 @@ void AsmAnalyzer::operator()(Assignment const& _assignment) vector types = std::visit(*this, *_assignment.value); if (types.size() != numVariables) - declarationError( + m_errorReporter.declarationError( + 8678_error, _assignment.location, "Variable count does not match number of values (" + to_string(numVariables) + @@ -202,7 +209,9 @@ void AsmAnalyzer::operator()(VariableDeclaration const& _varDecl) { vector types = std::visit(*this, *_varDecl.value); if (types.size() != numVariables) - declarationError(_varDecl.location, + m_errorReporter.declarationError( + 3812_error, + _varDecl.location, "Variable count mismatch: " + to_string(numVariables) + " variables and " + @@ -217,7 +226,8 @@ void AsmAnalyzer::operator()(VariableDeclaration const& _varDecl) givenType = types[i]; TypedName const& variable = _varDecl.variables[i]; if (variable.type != givenType) - typeError( + m_errorReporter.typeError( + 3947_error, variable.location, "Assigning value of type \"" + givenType.str() + "\" to variable of type \"" + variable.type.str() + "." ); @@ -265,7 +275,8 @@ vector AsmAnalyzer::operator()(FunctionCall const& _funCall) else if (!m_currentScope->lookup(_funCall.functionName.name, GenericVisitor{ [&](Scope::Variable const&) { - typeError( + m_errorReporter.typeError( + 4202_error, _funCall.functionName.location, "Attempt to call variable instead of function." ); @@ -278,12 +289,13 @@ vector AsmAnalyzer::operator()(FunctionCall const& _funCall) })) { if (!warnOnInstructions(_funCall)) - declarationError(_funCall.functionName.location, "Function not found."); + m_errorReporter.declarationError(4619_error, _funCall.functionName.location, "Function not found."); yulAssert(!watcher.ok(), "Expected a reported error."); } if (parameterTypes && _funCall.arguments.size() != parameterTypes->size()) - typeError( + m_errorReporter.typeError( + 7000_error, _funCall.functionName.location, "Function expects " + to_string(parameterTypes->size()) + @@ -301,7 +313,8 @@ vector AsmAnalyzer::operator()(FunctionCall const& _funCall) if (needsLiteralArguments && (*needsLiteralArguments)[i - 1]) { if (!holds_alternative(arg)) - typeError( + m_errorReporter.typeError( + 9114_error, _funCall.functionName.location, "Function expects direct literals as arguments." ); @@ -310,7 +323,8 @@ vector AsmAnalyzer::operator()(FunctionCall const& _funCall) _funCall.functionName.name.str() == "dataoffset" ) if (!m_dataNames.count(std::get(arg).value)) - typeError( + m_errorReporter.typeError( + 3517_error, _funCall.functionName.location, "Unknown data object \"" + std::get(arg).value.str() + "\"." ); @@ -362,7 +376,7 @@ void AsmAnalyzer::operator()(Switch const& _switch) /// Note: the parser ensures there is only one default case if (watcher.ok() && !cases.insert(valueOfLiteral(*_case.value)).second) - declarationError(_case.location, "Duplicate case defined."); + m_errorReporter.declarationError(6792_error, _case.location, "Duplicate case defined."); } (*this)(_case.body); @@ -408,7 +422,8 @@ YulString AsmAnalyzer::expectExpression(Expression const& _expr) { vector types = std::visit(*this, _expr); if (types.size() != 1) - typeError( + m_errorReporter.typeError( + 3950_error, locationOf(_expr), "Expected expression to evaluate to one value, but got " + to_string(types.size()) + @@ -421,7 +436,9 @@ void AsmAnalyzer::expectBoolExpression(Expression const& _expr) { YulString type = expectExpression(_expr); if (type != m_dialect.boolType) - typeError(locationOf(_expr), + m_errorReporter.typeError( + 1733_error, + locationOf(_expr), "Expected a value of boolean type \"" + m_dialect.boolType.str() + "\" but got \"" + @@ -440,9 +457,10 @@ void AsmAnalyzer::checkAssignment(Identifier const& _variable, YulString _valueT { // Check that it is a variable if (!holds_alternative(*var)) - typeError(_variable.location, "Assignment requires variable."); + m_errorReporter.typeError(2657_error, _variable.location, "Assignment requires variable."); else if (!m_activeVariables.count(&std::get(*var))) - declarationError( + m_errorReporter.declarationError( + 1133_error, _variable.location, "Variable " + _variable.name.str() + " used before it was declared." ); @@ -464,9 +482,11 @@ void AsmAnalyzer::checkAssignment(Identifier const& _variable, YulString _valueT if (!found && watcher.ok()) // Only add message if the callback did not. - declarationError(_variable.location, "Variable not found or variable not lvalue."); + m_errorReporter.declarationError(4634_error, _variable.location, "Variable not found or variable not lvalue."); if (variableType && *variableType != _valueType) - typeError(_variable.location, + m_errorReporter.typeError( + 9547_error, + _variable.location, "Assigning a value of type \"" + _valueType.str() + "\" to a variable of type \"" + @@ -488,7 +508,8 @@ Scope& AsmAnalyzer::scope(Block const* _block) void AsmAnalyzer::expectValidType(YulString _type, SourceLocation const& _location) { if (!m_dialect.types.count(_type)) - typeError( + m_errorReporter.typeError( + 5473_error, _location, "\"" + _type.str() + "\" is not a valid type (user defined types are not yet supported)." ); @@ -497,7 +518,9 @@ void AsmAnalyzer::expectValidType(YulString _type, SourceLocation const& _locati void AsmAnalyzer::expectType(YulString _expectedType, YulString _givenType, SourceLocation const& _location) { if (_expectedType != _givenType) - typeError(_location, + m_errorReporter.typeError( + 3781_error, + _location, "Expected a value of type \"" + _expectedType.str() + "\" but got \"" + @@ -524,7 +547,8 @@ bool AsmAnalyzer::warnOnInstructions(evmasm::Instruction _instr, SourceLocation yulAssert(m_evmVersion.hasBitwiseShifting() == m_evmVersion.hasCreate2(), ""); auto errorForVM = [&](string const& vmKindMessage) { - typeError( + m_errorReporter.typeError( + 7079_error, _location, "The \"" + boost::to_lower_copy(instructionInfo(_instr).name) @@ -584,13 +608,3 @@ bool AsmAnalyzer::warnOnInstructions(evmasm::Instruction _instr, SourceLocation return true; } - -void AsmAnalyzer::typeError(SourceLocation const& _location, string const& _description) -{ - m_errorReporter.typeError(7569_error, _location, _description); -} - -void AsmAnalyzer::declarationError(SourceLocation const& _location, string const& _description) -{ - m_errorReporter.declarationError(9595_error, _location, _description); -} diff --git a/libyul/AsmAnalysis.h b/libyul/AsmAnalysis.h index 438c31bbd..3804476ae 100644 --- a/libyul/AsmAnalysis.h +++ b/libyul/AsmAnalysis.h @@ -117,9 +117,6 @@ private: return warnOnInstructions(_functionCall.functionName.name.str(), _functionCall.functionName.location); } - void typeError(langutil::SourceLocation const& _location, std::string const& _description); - void declarationError(langutil::SourceLocation const& _location, std::string const& _description); - yul::ExternalIdentifierAccess::Resolver m_resolver; Scope* m_currentScope = nullptr; /// Variables that are active at the current point in assembly (as opposed to From be83c54d79c2a6388a7254dada03917c634ad63b Mon Sep 17 00:00:00 2001 From: a3d4 Date: Thu, 28 May 2020 01:17:53 +0200 Subject: [PATCH 07/36] Throw TestParserError instead of Error in tests --- test/libsolidity/util/BytesUtils.cpp | 8 +-- test/libsolidity/util/SoltestErrors.h | 9 ++++ test/libsolidity/util/TestFileParser.cpp | 41 +++++++--------- test/libsolidity/util/TestFileParserTests.cpp | 49 ++++++++++--------- 4 files changed, 57 insertions(+), 50 deletions(-) diff --git a/test/libsolidity/util/BytesUtils.cpp b/test/libsolidity/util/BytesUtils.cpp index fd5cb7f49..72f7c7594 100644 --- a/test/libsolidity/util/BytesUtils.cpp +++ b/test/libsolidity/util/BytesUtils.cpp @@ -81,7 +81,7 @@ bytes BytesUtils::convertBoolean(string const& _literal) else if (_literal == "false") return bytes{false}; else - throw Error(Error::Type::ParserError, "Boolean literal invalid."); + throw TestParserError("Boolean literal invalid."); } bytes BytesUtils::convertNumber(string const& _literal) @@ -92,7 +92,7 @@ bytes BytesUtils::convertNumber(string const& _literal) } catch (std::exception const&) { - throw Error(Error::Type::ParserError, "Number encoding invalid."); + throw TestParserError("Number encoding invalid."); } } @@ -104,7 +104,7 @@ bytes BytesUtils::convertHexNumber(string const& _literal) } catch (std::exception const&) { - throw Error(Error::Type::ParserError, "Hex number encoding invalid."); + throw TestParserError("Hex number encoding invalid."); } } @@ -116,7 +116,7 @@ bytes BytesUtils::convertString(string const& _literal) } catch (std::exception const&) { - throw Error(Error::Type::ParserError, "String encoding invalid."); + throw TestParserError("String encoding invalid."); } } diff --git a/test/libsolidity/util/SoltestErrors.h b/test/libsolidity/util/SoltestErrors.h index f895e706b..3291692a8 100644 --- a/test/libsolidity/util/SoltestErrors.h +++ b/test/libsolidity/util/SoltestErrors.h @@ -30,6 +30,15 @@ namespace solidity::frontend::test while (false) +class TestParserError: virtual public util::Exception +{ +public: + explicit TestParserError(std::string const& _description) + { + *this << util::errinfo_comment(_description); + } +}; + /** * Representation of a notice, warning or error that can occur while * formatting and therefore updating an interactive function call test. diff --git a/test/libsolidity/util/TestFileParser.cpp b/test/libsolidity/util/TestFileParser.cpp index 50ca68f63..52e4156aa 100644 --- a/test/libsolidity/util/TestFileParser.cpp +++ b/test/libsolidity/util/TestFileParser.cpp @@ -18,6 +18,7 @@ #include #include +#include #include #include @@ -128,9 +129,9 @@ vector TestFileParser::parseFunctionCall calls.emplace_back(std::move(call)); } - catch (Error const& _e) + catch (TestParserError const& _e) { - throw Error{_e.type(), "Line " + to_string(_lineOffset + m_lineNumber) + ": " + _e.what()}; + throw TestParserError("Line " + to_string(_lineOffset + m_lineNumber) + ": " + _e.what()); } } } @@ -150,8 +151,7 @@ bool TestFileParser::accept(soltest::Token _token, bool const _expect) bool TestFileParser::expect(soltest::Token _token, bool const _advance) { if (m_scanner.currentToken() != _token || m_scanner.currentToken() == Token::Invalid) - throw Error( - Error::Type::ParserError, + throw TestParserError( "Unexpected " + formatToken(m_scanner.currentToken()) + ": \"" + m_scanner.currentLiteral() + "\". " + "Expected \"" + formatToken(_token) + "\"." @@ -187,10 +187,10 @@ pair TestFileParser::parseFunctionSignature() parameters += parseIdentifierOrTuple(); } if (accept(Token::Arrow, true)) - throw Error(Error::Type::ParserError, "Invalid signature detected: " + signature); + throw TestParserError("Invalid signature detected: " + signature); if (!hasName && !parameters.empty()) - throw Error(Error::Type::ParserError, "Signatures without a name cannot have parameters: " + signature); + throw TestParserError("Signatures without a name cannot have parameters: " + signature); else signature += parameters; @@ -207,7 +207,7 @@ FunctionValue TestFileParser::parseFunctionCallValue() u256 value{ parseDecimalNumber() }; Token token = m_scanner.currentToken(); if (token != Token::Ether && token != Token::Wei) - throw Error(Error::Type::ParserError, "Invalid value unit provided. Coins can be wei or ether."); + throw TestParserError("Invalid value unit provided. Coins can be wei or ether."); m_scanner.scanNextToken(); @@ -216,7 +216,7 @@ FunctionValue TestFileParser::parseFunctionCallValue() } catch (std::exception const&) { - throw Error(Error::Type::ParserError, "Ether value encoding invalid."); + throw TestParserError("Ether value encoding invalid."); } } @@ -226,7 +226,7 @@ FunctionCallArgs TestFileParser::parseFunctionCallArguments() auto param = parseParameter(); if (param.abiType.type == ABIType::None) - throw Error(Error::Type::ParserError, "No argument provided."); + throw TestParserError("No argument provided."); arguments.parameters.emplace_back(param); while (accept(Token::Comma, true)) @@ -290,7 +290,7 @@ Parameter TestFileParser::parseParameter() if (accept(Token::Boolean)) { if (isSigned) - throw Error(Error::Type::ParserError, "Invalid boolean literal."); + throw TestParserError("Invalid boolean literal."); parameter.abiType = ABIType{ABIType::Boolean, ABIType::AlignRight, 32}; string parsed = parseBoolean(); @@ -304,7 +304,7 @@ Parameter TestFileParser::parseParameter() else if (accept(Token::HexNumber)) { if (isSigned) - throw Error(Error::Type::ParserError, "Invalid hex number literal."); + throw TestParserError("Invalid hex number literal."); parameter.abiType = ABIType{ABIType::Hex, ABIType::AlignRight, 32}; string parsed = parseHexNumber(); @@ -318,9 +318,9 @@ Parameter TestFileParser::parseParameter() else if (accept(Token::Hex, true)) { if (isSigned) - throw Error(Error::Type::ParserError, "Invalid hex string literal."); + throw TestParserError("Invalid hex string literal."); if (parameter.alignment != Parameter::Alignment::None) - throw Error(Error::Type::ParserError, "Hex string literals cannot be aligned or padded."); + throw TestParserError("Hex string literals cannot be aligned or padded."); string parsed = parseString(); parameter.rawString += "hex\"" + parsed + "\""; @@ -332,9 +332,9 @@ Parameter TestFileParser::parseParameter() else if (accept(Token::String)) { if (isSigned) - throw Error(Error::Type::ParserError, "Invalid string literal."); + throw TestParserError("Invalid string literal."); if (parameter.alignment != Parameter::Alignment::None) - throw Error(Error::Type::ParserError, "String literals cannot be aligned or padded."); + throw TestParserError("String literals cannot be aligned or padded."); string parsed = parseString(); parameter.abiType = ABIType{ABIType::String, ABIType::AlignLeft, parsed.size()}; @@ -364,7 +364,7 @@ Parameter TestFileParser::parseParameter() else if (accept(Token::Failure, true)) { if (isSigned) - throw Error(Error::Type::ParserError, "Invalid failure literal."); + throw TestParserError("Invalid failure literal."); parameter.abiType = ABIType{ABIType::Failure, ABIType::AlignRight, 0}; parameter.rawBytes = bytes{}; @@ -555,10 +555,7 @@ void TestFileParser::Scanner::scanNextToken() else if (isEndOfLine()) token = make_pair(Token::EOS, "EOS"); else - throw Error( - Error::Type::ParserError, - "Unexpected character: '" + string{current()} + "'" - ); + throw TestParserError("Unexpected character: '" + string{current()} + "'"); break; } } @@ -651,7 +648,7 @@ string TestFileParser::Scanner::scanString() str += scanHexPart(); break; default: - throw Error(Error::Type::ParserError, "Invalid or escape sequence found in string literal."); + throw TestParserError("Invalid or escape sequence found in string literal."); } } else @@ -673,7 +670,7 @@ char TestFileParser::Scanner::scanHexPart() else if (tolower(current()) >= 'a' && tolower(current()) <= 'f') value = tolower(current()) - 'a' + 10; else - throw Error(Error::Type::ParserError, "\\x used with no following hex digits."); + throw TestParserError("\\x used with no following hex digits."); advance(); if (current() == '"') diff --git a/test/libsolidity/util/TestFileParserTests.cpp b/test/libsolidity/util/TestFileParserTests.cpp index f074fc1bb..d24a80b95 100644 --- a/test/libsolidity/util/TestFileParserTests.cpp +++ b/test/libsolidity/util/TestFileParserTests.cpp @@ -25,6 +25,7 @@ #include #include +#include #include using namespace std; @@ -365,7 +366,7 @@ BOOST_AUTO_TEST_CASE(scanner_hex_values_invalid1) char const* source = R"( // f(uint256): "\x" -> )"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(scanner_hex_values_invalid2) @@ -383,7 +384,7 @@ BOOST_AUTO_TEST_CASE(scanner_hex_values_invalid3) char const* source = R"( // f(uint256): "\xZ" -> )"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(scanner_hex_values_invalid4) @@ -391,7 +392,7 @@ BOOST_AUTO_TEST_CASE(scanner_hex_values_invalid4) char const* source = R"( // f(uint256): "\xZZ" -> )"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(call_arguments_hex_string) @@ -741,7 +742,7 @@ BOOST_AUTO_TEST_CASE(call_arguments_hex_string_left_align) char const* source = R"( // f(bytes): left(hex"4200ef") -> )"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(call_arguments_hex_string_right_align) @@ -749,7 +750,7 @@ BOOST_AUTO_TEST_CASE(call_arguments_hex_string_right_align) char const* source = R"( // f(bytes): right(hex"4200ef") -> )"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(call_newline_invalid) @@ -757,7 +758,7 @@ BOOST_AUTO_TEST_CASE(call_newline_invalid) char const* source = R"( / )"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(call_invalid) @@ -765,7 +766,7 @@ BOOST_AUTO_TEST_CASE(call_invalid) char const* source = R"( / f() -> )"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(call_signature_invalid) @@ -773,7 +774,7 @@ BOOST_AUTO_TEST_CASE(call_signature_invalid) char const* source = R"( // f(uint8,) -> FAILURE )"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(call_arguments_tuple_invalid) @@ -781,7 +782,7 @@ BOOST_AUTO_TEST_CASE(call_arguments_tuple_invalid) char const* source = R"( // f((uint8,) -> FAILURE )"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(call_arguments_tuple_invalid_empty) @@ -789,7 +790,7 @@ BOOST_AUTO_TEST_CASE(call_arguments_tuple_invalid_empty) char const* source = R"( // f(uint8, ()) -> FAILURE )"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(call_arguments_tuple_invalid_parantheses) @@ -797,14 +798,14 @@ BOOST_AUTO_TEST_CASE(call_arguments_tuple_invalid_parantheses) char const* source = R"( // f((uint8,() -> FAILURE )"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(call_ether_value_expectations_missing) { char const* source = R"( // f(), 0)"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(call_arguments_invalid) @@ -812,7 +813,7 @@ BOOST_AUTO_TEST_CASE(call_arguments_invalid) char const* source = R"( // f(uint256): abc -> 1 )"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(call_arguments_invalid_decimal) @@ -820,7 +821,7 @@ BOOST_AUTO_TEST_CASE(call_arguments_invalid_decimal) char const* source = R"( // sig(): 0.h3 -> )"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(call_ether_value_invalid) @@ -828,7 +829,7 @@ BOOST_AUTO_TEST_CASE(call_ether_value_invalid) char const* source = R"( // f(uint256), abc : 1 -> 1 )"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(call_ether_value_invalid_decimal) @@ -836,7 +837,7 @@ BOOST_AUTO_TEST_CASE(call_ether_value_invalid_decimal) char const* source = R"( // sig(): 0.1hd ether -> )"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(call_ether_type_invalid) @@ -844,7 +845,7 @@ BOOST_AUTO_TEST_CASE(call_ether_type_invalid) char const* source = R"( // f(uint256), 2 btc : 1 -> 1 )"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(call_signed_bool_invalid) @@ -852,7 +853,7 @@ BOOST_AUTO_TEST_CASE(call_signed_bool_invalid) char const* source = R"( // f() -> -true )"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(call_signed_failure_invalid) @@ -860,7 +861,7 @@ BOOST_AUTO_TEST_CASE(call_signed_failure_invalid) char const* source = R"( // f() -> -FAILURE )"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(call_signed_hex_number_invalid) @@ -868,7 +869,7 @@ BOOST_AUTO_TEST_CASE(call_signed_hex_number_invalid) char const* source = R"( // f() -> -0x42 )"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(call_arguments_colon) @@ -877,7 +878,7 @@ BOOST_AUTO_TEST_CASE(call_arguments_colon) // h256(): // -> 1 )"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(call_arguments_newline_colon) @@ -887,7 +888,7 @@ BOOST_AUTO_TEST_CASE(call_arguments_newline_colon) // : // -> 1 )"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(call_arrow_missing) @@ -895,7 +896,7 @@ BOOST_AUTO_TEST_CASE(call_arrow_missing) char const* source = R"( // h256() FAILURE )"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(call_unexpected_character) @@ -903,7 +904,7 @@ BOOST_AUTO_TEST_CASE(call_unexpected_character) char const* source = R"( // f() -> ?? )"; - BOOST_REQUIRE_THROW(parse(source), langutil::Error); + BOOST_REQUIRE_THROW(parse(source), TestParserError); } BOOST_AUTO_TEST_CASE(constructor) From cb1cbbc1f1cf34cee5ff5be057babb5975c04162 Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Thu, 28 May 2020 10:55:35 +0200 Subject: [PATCH 08/36] [SMTChecker] Fix fixed point inc/dec --- Changelog.md | 1 + libsolidity/formal/SMTEncoder.cpp | 7 ++----- libsolidity/formal/SymbolicTypes.cpp | 8 ++++++++ libsolidity/formal/SymbolicTypes.h | 1 + 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/Changelog.md b/Changelog.md index a908ad23f..69ad9a879 100644 --- a/Changelog.md +++ b/Changelog.md @@ -23,6 +23,7 @@ Bugfixes: * NatSpec: DocString block is terminated when encountering an empty line. * Scanner: Fix bug when two empty NatSpec comments lead to scanning past EOL. * Code Generator: Trigger proper unimplemented errors on certain array copy operations. + * SMTChecker: Fix internal error when applying arithmetic operators to fixed point variables. ### 0.6.8 (2020-05-14) diff --git a/libsolidity/formal/SMTEncoder.cpp b/libsolidity/formal/SMTEncoder.cpp index 258c74bcc..fd1acb66f 100644 --- a/libsolidity/formal/SMTEncoder.cpp +++ b/libsolidity/formal/SMTEncoder.cpp @@ -463,9 +463,6 @@ void SMTEncoder::endVisit(UnaryOperation const& _op) createExpr(_op); - if (_op.annotation().type->category() == Type::Category::FixedPoint) - return; - switch (_op.getOperator()) { case Token::Not: // ! @@ -477,8 +474,8 @@ void SMTEncoder::endVisit(UnaryOperation const& _op) case Token::Inc: // ++ (pre- or postfix) case Token::Dec: // -- (pre- or postfix) { - - solAssert(smt::isInteger(_op.annotation().type->category()), ""); + auto cat = _op.annotation().type->category(); + solAssert(smt::isInteger(cat) || smt::isFixedPoint(cat), ""); solAssert(_op.subExpression().annotation().willBeWrittenTo, ""); if (auto identifier = dynamic_cast(&_op.subExpression())) { diff --git a/libsolidity/formal/SymbolicTypes.cpp b/libsolidity/formal/SymbolicTypes.cpp index 0ed81ee31..a46b88e03 100644 --- a/libsolidity/formal/SymbolicTypes.cpp +++ b/libsolidity/formal/SymbolicTypes.cpp @@ -224,6 +224,8 @@ pair> newSymbolicVariable( } else if (isInteger(_type.category())) var = make_shared(type, type, _uniqueName, _context); + else if (isFixedPoint(_type.category())) + var = make_shared(type, type, _uniqueName, _context); else if (isFixedBytes(_type.category())) { auto fixedBytesType = dynamic_cast(type); @@ -272,6 +274,11 @@ bool isInteger(frontend::Type::Category _category) return _category == frontend::Type::Category::Integer; } +bool isFixedPoint(frontend::Type::Category _category) +{ + return _category == frontend::Type::Category::FixedPoint; +} + bool isRational(frontend::Type::Category _category) { return _category == frontend::Type::Category::RationalNumber; @@ -300,6 +307,7 @@ bool isEnum(frontend::Type::Category _category) bool isNumber(frontend::Type::Category _category) { return isInteger(_category) || + isFixedPoint(_category) || isRational(_category) || isFixedBytes(_category) || isAddress(_category) || diff --git a/libsolidity/formal/SymbolicTypes.h b/libsolidity/formal/SymbolicTypes.h index 2e06f4c86..226bd5f94 100644 --- a/libsolidity/formal/SymbolicTypes.h +++ b/libsolidity/formal/SymbolicTypes.h @@ -43,6 +43,7 @@ bool isSupportedTypeDeclaration(frontend::Type::Category _category); bool isSupportedTypeDeclaration(frontend::Type const& _type); bool isInteger(frontend::Type::Category _category); +bool isFixedPoint(frontend::Type::Category _category); bool isRational(frontend::Type::Category _category); bool isFixedBytes(frontend::Type::Category _category); bool isAddress(frontend::Type::Category _category); From 13f32268da48f0c3bf5a888fc5823b7cf7eb42c7 Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Thu, 28 May 2020 13:05:56 +0200 Subject: [PATCH 09/36] [SMTChecker] Add test that shows that deleting arrays takes the index into account --- .../operators/delete_multid_array.sol | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 test/libsolidity/smtCheckerTests/operators/delete_multid_array.sol diff --git a/test/libsolidity/smtCheckerTests/operators/delete_multid_array.sol b/test/libsolidity/smtCheckerTests/operators/delete_multid_array.sol new file mode 100644 index 000000000..0d19b2877 --- /dev/null +++ b/test/libsolidity/smtCheckerTests/operators/delete_multid_array.sol @@ -0,0 +1,42 @@ +pragma experimental SMTChecker; + +contract C { + uint[] a; + uint[][] b; + function f(uint x, uint y, uint v) public { + a[x] = v; + delete a; + assert(a[y] == 0); + } + function g(uint x, uint y, uint v) public { + b[x][y] = v; + delete b; + assert(b[y][x] == 0); + } + function h(uint x, uint y, uint v) public { + b[x][y] = v; + delete b[x]; + // Not necessarily the case. + assert(b[y][x] == 0); + } + function i(uint x, uint y, uint v) public { + b[x][y] = v; + delete b[y]; + assert(b[y][x] == 0); + } + function j(uint x, uint y, uint z, uint v) public { + b[x][y] = v; + delete b[z]; + // Not necessarily the case. + assert(b[y][x] == 0); + } + function setA(uint x, uint y) public { + a[x] = y; + } + function setB(uint x, uint y, uint z) public { + b[x][y] = z; + } +} +// ---- +// Warning: (372-392): Assertion violation happens here +// Warning: (617-637): Assertion violation happens here From a73ec6a82f1da9f5cfa9ea1033a73ac78a9b0580 Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Thu, 28 May 2020 12:17:53 +0200 Subject: [PATCH 10/36] [SMTChecker] Fix ICE in inlining function calls while short circuiting --- Changelog.md | 1 + libsolidity/formal/SMTEncoder.cpp | 1 - libsolidity/formal/VariableUsage.cpp | 3 ++- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Changelog.md b/Changelog.md index 9006ac777..992436ea3 100644 --- a/Changelog.md +++ b/Changelog.md @@ -25,6 +25,7 @@ Bugfixes: * Scanner: Fix bug when two empty NatSpec comments lead to scanning past EOL. * Code Generator: Trigger proper unimplemented errors on certain array copy operations. * SMTChecker: Fix internal error when applying arithmetic operators to fixed point variables. + * SMTChecker: Fix internal error when short circuiting Boolean expressions with function calls in state variable initialization. ### 0.6.8 (2020-05-14) diff --git a/libsolidity/formal/SMTEncoder.cpp b/libsolidity/formal/SMTEncoder.cpp index fd1acb66f..be9bea10f 100644 --- a/libsolidity/formal/SMTEncoder.cpp +++ b/libsolidity/formal/SMTEncoder.cpp @@ -1791,7 +1791,6 @@ Expression const* SMTEncoder::leftmostBase(IndexAccess const& _indexAccess) set SMTEncoder::touchedVariables(ASTNode const& _node) { - solAssert(!m_callStack.empty(), ""); vector callStack; for (auto const& call: m_callStack) callStack.push_back(call.first); diff --git a/libsolidity/formal/VariableUsage.cpp b/libsolidity/formal/VariableUsage.cpp index 1871c0908..acfd34909 100644 --- a/libsolidity/formal/VariableUsage.cpp +++ b/libsolidity/formal/VariableUsage.cpp @@ -33,7 +33,8 @@ set VariableUsage::touchedVariables(ASTNode const& _ m_touchedVariables.clear(); m_callStack.clear(); m_callStack += _outerCallstack; - m_lastCall = m_callStack.back(); + if (!m_callStack.empty()) + m_lastCall = m_callStack.back(); _node.accept(*this); return m_touchedVariables; } From ec766958eace1acdb36abfce888f7d539b49c72c Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Thu, 28 May 2020 12:40:08 +0200 Subject: [PATCH 11/36] Add test --- .../functions/internal_call_state_var_init.sol | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 test/libsolidity/smtCheckerTests/functions/internal_call_state_var_init.sol diff --git a/test/libsolidity/smtCheckerTests/functions/internal_call_state_var_init.sol b/test/libsolidity/smtCheckerTests/functions/internal_call_state_var_init.sol new file mode 100644 index 000000000..8f6276e0e --- /dev/null +++ b/test/libsolidity/smtCheckerTests/functions/internal_call_state_var_init.sol @@ -0,0 +1,5 @@ +pragma experimental SMTChecker; +contract c { + bool b = (f() == 0) && (f() == 0); + function f() internal returns (uint) {} +} From c9593417207b04ca7fc6bd0b24f1009d63eaa046 Mon Sep 17 00:00:00 2001 From: a3d4 Date: Thu, 28 May 2020 04:32:46 +0200 Subject: [PATCH 12/36] Add errorId to Error class --- liblangutil/ErrorReporter.cpp | 14 ++++++-------- liblangutil/ErrorReporter.h | 12 +----------- liblangutil/Exceptions.cpp | 7 ++++--- liblangutil/Exceptions.h | 18 ++++++++++++++++-- libsolidity/ast/Types.cpp | 9 +++------ 5 files changed, 30 insertions(+), 30 deletions(-) diff --git a/liblangutil/ErrorReporter.cpp b/liblangutil/ErrorReporter.cpp index d06fbc65b..a8dccd557 100644 --- a/liblangutil/ErrorReporter.cpp +++ b/liblangutil/ErrorReporter.cpp @@ -28,8 +28,6 @@ using namespace std; using namespace solidity; using namespace solidity::langutil; -ErrorId solidity::langutil::operator"" _error(unsigned long long _error) { return ErrorId{ _error }; } - ErrorReporter& ErrorReporter::operator=(ErrorReporter const& _errorReporter) { if (&_errorReporter == this) @@ -62,12 +60,12 @@ void ErrorReporter::warning( error(_error, Error::Type::Warning, _location, _secondaryLocation, _description); } -void ErrorReporter::error(ErrorId, Error::Type _type, SourceLocation const& _location, string const& _description) +void ErrorReporter::error(ErrorId _errorId, Error::Type _type, SourceLocation const& _location, string const& _description) { if (checkForExcessiveErrors(_type)) return; - auto err = make_shared(_type); + auto err = make_shared(_errorId, _type); *err << errinfo_sourceLocation(_location) << util::errinfo_comment(_description); @@ -75,12 +73,12 @@ void ErrorReporter::error(ErrorId, Error::Type _type, SourceLocation const& _loc m_errorList.push_back(err); } -void ErrorReporter::error(ErrorId, Error::Type _type, SourceLocation const& _location, SecondarySourceLocation const& _secondaryLocation, string const& _description) +void ErrorReporter::error(ErrorId _errorId, Error::Type _type, SourceLocation const& _location, SecondarySourceLocation const& _secondaryLocation, string const& _description) { if (checkForExcessiveErrors(_type)) return; - auto err = make_shared(_type); + auto err = make_shared(_errorId, _type); *err << errinfo_sourceLocation(_location) << errinfo_secondarySourceLocation(_secondaryLocation) << @@ -102,7 +100,7 @@ bool ErrorReporter::checkForExcessiveErrors(Error::Type _type) if (m_warningCount == c_maxWarningsAllowed) { - auto err = make_shared(Error::Type::Warning); + auto err = make_shared(4591_error, Error::Type::Warning); *err << util::errinfo_comment("There are more than 256 warnings. Ignoring the rest."); m_errorList.push_back(err); } @@ -116,7 +114,7 @@ bool ErrorReporter::checkForExcessiveErrors(Error::Type _type) if (m_errorCount > c_maxErrorsAllowed) { - auto err = make_shared(Error::Type::Warning); + auto err = make_shared(4013_error, Error::Type::Warning); *err << util::errinfo_comment("There are more than 256 errors. Aborting."); m_errorList.push_back(err); BOOST_THROW_EXCEPTION(FatalError()); diff --git a/liblangutil/ErrorReporter.h b/liblangutil/ErrorReporter.h index fed3430f2..3dc8c59c7 100644 --- a/liblangutil/ErrorReporter.h +++ b/liblangutil/ErrorReporter.h @@ -23,6 +23,7 @@ #pragma once #include +#include #include #include @@ -33,17 +34,6 @@ namespace solidity::langutil { -/** - * Unique identifiers are used to tag and track individual error cases. - * They are passed as the first parameter of error reporting functions. - * Suffix _error helps to find them in the sources. - * The struct ErrorId prevents incidental calls like typeError(3141) instead of typeError(3141_error). - * To create a new ID, one can add 0000_error and then run "python ./scripts/correct_error_ids.py" - * from the root of the repo. - */ -struct ErrorId { unsigned long long error = 0; }; -ErrorId operator"" _error(unsigned long long error); - class ErrorReporter { public: diff --git a/liblangutil/Exceptions.cpp b/liblangutil/Exceptions.cpp index f035f5044..fa65031cd 100644 --- a/liblangutil/Exceptions.cpp +++ b/liblangutil/Exceptions.cpp @@ -26,7 +26,8 @@ using namespace std; using namespace solidity; using namespace solidity::langutil; -Error::Error(Type _type, SourceLocation const& _location, string const& _description): +Error::Error(ErrorId _errorId, Type _type, SourceLocation const& _location, string const& _description): + m_errorId(_errorId), m_type(_type) { switch (m_type) @@ -57,8 +58,8 @@ Error::Error(Type _type, SourceLocation const& _location, string const& _descrip *this << util::errinfo_comment(_description); } -Error::Error(Error::Type _type, std::string const& _description, SourceLocation const& _location): - Error(_type) +Error::Error(ErrorId _errorId, Error::Type _type, std::string const& _description, SourceLocation const& _location): + Error(_errorId, _type) { if (_location.isValid()) *this << errinfo_sourceLocation(_location); diff --git a/liblangutil/Exceptions.h b/liblangutil/Exceptions.h index b8688afa6..42b76a1fe 100644 --- a/liblangutil/Exceptions.h +++ b/liblangutil/Exceptions.h @@ -56,6 +56,17 @@ struct InvalidAstError: virtual util::Exception {}; #define astAssert(CONDITION, DESCRIPTION) \ assertThrow(CONDITION, ::solidity::langutil::InvalidAstError, DESCRIPTION) +/** + * Unique identifiers are used to tag and track individual error cases. + * They are passed as the first parameter of error reporting functions. + * Suffix _error helps to find them in the sources. + * The struct ErrorId prevents incidental calls like typeError(3141) instead of typeError(3141_error). + * To create a new ID, one can add 0000_error and then run "python ./scripts/correct_error_ids.py" + * from the root of the repo. + */ +struct ErrorId { unsigned long long error = 0; }; +constexpr ErrorId operator"" _error(unsigned long long _error) { return ErrorId{ _error }; } + class Error: virtual public util::Exception { public: @@ -69,14 +80,16 @@ public: Warning }; - explicit Error( + Error( + ErrorId _errorId, Type _type, SourceLocation const& _location = SourceLocation(), std::string const& _description = std::string() ); - Error(Type _type, std::string const& _description, SourceLocation const& _location = SourceLocation()); + Error(ErrorId _errorId, Type _type, std::string const& _description, SourceLocation const& _location = SourceLocation()); + ErrorId errorId() const { return m_errorId; } Type type() const { return m_type; } std::string const& typeName() const { return m_typeName; } @@ -100,6 +113,7 @@ public: return true; } private: + ErrorId m_errorId; Type m_type; std::string m_typeName; }; diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index fbb119e42..66681ea10 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -172,8 +172,7 @@ void StorageOffsets::computeOffsets(TypePointers const& _types) ++slotOffset; byteOffset = 0; } - if (slotOffset >= bigint(1) << 256) - BOOST_THROW_EXCEPTION(Error(Error::Type::TypeError) << util::errinfo_comment("Object too large for storage.")); + solAssert(slotOffset < bigint(1) << 256 ,"Object too large for storage."); offsets[i] = make_pair(u256(slotOffset), byteOffset); solAssert(type->storageSize() >= 1, "Invalid storage size."); if (type->storageSize() == 1 && byteOffset + type->storageBytes() <= 32) @@ -186,8 +185,7 @@ void StorageOffsets::computeOffsets(TypePointers const& _types) } if (byteOffset > 0) ++slotOffset; - if (slotOffset >= bigint(1) << 256) - BOOST_THROW_EXCEPTION(Error(Error::Type::TypeError) << util::errinfo_comment("Object too large for storage.")); + solAssert(slotOffset < bigint(1) << 256, "Object too large for storage."); m_storageSize = u256(slotOffset); swap(m_offsets, offsets); } @@ -1776,8 +1774,7 @@ u256 ArrayType::storageSize() const } else size = bigint(length()) * baseType()->storageSize(); - if (size >= bigint(1) << 256) - BOOST_THROW_EXCEPTION(Error(Error::Type::TypeError) << util::errinfo_comment("Array too large for storage.")); + solAssert(size < bigint(1) << 256, "Array too large for storage."); return max(1, u256(size)); } From b90fb1cab69ff186fe8ad0ad6480006850b263ee Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Thu, 28 May 2020 11:26:32 +0200 Subject: [PATCH 13/36] [SMTChecker] Fix ICE on index access assignment inside single branches --- Changelog.md | 1 + libsolidity/formal/SMTEncoder.cpp | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index 573d83e5b..9f06339b8 100644 --- a/Changelog.md +++ b/Changelog.md @@ -27,6 +27,7 @@ Bugfixes: * Code Generator: Trigger proper unimplemented errors on certain array copy operations. * SMTChecker: Fix internal error when applying arithmetic operators to fixed point variables. * SMTChecker: Fix internal error when short circuiting Boolean expressions with function calls in state variable initialization. + * SMTChecker: Fix internal error when assigning to index access inside branches. ### 0.6.8 (2020-05-14) diff --git a/libsolidity/formal/SMTEncoder.cpp b/libsolidity/formal/SMTEncoder.cpp index be9bea10f..6729703b7 100644 --- a/libsolidity/formal/SMTEncoder.cpp +++ b/libsolidity/formal/SMTEncoder.cpp @@ -361,7 +361,12 @@ void SMTEncoder::endVisit(Assignment const& _assignment) else if (!smt::isSupportedType(_assignment.annotation().type->category())) { // Give it a new index anyway to keep the SSA scheme sound. - if (auto varDecl = identifierToVariable(_assignment.leftHandSide())) + + Expression const* base = &_assignment.leftHandSide(); + if (auto const* indexAccess = dynamic_cast(base)) + base = leftmostBase(*indexAccess); + + if (auto varDecl = identifierToVariable(*base)) m_context.newValue(*varDecl); } else From 9b0146be422ce687909411455db08c6ffaec5cd8 Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Thu, 28 May 2020 12:28:22 +0200 Subject: [PATCH 14/36] Add test --- .../types/mapping_struct_assignment.sol | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 test/libsolidity/smtCheckerTests/types/mapping_struct_assignment.sol diff --git a/test/libsolidity/smtCheckerTests/types/mapping_struct_assignment.sol b/test/libsolidity/smtCheckerTests/types/mapping_struct_assignment.sol new file mode 100644 index 000000000..1bcf2a2eb --- /dev/null +++ b/test/libsolidity/smtCheckerTests/types/mapping_struct_assignment.sol @@ -0,0 +1,19 @@ +pragma experimental SMTChecker; +contract C +{ + struct S { + uint x; + } + mapping (uint => S) smap; + function f(uint y, uint v) public { + if (0==1) + smap[y] = S(v); + } +} +// ---- +// Warning: (140-144): Condition is always false. +// Warning: (149-156): Assertion checker does not yet implement type struct C.S storage ref +// Warning: (159-160): Assertion checker does not yet implement type type(struct C.S storage pointer) +// Warning: (159-163): Assertion checker does not yet implement type struct C.S memory +// Warning: (159-163): Assertion checker does not yet implement this expression. +// Warning: (149-163): Assertion checker does not yet implement type struct C.S storage ref From c96196c346b2c5895b25271be8ba69160763efca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Mon, 25 May 2020 20:57:04 +0200 Subject: [PATCH 15/36] Define constants to make flag meanings more apparent in binary wasm --- libyul/backends/wasm/BinaryTransform.cpp | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/libyul/backends/wasm/BinaryTransform.cpp b/libyul/backends/wasm/BinaryTransform.cpp index f5dfd8410..84e365016 100644 --- a/libyul/backends/wasm/BinaryTransform.cpp +++ b/libyul/backends/wasm/BinaryTransform.cpp @@ -39,6 +39,18 @@ bytes toBytes(uint8_t _b) return bytes(1, _b); } +enum class LimitsKind: uint8_t +{ + Min = 0x00, + MinMax = 0x01, +}; + +enum class Mutability: uint8_t +{ + Const = 0x00, + Var = 0x01, +}; + enum class Section: uint8_t { CUSTOM = 0x00, @@ -530,8 +542,8 @@ bytes BinaryTransform::functionSection(vector const& _functi bytes BinaryTransform::memorySection() { bytes result = lebEncode(1); - result.push_back(0); // flags - result.push_back(1); // initial + result.push_back(static_cast(LimitsKind::Min)); + result.push_back(1); // initial length return makeSection(Section::MEMORY, std::move(result)); } @@ -540,8 +552,8 @@ bytes BinaryTransform::globalSection() bytes result = lebEncode(m_globals.size()); for (size_t i = 0; i < m_globals.size(); ++i) result += - // mutable i64 - bytes{uint8_t(ValueType::I64), 1} + + toBytes(ValueType::I64) + + lebEncode(static_cast(Mutability::Var)) + toBytes(Opcode::I64Const) + lebEncodeSigned(0) + toBytes(Opcode::End); From 2128ff9f137cd6be7be283a7bfc7cffb08773695 Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Fri, 29 May 2020 19:13:27 +0200 Subject: [PATCH 16/36] Fix ICE on push for member access --- libsolidity/formal/SMTEncoder.cpp | 6 ++++ .../array_members/push_struct_member_1.sol | 27 +++++++++++++++ .../array_members/push_struct_member_2.sol | 33 +++++++++++++++++++ 3 files changed, 66 insertions(+) create mode 100644 test/libsolidity/smtCheckerTests/array_members/push_struct_member_1.sol create mode 100644 test/libsolidity/smtCheckerTests/array_members/push_struct_member_2.sol diff --git a/libsolidity/formal/SMTEncoder.cpp b/libsolidity/formal/SMTEncoder.cpp index 6729703b7..8ed8d71fa 100644 --- a/libsolidity/formal/SMTEncoder.cpp +++ b/libsolidity/formal/SMTEncoder.cpp @@ -1150,6 +1150,12 @@ void SMTEncoder::arrayPushPopAssign(Expression const& _expr, smtutil::Expression } else if (auto const* indexAccess = dynamic_cast(&_expr)) arrayIndexAssignment(*indexAccess, _array); + else if (dynamic_cast(&_expr)) + m_errorReporter.warning( + 9599_error, + _expr.location(), + "Assertion checker does not yet implement this expression." + ); else solAssert(false, ""); } diff --git a/test/libsolidity/smtCheckerTests/array_members/push_struct_member_1.sol b/test/libsolidity/smtCheckerTests/array_members/push_struct_member_1.sol new file mode 100644 index 000000000..4647742db --- /dev/null +++ b/test/libsolidity/smtCheckerTests/array_members/push_struct_member_1.sol @@ -0,0 +1,27 @@ +pragma experimental SMTChecker; +contract C { + struct S { + int[] b; + } + S s; + struct T { + S s; + } + T t; + function f() public { + s.b.push(); + t.s.b.push(); + } +} + +// ---- +// Warning: (72-75): Assertion checker does not yet support the type of this variable. +// Warning: (100-103): Assertion checker does not yet support the type of this variable. +// Warning: (130-133): Assertion checker does not yet support this expression. +// Warning: (130-131): Assertion checker does not yet implement type struct C.S storage ref +// Warning: (130-133): Assertion checker does not yet implement this expression. +// Warning: (144-149): Assertion checker does not yet support this expression. +// Warning: (144-147): Assertion checker does not yet implement type struct C.S storage ref +// Warning: (144-147): Assertion checker does not yet support this expression. +// Warning: (144-145): Assertion checker does not yet implement type struct C.T storage ref +// Warning: (144-149): Assertion checker does not yet implement this expression. diff --git a/test/libsolidity/smtCheckerTests/array_members/push_struct_member_2.sol b/test/libsolidity/smtCheckerTests/array_members/push_struct_member_2.sol new file mode 100644 index 000000000..353d328fd --- /dev/null +++ b/test/libsolidity/smtCheckerTests/array_members/push_struct_member_2.sol @@ -0,0 +1,33 @@ +pragma experimental SMTChecker; +contract C { + struct S { + int[] b; + } + S s; + struct T { + S[] s; + } + T t; + function f() public { + s.b.push(); + t.s.push(); + t.s[0].b.push(); + } +} + +// ---- +// Warning: (72-75): Assertion checker does not yet support the type of this variable. +// Warning: (102-105): Assertion checker does not yet support the type of this variable. +// Warning: (132-135): Assertion checker does not yet support this expression. +// Warning: (132-133): Assertion checker does not yet implement type struct C.S storage ref +// Warning: (132-135): Assertion checker does not yet implement this expression. +// Warning: (146-149): Assertion checker does not yet support this expression. +// Warning: (146-147): Assertion checker does not yet implement type struct C.T storage ref +// Warning: (146-156): Assertion checker does not yet implement type struct C.S storage ref +// Warning: (146-149): Assertion checker does not yet implement this expression. +// Warning: (160-168): Assertion checker does not yet support this expression. +// Warning: (160-163): Assertion checker does not yet support this expression. +// Warning: (160-161): Assertion checker does not yet implement type struct C.T storage ref +// Warning: (160-166): Assertion checker does not yet implement type struct C.S storage ref +// Warning: (160-166): Assertion checker does not yet implement this expression. +// Warning: (160-168): Assertion checker does not yet implement this expression. From 7ce760388f24a3f63d95f81fa7fbc7a4cf09c88a Mon Sep 17 00:00:00 2001 From: Flash Sheridan Date: Fri, 29 May 2020 16:50:05 -0400 Subject: [PATCH 17/36] Spell-check --- docs/060-breaking-changes.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/060-breaking-changes.rst b/docs/060-breaking-changes.rst index a1ee96d95..a661f1518 100644 --- a/docs/060-breaking-changes.rst +++ b/docs/060-breaking-changes.rst @@ -16,8 +16,8 @@ This section lists changes where the behaviour of your code might change without the compiler telling you about it. * The resulting type of an exponentiation is the type of the base. It used to be the smallest type - that can hold both the type of the base and the type of the exponent, as with symmentric - operations. Additionally, signed types are allowed for the base of the exponetation. + that can hold both the type of the base and the type of the exponent, as with symmetric + operations. Additionally, signed types are allowed for the base of the exponentiation. Explicitness Requirements From 36b54561a57894967bdf9e3495f303826aa07eb3 Mon Sep 17 00:00:00 2001 From: Flash Sheridan Date: Fri, 29 May 2020 17:09:15 -0400 Subject: [PATCH 18/36] Fix grammar and remove redundant words --- docs/060-breaking-changes.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/060-breaking-changes.rst b/docs/060-breaking-changes.rst index a1ee96d95..45fb3dba3 100644 --- a/docs/060-breaking-changes.rst +++ b/docs/060-breaking-changes.rst @@ -36,9 +36,9 @@ For most of the topics the compiler will provide suggestions. like so: ``override(Base1, Base2)``. * Member-access to ``length`` of arrays is now always read-only, even for storage arrays. It is no - longer possible to resize storage arrays assigning a new value to their length. Use ``push()``, - ``push(value)`` or ``pop()`` instead, or assign a full array, which will of course overwrite existing content. - The reason behind this is to prevent storage collisions by gigantic + longer possible to resize storage arrays by assigning a new value to their length. Use ``push()``, + ``push(value)`` or ``pop()`` instead, or assign a full array, which will of course overwrite the existing content. + The reason behind this is to prevent storage collisions of gigantic storage arrays. * The new keyword ``abstract`` can be used to mark contracts as abstract. It has to be used @@ -86,7 +86,7 @@ New Features ============ This section lists things that were not possible prior to Solidity 0.6.0 -or at least were more difficult to achieve prior to Solidity 0.6.0. +or were more difficult to achieve. * The :ref:`try/catch statement ` allows you to react on failed external calls. * ``struct`` and ``enum`` types can be declared at file level. @@ -103,7 +103,7 @@ Interface Changes This section lists changes that are unrelated to the language itself, but that have an effect on the interfaces of the compiler. These may change the way how you use the compiler on the command line, how you use its programmable -interface or how you analyze the output produced by it. +interface, or how you analyze the output produced by it. New Error Reporter ~~~~~~~~~~~~~~~~~~ From 311f025eb54f3b754f262d8a94043ffc402623a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Mon, 1 Jun 2020 18:17:50 +0200 Subject: [PATCH 19/36] SolidityExecutionFramework: Fix compileContract() to print Yul errors from the correct source --- test/libsolidity/SolidityExecutionFramework.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/libsolidity/SolidityExecutionFramework.cpp b/test/libsolidity/SolidityExecutionFramework.cpp index 0afe95c6d..d5e9c82cf 100644 --- a/test/libsolidity/SolidityExecutionFramework.cpp +++ b/test/libsolidity/SolidityExecutionFramework.cpp @@ -76,7 +76,7 @@ bytes SolidityExecutionFramework::compileContract( { langutil::SourceReferenceFormatter formatter(std::cerr); - for (auto const& error: m_compiler.errors()) + for (auto const& error: asmStack.errors()) formatter.printErrorInformation(*error); BOOST_ERROR("Assembly contract failed. IR: " + m_compiler.yulIROptimized({})); } From df7b82bf31ba23e23e4b5529523331fe6181d9ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Mon, 1 Jun 2020 18:24:31 +0200 Subject: [PATCH 20/36] SolidityExecutionFramework: Remove dead code for handling errors from parsing and analysis - This code can never be reached because CompilerStack.compile() called above does the same analysis and fails if it's not successful. --- test/libsolidity/SolidityExecutionFramework.cpp | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/test/libsolidity/SolidityExecutionFramework.cpp b/test/libsolidity/SolidityExecutionFramework.cpp index d5e9c82cf..1ab2c1db0 100644 --- a/test/libsolidity/SolidityExecutionFramework.cpp +++ b/test/libsolidity/SolidityExecutionFramework.cpp @@ -72,14 +72,9 @@ bytes SolidityExecutionFramework::compileContract( // get code that does not exhaust the stack. OptimiserSettings::full() ); - if (!asmStack.parseAndAnalyze("", m_compiler.yulIROptimized(contractName))) - { - langutil::SourceReferenceFormatter formatter(std::cerr); + bool analysisSuccessful = asmStack.parseAndAnalyze("", m_compiler.yulIROptimized(contractName)); + solAssert(analysisSuccessful, "Code that passed analysis in CompilerStack can't have errors"); - for (auto const& error: asmStack.errors()) - formatter.printErrorInformation(*error); - BOOST_ERROR("Assembly contract failed. IR: " + m_compiler.yulIROptimized({})); - } asmStack.optimize(); obj = std::move(*asmStack.assemble(yul::AssemblyStack::Machine::EVM).bytecode); } From 9bb9b6345c032635f15f1a63ddc0c4ab26196c47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Mon, 1 Jun 2020 18:25:02 +0200 Subject: [PATCH 21/36] SolidityExecutionFramework: Fix indentation - This code can never be reached because CompilerStack.compile() called above does the same analysis and fails if it's not successful. --- test/libsolidity/SolidityExecutionFramework.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/libsolidity/SolidityExecutionFramework.cpp b/test/libsolidity/SolidityExecutionFramework.cpp index 1ab2c1db0..5e4ebe088 100644 --- a/test/libsolidity/SolidityExecutionFramework.cpp +++ b/test/libsolidity/SolidityExecutionFramework.cpp @@ -66,12 +66,12 @@ bytes SolidityExecutionFramework::compileContract( if (m_compileViaYul) { yul::AssemblyStack asmStack( - m_evmVersion, - yul::AssemblyStack::Language::StrictAssembly, - // Ignore optimiser settings here because we need Yul optimisation to - // get code that does not exhaust the stack. - OptimiserSettings::full() - ); + m_evmVersion, + yul::AssemblyStack::Language::StrictAssembly, + // Ignore optimiser settings here because we need Yul optimisation to + // get code that does not exhaust the stack. + OptimiserSettings::full() + ); bool analysisSuccessful = asmStack.parseAndAnalyze("", m_compiler.yulIROptimized(contractName)); solAssert(analysisSuccessful, "Code that passed analysis in CompilerStack can't have errors"); From 2d7edeea3fb0ba19a4b1a159519cb4bb8f4e79a9 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Mon, 11 May 2020 15:41:39 +0100 Subject: [PATCH 22/36] Add shortcut to to/fromHex for empty input and fix signedness warning --- libsolutil/CommonData.cpp | 8 +++++++- libsolutil/CommonData.h | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/libsolutil/CommonData.cpp b/libsolutil/CommonData.cpp index bb1a7696c..d23a2ce6c 100644 --- a/libsolutil/CommonData.cpp +++ b/libsolutil/CommonData.cpp @@ -53,6 +53,9 @@ string solidity::util::toHex(uint8_t _data, HexCase _case) string solidity::util::toHex(bytes const& _data, HexPrefix _prefix, HexCase _case) { + if (_data.empty()) + return {}; + std::string ret(_data.size() * 2 + (_prefix == HexPrefix::Add ? 2 : 0), 0); size_t i = 0; @@ -64,7 +67,7 @@ string solidity::util::toHex(bytes const& _data, HexPrefix _prefix, HexCase _cas // Mixed case will be handled inside the loop. char const* chars = _case == HexCase::Upper ? upperHexChars : lowerHexChars; - int rix = _data.size() - 1; + size_t rix = _data.size() - 1; for (uint8_t c: _data) { // switch hex case every four hexchars @@ -95,6 +98,9 @@ int solidity::util::fromHex(char _i, WhenError _throw) bytes solidity::util::fromHex(std::string const& _s, WhenError _throw) { + if (_s.empty()) + return {}; + unsigned s = (_s.size() >= 2 && _s[0] == '0' && _s[1] == 'x') ? 2 : 0; std::vector ret; ret.reserve((_s.size() - s + 1) / 2); diff --git a/libsolutil/CommonData.h b/libsolutil/CommonData.h index adac1fb05..8fbe20568 100644 --- a/libsolutil/CommonData.h +++ b/libsolutil/CommonData.h @@ -299,7 +299,7 @@ template inline bytes toCompactBigEndian(T _val, unsigned _min = 0) { static_assert(std::is_same::value || !std::numeric_limits::is_signed, "only unsigned types or bigint supported"); //bigint does not carry sign bit on shift - int i = 0; + unsigned i = 0; for (T v = _val; v; ++i, v >>= 8) {} bytes ret(std::max(_min, i), 0); toBigEndian(_val, ret); From 0bc9f772e776026548928a7f214b1498e69f7861 Mon Sep 17 00:00:00 2001 From: Harikrishnan Mulackal Date: Fri, 29 May 2020 17:21:16 +0530 Subject: [PATCH 23/36] Added an assert for FixedPointType in InlineAssembly --- libsolidity/analysis/TypeChecker.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 606689368..075078e66 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -699,6 +699,8 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly) } } + solAssert(!dynamic_cast(var->type()), "FixedPointType not implemented."); + if (requiresStorage) { if (!var->isStateVariable() && !var->type()->dataStoredIn(DataLocation::Storage)) From ede39fc2dad819acc5027196af7cf9fae0a28fdd Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Fri, 29 May 2020 18:20:15 +0200 Subject: [PATCH 24/36] [SMTChecker] Relax assertion about callstack --- libsolidity/formal/VariableUsage.cpp | 3 +-- .../functions/internal_call_state_var_init_2.sol | 12 ++++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 test/libsolidity/smtCheckerTests/functions/internal_call_state_var_init_2.sol diff --git a/libsolidity/formal/VariableUsage.cpp b/libsolidity/formal/VariableUsage.cpp index acfd34909..11ca44ff3 100644 --- a/libsolidity/formal/VariableUsage.cpp +++ b/libsolidity/formal/VariableUsage.cpp @@ -104,8 +104,7 @@ void VariableUsage::checkIdentifier(Identifier const& _identifier) solAssert(declaration, ""); if (VariableDeclaration const* varDecl = dynamic_cast(declaration)) { - solAssert(m_lastCall, ""); - if (!varDecl->isLocalVariable() || varDecl->functionOrModifierDefinition() == m_lastCall) + if (!varDecl->isLocalVariable() || (m_lastCall && varDecl->functionOrModifierDefinition() == m_lastCall)) m_touchedVariables.insert(varDecl); } } diff --git a/test/libsolidity/smtCheckerTests/functions/internal_call_state_var_init_2.sol b/test/libsolidity/smtCheckerTests/functions/internal_call_state_var_init_2.sol new file mode 100644 index 000000000..285bfb15d --- /dev/null +++ b/test/libsolidity/smtCheckerTests/functions/internal_call_state_var_init_2.sol @@ -0,0 +1,12 @@ +pragma experimental SMTChecker; +contract c { + uint x; + function f() internal returns (uint) { + x = x + 1; + } + bool b = (f() > 0) || (f() > 0); +} +// ---- +// Warning: (100-105): Overflow (resulting value larger than 2**256 - 1) happens here +// Warning: (100-105): Underflow (resulting value less than 0) happens here +// Warning: (100-105): Overflow (resulting value larger than 2**256 - 1) happens here From bdc2c63327fa52e80182f0ede34d2aee159207a9 Mon Sep 17 00:00:00 2001 From: Mathias Baumann Date: Thu, 28 May 2020 16:09:52 +0200 Subject: [PATCH 25/36] Output error when forward referencing constants in inline assembly --- Changelog.md | 1 + libsolidity/analysis/TypeChecker.cpp | 25 +++++++++++++------ .../const_forward_reference.sol | 8 ++++++ .../invalid/const_forward_reference.sol | 12 +++++++++ 4 files changed, 38 insertions(+), 8 deletions(-) create mode 100644 test/libsolidity/syntaxTests/inlineAssembly/const_forward_reference.sol create mode 100644 test/libsolidity/syntaxTests/inlineAssembly/invalid/const_forward_reference.sol diff --git a/Changelog.md b/Changelog.md index 573d83e5b..ebde3000b 100644 --- a/Changelog.md +++ b/Changelog.md @@ -22,6 +22,7 @@ Bugfixes: * Type Checker: Fix internal compiler error when accessing members of array slices. * Type Checker: Fix internal compiler error when trying to decode too large static arrays. * Type Checker: Fix wrong compiler error when referencing an overridden function without calling it. + * Type Checker: Fix internal compiler error when forward referencing non-literal constants from inline assembly. * NatSpec: DocString block is terminated when encountering an empty line. * Scanner: Fix bug when two empty NatSpec comments lead to scanning past EOL. * Code Generator: Trigger proper unimplemented errors on certain array copy operations. diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index e6694043c..afdc7c869 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -691,14 +691,6 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly) m_errorReporter.typeError(3224_error, _identifier.location, "Constant has no value."); return size_t(-1); } - else if (!var || !type(*var)->isValueType() || ( - dynamic_cast(var->value().get()) == nullptr && - type(*var->value())->category() != Type::Category::RationalNumber - )) - { - m_errorReporter.typeError(7615_error, _identifier.location, "Only direct number constants and references to such constants are supported by inline assembly."); - return size_t(-1); - } else if (_context == yul::IdentifierContext::LValue) { m_errorReporter.typeError(6252_error, _identifier.location, "Constant variables cannot be assigned to."); @@ -709,6 +701,23 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly) m_errorReporter.typeError(6617_error, _identifier.location, "The suffixes _offset and _slot can only be used on non-constant storage variables."); return size_t(-1); } + else if (var && var->value() && !var->value()->annotation().type && !dynamic_cast(var->value().get())) + { + m_errorReporter.typeError( + 2249_error, + _identifier.location, + "Constant variables with non-literal values cannot be forward referenced from inline assembly." + ); + return size_t(-1); + } + else if (!var || !type(*var)->isValueType() || ( + !dynamic_cast(var->value().get()) && + type(*var->value())->category() != Type::Category::RationalNumber + )) + { + m_errorReporter.typeError(7615_error, _identifier.location, "Only direct number constants and references to such constants are supported by inline assembly."); + return size_t(-1); + } } if (requiresStorage) diff --git a/test/libsolidity/syntaxTests/inlineAssembly/const_forward_reference.sol b/test/libsolidity/syntaxTests/inlineAssembly/const_forward_reference.sol new file mode 100644 index 000000000..8c61d318d --- /dev/null +++ b/test/libsolidity/syntaxTests/inlineAssembly/const_forward_reference.sol @@ -0,0 +1,8 @@ +contract C { + function f() public pure { + assembly { + pop(add(add(1, 2), c)) + } + } + int constant c = 1; +} diff --git a/test/libsolidity/syntaxTests/inlineAssembly/invalid/const_forward_reference.sol b/test/libsolidity/syntaxTests/inlineAssembly/invalid/const_forward_reference.sol new file mode 100644 index 000000000..b8d7c0212 --- /dev/null +++ b/test/libsolidity/syntaxTests/inlineAssembly/invalid/const_forward_reference.sol @@ -0,0 +1,12 @@ +contract C { + function f() { + assembly { + c := add(add(1, 2), c) + } + } + int constant c = 0 + 1; +} +// ---- +// SyntaxError: (15-83): No visibility specified. Did you intend to add "public"? +// TypeError: (71-72): Constant variables with non-literal values cannot be forward referenced from inline assembly. +// TypeError: (51-52): Constant variables cannot be assigned to. From 79407c87fb7c54d6924d9262ee3f6a7d6d73cce5 Mon Sep 17 00:00:00 2001 From: ssi91 Date: Mon, 25 May 2020 20:31:24 +0700 Subject: [PATCH 26/36] remove extra definition of printIndented add CommonBaseTestCase class and inherit some of TestCase classes from it. Since that, remove print source extra definitions create the base defifnition for printUpdatedExpectations and remove extra definitions of the method make CommonBaseTestCase c-tor explicit include AnsiColorized.h and sort includes implement a common result checker move the common implementations into TastCase --- test/TestCase.cpp | 35 +++++++++++++++++++++++++--- test/TestCase.h | 9 +++++-- test/libsolidity/ABIJsonTest.cpp | 21 +---------------- test/libsolidity/ABIJsonTest.h | 8 ------- test/libsolidity/ASTJSONTest.h | 1 - test/libsolidity/GasTest.h | 1 - test/libyul/EwasmTranslationTest.cpp | 22 +---------------- test/libyul/EwasmTranslationTest.h | 7 ------ test/libyul/FunctionSideEffects.cpp | 34 +-------------------------- test/libyul/FunctionSideEffects.h | 10 -------- test/libyul/ObjectCompilerTest.cpp | 33 +------------------------- test/libyul/ObjectCompilerTest.h | 7 ------ test/libyul/YulInterpreterTest.cpp | 22 +---------------- test/libyul/YulInterpreterTest.h | 7 ------ test/libyul/YulOptimizerTest.cpp | 22 +---------------- test/libyul/YulOptimizerTest.h | 6 ----- 16 files changed, 45 insertions(+), 200 deletions(-) diff --git a/test/TestCase.cpp b/test/TestCase.cpp index 23f23ac73..e6f4b212a 100644 --- a/test/TestCase.cpp +++ b/test/TestCase.cpp @@ -18,11 +18,13 @@ #include #include -#include -#include +#include + +#include +#include -#include #include +#include using namespace std; using namespace solidity; @@ -78,6 +80,33 @@ void TestCase::printIndented(ostream& _stream, string const& _output, string con _stream << _linePrefix << line << endl; } +void TestCase::printSource(ostream& _stream, string const& _linePrefix, bool const) const +{ + printIndented(_stream, m_source, _linePrefix); +} + +void TestCase::printUpdatedExpectations(ostream& _stream, string const& _linePrefix) const +{ + printIndented(_stream, m_obtainedResult, _linePrefix); +} + +TestCase::TestResult TestCase::checkResult(std::ostream& _stream, const std::string& _linePrefix, bool const _formatted) +{ + if (m_expectation != m_obtainedResult) + { + string nextIndentLevel = _linePrefix + " "; + util::AnsiColorized(_stream, _formatted, {util::formatting::BOLD, util::formatting::CYAN}) + << _linePrefix << "Expected result:" << endl; + // TODO could compute a simple diff with highlighted lines + printIndented(_stream, m_expectation, nextIndentLevel); + util::AnsiColorized(_stream, _formatted, {util::formatting::BOLD, util::formatting::CYAN}) + << _linePrefix << "Obtained result:" << endl; + printIndented(_stream, m_obtainedResult, nextIndentLevel); + return TestResult::Failure; + } + return TestResult::Success; +} + EVMVersionRestrictedTestCase::EVMVersionRestrictedTestCase(string const& _filename): TestCase(_filename) { diff --git a/test/TestCase.h b/test/TestCase.h index 442b4fd5f..f846b7f87 100644 --- a/test/TestCase.h +++ b/test/TestCase.h @@ -57,14 +57,14 @@ public: /// Each line of output is prefixed with @arg _linePrefix. /// If @arg _formatted is true, color-coding may be used to indicate /// error locations in the contract, if applicable. - virtual void printSource(std::ostream &_stream, std::string const &_linePrefix = "", bool const _formatted = false) const = 0; + virtual void printSource(std::ostream &_stream, std::string const &_linePrefix = "", bool const _formatted = false) const; /// Outputs settings. virtual void printSettings(std::ostream &_stream, std::string const &_linePrefix = "", bool const _formatted = false); /// Outputs updated settings virtual void printUpdatedSettings(std::ostream& _stream, std::string const& _linePrefix = ""); /// Outputs test expectations to @arg _stream that match the actual results of the test. /// Each line of output is prefixed with @arg _linePrefix. - virtual void printUpdatedExpectations(std::ostream& _stream, std::string const& _linePrefix) const = 0; + virtual void printUpdatedExpectations(std::ostream& _stream, std::string const& _linePrefix) const; static bool isTestFilename(boost::filesystem::path const& _filename); @@ -97,6 +97,11 @@ protected: } void printIndented(std::ostream& _stream, std::string const& _output, std::string const& _linePrefix = "") const; + TestCase::TestResult checkResult(std::ostream& _stream, const std::string& _linePrefix, bool const _formatted); + + std::string m_source; + std::string m_obtainedResult; + std::string m_expectation; TestCaseReader m_reader; bool m_shouldRun = true; diff --git a/test/libsolidity/ABIJsonTest.cpp b/test/libsolidity/ABIJsonTest.cpp index a46c1d470..aa8153454 100644 --- a/test/libsolidity/ABIJsonTest.cpp +++ b/test/libsolidity/ABIJsonTest.cpp @@ -64,25 +64,6 @@ TestCase::TestResult ABIJsonTest::run(ostream& _stream, string const& _linePrefi m_obtainedResult += jsonPrettyPrint(compiler.contractABI(contractName)) + "\n"; first = false; } - if (m_expectation != m_obtainedResult) - { - string nextIndentLevel = _linePrefix + " "; - AnsiColorized(_stream, _formatted, {formatting::BOLD, formatting::CYAN}) << _linePrefix << "Expected result:" << endl; - printIndented(_stream, m_expectation, nextIndentLevel); - AnsiColorized(_stream, _formatted, {formatting::BOLD, formatting::CYAN}) << _linePrefix << "Obtained result:" << endl; - printIndented(_stream, m_obtainedResult, nextIndentLevel); - return TestResult::Failure; - } - return TestResult::Success; -} - -void ABIJsonTest::printSource(ostream& _stream, string const& _linePrefix, bool const) const -{ - printIndented(_stream, m_source, _linePrefix); -} - -void ABIJsonTest::printUpdatedExpectations(ostream& _stream, string const& _linePrefix) const -{ - printIndented(_stream, m_obtainedResult, _linePrefix); + return checkResult(_stream, _linePrefix, _formatted); } diff --git a/test/libsolidity/ABIJsonTest.h b/test/libsolidity/ABIJsonTest.h index 3a7cd397c..702d36dbf 100644 --- a/test/libsolidity/ABIJsonTest.h +++ b/test/libsolidity/ABIJsonTest.h @@ -36,14 +36,6 @@ public: ABIJsonTest(std::string const& _filename); TestResult run(std::ostream& _stream, std::string const& _linePrefix = "", bool const _formatted = false) override; - - void printSource(std::ostream& _stream, std::string const& _linePrefix = "", bool const _formatted = false) const override; - void printUpdatedExpectations(std::ostream& _stream, std::string const& _linePrefix) const override; - -private: - std::string m_source; - std::string m_expectation; - std::string m_obtainedResult; }; } diff --git a/test/libsolidity/ASTJSONTest.h b/test/libsolidity/ASTJSONTest.h index 92658edd6..45d7a08b9 100644 --- a/test/libsolidity/ASTJSONTest.h +++ b/test/libsolidity/ASTJSONTest.h @@ -41,7 +41,6 @@ public: void printUpdatedExpectations(std::ostream& _stream, std::string const& _linePrefix) const override; private: std::vector> m_sources; - std::string m_expectation; std::string m_expectationLegacy; std::string m_astFilename; std::string m_legacyAstFilename; diff --git a/test/libsolidity/GasTest.h b/test/libsolidity/GasTest.h index 861e47bc9..d8bd0acad 100644 --- a/test/libsolidity/GasTest.h +++ b/test/libsolidity/GasTest.h @@ -47,7 +47,6 @@ private: bool m_optimise = false; bool m_optimiseYul = false; size_t m_optimiseRuns = 200; - std::string m_source; std::map> m_expectations; }; diff --git a/test/libyul/EwasmTranslationTest.cpp b/test/libyul/EwasmTranslationTest.cpp index 46ede4221..8ef5ba0a0 100644 --- a/test/libyul/EwasmTranslationTest.cpp +++ b/test/libyul/EwasmTranslationTest.cpp @@ -71,27 +71,7 @@ TestCase::TestResult EwasmTranslationTest::run(ostream& _stream, string const& _ m_obtainedResult = interpret(); - if (m_expectation != m_obtainedResult) - { - string nextIndentLevel = _linePrefix + " "; - AnsiColorized(_stream, _formatted, {formatting::BOLD, formatting::CYAN}) << _linePrefix << "Expected result:" << endl; - // TODO could compute a simple diff with highlighted lines - printIndented(_stream, m_expectation, nextIndentLevel); - AnsiColorized(_stream, _formatted, {formatting::BOLD, formatting::CYAN}) << _linePrefix << "Obtained result:" << endl; - printIndented(_stream, m_obtainedResult, nextIndentLevel); - return TestResult::Failure; - } - return TestResult::Success; -} - -void EwasmTranslationTest::printSource(ostream& _stream, string const& _linePrefix, bool const) const -{ - printIndented(_stream, m_source, _linePrefix); -} - -void EwasmTranslationTest::printUpdatedExpectations(ostream& _stream, string const& _linePrefix) const -{ - printIndented(_stream, m_obtainedResult, _linePrefix); + return checkResult(_stream, _linePrefix, _formatted); } bool EwasmTranslationTest::parse(ostream& _stream, string const& _linePrefix, bool const _formatted) diff --git a/test/libyul/EwasmTranslationTest.h b/test/libyul/EwasmTranslationTest.h index f9c6e1d3c..e0a90482b 100644 --- a/test/libyul/EwasmTranslationTest.h +++ b/test/libyul/EwasmTranslationTest.h @@ -42,20 +42,13 @@ public: TestResult run(std::ostream& _stream, std::string const& _linePrefix = "", bool const _formatted = false) override; - void printSource(std::ostream& _stream, std::string const &_linePrefix = "", bool const _formatted = false) const override; - void printUpdatedExpectations(std::ostream& _stream, std::string const& _linePrefix) const override; - private: bool parse(std::ostream& _stream, std::string const& _linePrefix, bool const _formatted); std::string interpret(); static void printErrors(std::ostream& _stream, langutil::ErrorList const& _errors); - std::string m_source; - std::string m_expectation; - std::shared_ptr m_object; - std::string m_obtainedResult; }; } diff --git a/test/libyul/FunctionSideEffects.cpp b/test/libyul/FunctionSideEffects.cpp index 3d3461b0e..aa7d1e503 100644 --- a/test/libyul/FunctionSideEffects.cpp +++ b/test/libyul/FunctionSideEffects.cpp @@ -87,37 +87,5 @@ TestCase::TestResult FunctionSideEffects::run(ostream& _stream, string const& _l for (auto const& fun: functionSideEffectsStr) m_obtainedResult += fun.first + ":" + (fun.second.empty() ? "" : " ") + fun.second + "\n"; - if (m_expectation != m_obtainedResult) - { - string nextIndentLevel = _linePrefix + " "; - AnsiColorized(_stream, _formatted, {formatting::BOLD, formatting::CYAN}) << _linePrefix << "Expected result:" << endl; - printIndented(_stream, m_expectation, nextIndentLevel); - AnsiColorized(_stream, _formatted, {formatting::BOLD, formatting::CYAN}) << _linePrefix << "Obtained result:" << endl; - printIndented(_stream, m_obtainedResult, nextIndentLevel); - return TestResult::Failure; - } - return TestResult::Success; -} - - -void FunctionSideEffects::printSource(ostream& _stream, string const& _linePrefix, bool const) const -{ - printIndented(_stream, m_source, _linePrefix); -} - -void FunctionSideEffects::printUpdatedExpectations(ostream& _stream, string const& _linePrefix) const -{ - printIndented(_stream, m_obtainedResult, _linePrefix); -} - -void FunctionSideEffects::printIndented(ostream& _stream, string const& _output, string const& _linePrefix) const -{ - stringstream output(_output); - string line; - while (getline(output, line)) - if (line.empty()) - // Avoid trailing spaces. - _stream << boost::trim_right_copy(_linePrefix) << endl; - else - _stream << _linePrefix << line << endl; + return checkResult(_stream, _linePrefix, _formatted); } diff --git a/test/libyul/FunctionSideEffects.h b/test/libyul/FunctionSideEffects.h index b35994b00..23bebebcc 100644 --- a/test/libyul/FunctionSideEffects.h +++ b/test/libyul/FunctionSideEffects.h @@ -36,16 +36,6 @@ public: explicit FunctionSideEffects(std::string const& _filename); TestResult run(std::ostream& _stream, std::string const& _linePrefix = "", bool const _formatted = false) override; - - void printSource(std::ostream& _stream, std::string const& _linePrefix = "", bool const _formatted = false) const override; - void printUpdatedExpectations(std::ostream& _stream, std::string const& _linePrefix) const override; - -private: - void printIndented(std::ostream& _stream, std::string const& _output, std::string const& _linePrefix = "") const; - - std::string m_source; - std::string m_expectation; - std::string m_obtainedResult; }; } diff --git a/test/libyul/ObjectCompilerTest.cpp b/test/libyul/ObjectCompilerTest.cpp index 9dd589748..4b34da7b1 100644 --- a/test/libyul/ObjectCompilerTest.cpp +++ b/test/libyul/ObjectCompilerTest.cpp @@ -78,38 +78,7 @@ TestCase::TestResult ObjectCompilerTest::run(ostream& _stream, string const& _li (obj.sourceMappings->empty() ? "" : " " + *obj.sourceMappings) + "\n"; - if (m_expectation != m_obtainedResult) - { - string nextIndentLevel = _linePrefix + " "; - AnsiColorized(_stream, _formatted, {formatting::BOLD, formatting::CYAN}) << _linePrefix << "Expected result:" << endl; - printIndented(_stream, m_expectation, nextIndentLevel); - AnsiColorized(_stream, _formatted, {formatting::BOLD, formatting::CYAN}) << _linePrefix << "Obtained result:" << endl; - printIndented(_stream, m_obtainedResult, nextIndentLevel); - return TestResult::Failure; - } - return TestResult::Success; -} - -void ObjectCompilerTest::printSource(ostream& _stream, string const& _linePrefix, bool const) const -{ - printIndented(_stream, m_source, _linePrefix); -} - -void ObjectCompilerTest::printUpdatedExpectations(ostream& _stream, string const& _linePrefix) const -{ - printIndented(_stream, m_obtainedResult, _linePrefix); -} - -void ObjectCompilerTest::printIndented(ostream& _stream, string const& _output, string const& _linePrefix) const -{ - stringstream output(_output); - string line; - while (getline(output, line)) - if (line.empty()) - // Avoid trailing spaces. - _stream << boost::trim_right_copy(_linePrefix) << endl; - else - _stream << _linePrefix << line << endl; + return checkResult(_stream, _linePrefix, _formatted); } void ObjectCompilerTest::printErrors(ostream& _stream, ErrorList const& _errors) diff --git a/test/libyul/ObjectCompilerTest.h b/test/libyul/ObjectCompilerTest.h index dc02d504c..4a1c75486 100644 --- a/test/libyul/ObjectCompilerTest.h +++ b/test/libyul/ObjectCompilerTest.h @@ -47,20 +47,13 @@ public: TestResult run(std::ostream& _stream, std::string const& _linePrefix = "", bool const _formatted = false) override; - void printSource(std::ostream& _stream, std::string const &_linePrefix = "", bool const _formatted = false) const override; - void printUpdatedExpectations(std::ostream& _stream, std::string const& _linePrefix) const override; - private: - void printIndented(std::ostream& _stream, std::string const& _output, std::string const& _linePrefix = "") const; bool parse(std::ostream& _stream, std::string const& _linePrefix, bool const _formatted); void disambiguate(); static void printErrors(std::ostream& _stream, langutil::ErrorList const& _errors); - std::string m_source; bool m_optimize = false; - std::string m_expectation; - std::string m_obtainedResult; }; } diff --git a/test/libyul/YulInterpreterTest.cpp b/test/libyul/YulInterpreterTest.cpp index 69e2c8d10..9925b9546 100644 --- a/test/libyul/YulInterpreterTest.cpp +++ b/test/libyul/YulInterpreterTest.cpp @@ -59,27 +59,7 @@ TestCase::TestResult YulInterpreterTest::run(ostream& _stream, string const& _li m_obtainedResult = interpret(); - if (m_expectation != m_obtainedResult) - { - string nextIndentLevel = _linePrefix + " "; - AnsiColorized(_stream, _formatted, {formatting::BOLD, formatting::CYAN}) << _linePrefix << "Expected result:" << endl; - // TODO could compute a simple diff with highlighted lines - printIndented(_stream, m_expectation, nextIndentLevel); - AnsiColorized(_stream, _formatted, {formatting::BOLD, formatting::CYAN}) << _linePrefix << "Obtained result:" << endl; - printIndented(_stream, m_obtainedResult, nextIndentLevel); - return TestResult::Failure; - } - return TestResult::Success; -} - -void YulInterpreterTest::printSource(ostream& _stream, string const& _linePrefix, bool const) const -{ - printIndented(_stream, m_source, _linePrefix); -} - -void YulInterpreterTest::printUpdatedExpectations(ostream& _stream, string const& _linePrefix) const -{ - printIndented(_stream, m_obtainedResult, _linePrefix); + return checkResult(_stream, _linePrefix, _formatted); } bool YulInterpreterTest::parse(ostream& _stream, string const& _linePrefix, bool const _formatted) diff --git a/test/libyul/YulInterpreterTest.h b/test/libyul/YulInterpreterTest.h index ec60dbaf4..ee60abe17 100644 --- a/test/libyul/YulInterpreterTest.h +++ b/test/libyul/YulInterpreterTest.h @@ -47,21 +47,14 @@ public: TestResult run(std::ostream& _stream, std::string const& _linePrefix = "", bool const _formatted = false) override; - void printSource(std::ostream& _stream, std::string const &_linePrefix = "", bool const _formatted = false) const override; - void printUpdatedExpectations(std::ostream& _stream, std::string const& _linePrefix) const override; - private: bool parse(std::ostream& _stream, std::string const& _linePrefix, bool const _formatted); std::string interpret(); static void printErrors(std::ostream& _stream, langutil::ErrorList const& _errors); - std::string m_source; - std::string m_expectation; - std::shared_ptr m_ast; std::shared_ptr m_analysisInfo; - std::string m_obtainedResult; }; } diff --git a/test/libyul/YulOptimizerTest.cpp b/test/libyul/YulOptimizerTest.cpp index 042d528db..bda7c6514 100644 --- a/test/libyul/YulOptimizerTest.cpp +++ b/test/libyul/YulOptimizerTest.cpp @@ -354,27 +354,7 @@ TestCase::TestResult YulOptimizerTest::run(ostream& _stream, string const& _line m_obtainedResult = "step: " + m_optimizerStep + "\n\n" + AsmPrinter{ *m_dialect }(*m_ast) + "\n"; - if (m_expectation != m_obtainedResult) - { - string nextIndentLevel = _linePrefix + " "; - AnsiColorized(_stream, _formatted, {formatting::BOLD, formatting::CYAN}) << _linePrefix << "Expected result:" << endl; - // TODO could compute a simple diff with highlighted lines - printIndented(_stream, m_expectation, nextIndentLevel); - AnsiColorized(_stream, _formatted, {formatting::BOLD, formatting::CYAN}) << _linePrefix << "Obtained result:" << endl; - printIndented(_stream, m_obtainedResult, nextIndentLevel); - return TestResult::Failure; - } - return TestResult::Success; -} - -void YulOptimizerTest::printSource(ostream& _stream, string const& _linePrefix, bool const) const -{ - printIndented(_stream, m_source, _linePrefix); -} - -void YulOptimizerTest::printUpdatedExpectations(ostream& _stream, string const& _linePrefix) const -{ - printIndented(_stream, m_obtainedResult, _linePrefix); + return checkResult(_stream, _linePrefix, _formatted); } bool YulOptimizerTest::parse(ostream& _stream, string const& _linePrefix, bool const _formatted) diff --git a/test/libyul/YulOptimizerTest.h b/test/libyul/YulOptimizerTest.h index 374ea9b19..3d351c3a2 100644 --- a/test/libyul/YulOptimizerTest.h +++ b/test/libyul/YulOptimizerTest.h @@ -56,9 +56,6 @@ public: TestResult run(std::ostream& _stream, std::string const& _linePrefix = "", bool const _formatted = false) override; - void printSource(std::ostream& _stream, std::string const &_linePrefix = "", bool const _formatted = false) const override; - void printUpdatedExpectations(std::ostream& _stream, std::string const& _linePrefix) const override; - private: bool parse(std::ostream& _stream, std::string const& _linePrefix, bool const _formatted); void disambiguate(); @@ -66,9 +63,7 @@ private: static void printErrors(std::ostream& _stream, langutil::ErrorList const& _errors); - std::string m_source; std::string m_optimizerStep; - std::string m_expectation; Dialect const* m_dialect = nullptr; std::set m_reservedIdentifiers; @@ -77,7 +72,6 @@ private: std::shared_ptr m_ast; std::shared_ptr m_analysisInfo; - std::string m_obtainedResult; }; } From 1ee6c490283a11ff796b8a7dfe91390a3955cc6a Mon Sep 17 00:00:00 2001 From: Djordje Mijovic Date: Tue, 2 Jun 2020 15:46:37 +0200 Subject: [PATCH 27/36] Adding fixes for signedness warnings in test/tools/yulInterpreter --- test/tools/yulInterpreter/EVMInstructionInterpreter.cpp | 8 ++++---- test/tools/yulInterpreter/EwasmBuiltinInterpreter.cpp | 6 +++--- test/tools/yulInterpreter/Interpreter.cpp | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/test/tools/yulInterpreter/EVMInstructionInterpreter.cpp b/test/tools/yulInterpreter/EVMInstructionInterpreter.cpp index d60b63b10..9c46ea856 100644 --- a/test/tools/yulInterpreter/EVMInstructionInterpreter.cpp +++ b/test/tools/yulInterpreter/EVMInstructionInterpreter.cpp @@ -48,10 +48,10 @@ u256 readZeroExtended(bytes const& _data, u256 const& _offset) if (_offset >= _data.size()) return 0; else if (_offset + 32 <= _data.size()) - return *reinterpret_cast(_data.data() + size_t(_offset)); + return *reinterpret_cast(_data.data() + static_cast(_offset)); else { - size_t off = size_t(_offset); + size_t off = static_cast(_offset); u256 val; for (size_t i = 0; i < 32; ++i) { @@ -88,7 +88,7 @@ u256 EVMInstructionInterpreter::eval( using evmasm::Instruction; auto info = instructionInfo(_instruction); - yulAssert(size_t(info.args) == _arguments.size(), ""); + yulAssert(static_cast(info.args) == _arguments.size(), ""); auto const& arg = _arguments; switch (_instruction) @@ -442,7 +442,7 @@ u256 EVMInstructionInterpreter::evalBuiltin(BuiltinFunctionForEVM const& _fun, c m_state.memory, m_state.code, size_t(_arguments.at(0)), - size_t(_arguments.at(1) & size_t(-1)), + size_t(_arguments.at(1) & numeric_limits::max()), size_t(_arguments.at(2)) ); } diff --git a/test/tools/yulInterpreter/EwasmBuiltinInterpreter.cpp b/test/tools/yulInterpreter/EwasmBuiltinInterpreter.cpp index e4c190e9c..b523e7efe 100644 --- a/test/tools/yulInterpreter/EwasmBuiltinInterpreter.cpp +++ b/test/tools/yulInterpreter/EwasmBuiltinInterpreter.cpp @@ -87,9 +87,9 @@ u256 EwasmBuiltinInterpreter::evalBuiltin(YulString _fun, vector const& _a copyZeroExtended( m_state.memory, m_state.code, - size_t(_arguments.at(0)), - size_t(_arguments.at(1) & size_t(-1)), - size_t(_arguments.at(2)) + static_cast(_arguments.at(0)), + static_cast(_arguments.at(1) & numeric_limits::max()), + static_cast(_arguments.at(2)) ); return 0; } diff --git a/test/tools/yulInterpreter/Interpreter.cpp b/test/tools/yulInterpreter/Interpreter.cpp index 6f989b873..5f0861d14 100644 --- a/test/tools/yulInterpreter/Interpreter.cpp +++ b/test/tools/yulInterpreter/Interpreter.cpp @@ -54,7 +54,7 @@ void InterpreterState::dumpTraceAndState(ostream& _out) const _out << "Memory dump:\n"; map words; for (auto const& [offset, value]: memory) - words[(offset / 0x20) * 0x20] |= u256(uint32_t(value)) << (256 - 8 - 8 * size_t(offset % 0x20)); + words[(offset / 0x20) * 0x20] |= u256(uint32_t(value)) << (256 - 8 - 8 * static_cast(offset % 0x20)); for (auto const& [offset, value]: words) if (value != 0) _out << " " << std::uppercase << std::hex << std::setw(4) << offset << ": " << h256(value).hex() << endl; From 10162016ae9db4bdb07041acf261bed3dafb1e5d Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Fri, 29 May 2020 19:00:40 +0200 Subject: [PATCH 28/36] [SMTChecker] Fix internal error on try/catch --- Changelog.md | 1 + libsolidity/formal/SMTEncoder.cpp | 14 ++++++++++++++ libsolidity/formal/SMTEncoder.h | 1 + .../smtCheckerTests/control_flow/try_catch_1.sol | 12 ++++++++++++ .../smtCheckerTests/control_flow/try_catch_2.sol | 14 ++++++++++++++ 5 files changed, 42 insertions(+) create mode 100644 test/libsolidity/smtCheckerTests/control_flow/try_catch_1.sol create mode 100644 test/libsolidity/smtCheckerTests/control_flow/try_catch_2.sol diff --git a/Changelog.md b/Changelog.md index 9f06339b8..9f716c799 100644 --- a/Changelog.md +++ b/Changelog.md @@ -28,6 +28,7 @@ Bugfixes: * SMTChecker: Fix internal error when applying arithmetic operators to fixed point variables. * SMTChecker: Fix internal error when short circuiting Boolean expressions with function calls in state variable initialization. * SMTChecker: Fix internal error when assigning to index access inside branches. + * SMTChecker: Fix internal error on try/catch clauses with parameters. ### 0.6.8 (2020-05-14) diff --git a/libsolidity/formal/SMTEncoder.cpp b/libsolidity/formal/SMTEncoder.cpp index 8ed8d71fa..b06abbe6e 100644 --- a/libsolidity/formal/SMTEncoder.cpp +++ b/libsolidity/formal/SMTEncoder.cpp @@ -274,6 +274,20 @@ bool SMTEncoder::visit(InlineAssembly const& _inlineAsm) return false; } +bool SMTEncoder::visit(TryCatchClause const& _clause) +{ + if (auto params = _clause.parameters()) + for (auto const& var: params->parameters()) + createVariable(*var); + + m_errorReporter.warning( + 7645_error, + _clause.location(), + "Assertion checker does not support try/catch clauses." + ); + return false; +} + bool SMTEncoder::visit(IfStatement const& _node) { _node.condition().accept(*this); diff --git a/libsolidity/formal/SMTEncoder.h b/libsolidity/formal/SMTEncoder.h index 8010ba820..0e4c170fc 100644 --- a/libsolidity/formal/SMTEncoder.h +++ b/libsolidity/formal/SMTEncoder.h @@ -91,6 +91,7 @@ protected: bool visit(InlineAssembly const& _node) override; void endVisit(Break const&) override {} void endVisit(Continue const&) override {} + bool visit(TryCatchClause const& _node) override; /// Do not visit subtree if node is a RationalNumber. /// Symbolic _expr is the rational literal. diff --git a/test/libsolidity/smtCheckerTests/control_flow/try_catch_1.sol b/test/libsolidity/smtCheckerTests/control_flow/try_catch_1.sol new file mode 100644 index 000000000..b41027ed4 --- /dev/null +++ b/test/libsolidity/smtCheckerTests/control_flow/try_catch_1.sol @@ -0,0 +1,12 @@ +pragma experimental SMTChecker; +contract C { + function g() public returns (uint) { + try this.g() returns (uint x) { x; } + catch Error(string memory s) { s; } + } +} +// ==== +// EVMVersion: >=byzantium +// ---- +// Warning: (98-121): Assertion checker does not support try/catch clauses. +// Warning: (124-159): Assertion checker does not support try/catch clauses. diff --git a/test/libsolidity/smtCheckerTests/control_flow/try_catch_2.sol b/test/libsolidity/smtCheckerTests/control_flow/try_catch_2.sol new file mode 100644 index 000000000..96de08886 --- /dev/null +++ b/test/libsolidity/smtCheckerTests/control_flow/try_catch_2.sol @@ -0,0 +1,14 @@ +pragma experimental SMTChecker; +contract C { + function f() public { + try this.f() {} + catch (bytes memory x) { + x; + } + } +} +// ==== +// EVMVersion: >=byzantium +// ---- +// Warning: (83-85): Assertion checker does not support try/catch clauses. +// Warning: (88-122): Assertion checker does not support try/catch clauses. From 6f75476f52e9a992f5d89ccd3656b0dd2f9b1e2f Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 2 Jun 2020 10:23:15 +0200 Subject: [PATCH 29/36] Fix documentation about return values. --- docs/contracts/functions.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/contracts/functions.rst b/docs/contracts/functions.rst index f33258fe1..acdfc0185 100644 --- a/docs/contracts/functions.rst +++ b/docs/contracts/functions.rst @@ -76,7 +76,7 @@ are initialized with their :ref:`default value ` and have that value until they are (re-)assigned. You can either explicitly assign to return variables and -then leave the function using ``return;``, +then leave the function as above, or you can provide return values (either a single or :ref:`multiple ones`) directly with the ``return`` statement:: @@ -94,8 +94,8 @@ statement:: } } -This form is equivalent to first assigning values to the -return variables and then using ``return;`` to leave the function. +If you use an early ``return`` to leave a function that has return variables, +you must provide return values together with the return statement. .. note:: You cannot return some types from non-internal functions, notably From d2924d83a2768ef749904d1d05a3b972b4c60436 Mon Sep 17 00:00:00 2001 From: Djordje Mijovic Date: Tue, 2 Jun 2020 15:37:45 +0200 Subject: [PATCH 30/36] Adding fixes for signedness warnings in smtutil --- libsmtutil/CVC4Interface.cpp | 2 +- libsmtutil/SMTLib2Interface.cpp | 2 +- libsmtutil/Z3Interface.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libsmtutil/CVC4Interface.cpp b/libsmtutil/CVC4Interface.cpp index 4c1ef378a..2af305e0e 100644 --- a/libsmtutil/CVC4Interface.cpp +++ b/libsmtutil/CVC4Interface.cpp @@ -201,7 +201,7 @@ CVC4::Expr CVC4Interface::toCVC4Expr(Expression const& _expr) smtAssert(tupleSort, ""); CVC4::DatatypeType tt = m_context.mkTupleType(cvc4Sort(tupleSort->components)); CVC4::Datatype const& dt = tt.getDatatype(); - size_t index = std::stoi(_expr.arguments[1].name); + size_t index = std::stoul(_expr.arguments[1].name); CVC4::Expr s = dt[0][index].getSelector(); return m_context.mkExpr(CVC4::kind::APPLY_SELECTOR, s, arguments[0]); } diff --git a/libsmtutil/SMTLib2Interface.cpp b/libsmtutil/SMTLib2Interface.cpp index 4e81444fe..94da07aec 100644 --- a/libsmtutil/SMTLib2Interface.cpp +++ b/libsmtutil/SMTLib2Interface.cpp @@ -151,7 +151,7 @@ string SMTLib2Interface::toSExpr(Expression const& _expr) { smtAssert(_expr.arguments.size() == 2, ""); auto tupleSort = dynamic_pointer_cast(_expr.arguments.at(0).sort); - unsigned index = std::stoi(_expr.arguments.at(1).name); + size_t index = std::stoul(_expr.arguments.at(1).name); smtAssert(index < tupleSort->members.size(), ""); sexpr += "|" + tupleSort->members.at(index) + "| " + toSExpr(_expr.arguments.at(0)); } diff --git a/libsmtutil/Z3Interface.cpp b/libsmtutil/Z3Interface.cpp index b42bc4765..fed82cc8b 100644 --- a/libsmtutil/Z3Interface.cpp +++ b/libsmtutil/Z3Interface.cpp @@ -194,7 +194,7 @@ z3::expr Z3Interface::toZ3Expr(Expression const& _expr) } else if (n == "tuple_get") { - size_t index = std::stoi(_expr.arguments[1].name); + size_t index = stoul(_expr.arguments[1].name); return z3::func_decl(m_context, Z3_get_tuple_sort_field_decl(m_context, z3Sort(*_expr.arguments[0].sort), index))(arguments[0]); } else if (n == "tuple_constructor") From b4454c49255e0276738f8512684c01682bb92110 Mon Sep 17 00:00:00 2001 From: Djordje Mijovic Date: Tue, 2 Jun 2020 15:40:27 +0200 Subject: [PATCH 31/36] Adding fixes for signedness warnings in libsolutil --- libsolutil/CommonData.cpp | 9 ++++++--- libsolutil/CommonData.h | 8 ++++---- libsolutil/CommonIO.cpp | 6 +++--- libsolutil/IpfsHash.cpp | 2 +- libsolutil/StringUtils.h | 8 ++++---- libsolutil/picosha2.h | 22 +++++++++++----------- test/libsolutil/IpfsHash.cpp | 8 ++++---- 7 files changed, 33 insertions(+), 30 deletions(-) diff --git a/libsolutil/CommonData.cpp b/libsolutil/CommonData.cpp index bb1a7696c..72071acde 100644 --- a/libsolutil/CommonData.cpp +++ b/libsolutil/CommonData.cpp @@ -64,15 +64,15 @@ string solidity::util::toHex(bytes const& _data, HexPrefix _prefix, HexCase _cas // Mixed case will be handled inside the loop. char const* chars = _case == HexCase::Upper ? upperHexChars : lowerHexChars; - int rix = _data.size() - 1; + size_t rix = _data.size() - 1; for (uint8_t c: _data) { // switch hex case every four hexchars if (_case == HexCase::Mixed) chars = (rix-- & 2) == 0 ? lowerHexChars : upperHexChars; - ret[i++] = chars[(unsigned(c) / 16) & 0xf]; - ret[i++] = chars[unsigned(c) & 0xf]; + ret[i++] = chars[(static_cast(c) >> 4ul) & 0xfu]; + ret[i++] = chars[c & 0xfu]; } assertThrow(i == ret.size(), Exception, ""); @@ -95,6 +95,9 @@ int solidity::util::fromHex(char _i, WhenError _throw) bytes solidity::util::fromHex(std::string const& _s, WhenError _throw) { + if (_s.empty()) + return {}; + unsigned s = (_s.size() >= 2 && _s[0] == '0' && _s[1] == 'x') ? 2 : 0; std::vector ret; ret.reserve((_s.size() - s + 1) / 2); diff --git a/libsolutil/CommonData.h b/libsolutil/CommonData.h index adac1fb05..a5c045646 100644 --- a/libsolutil/CommonData.h +++ b/libsolutil/CommonData.h @@ -42,7 +42,7 @@ template std::vector& operator+=(std::vector& _a, U& _b) { for (auto const& i: _b) - _a.push_back(i); + _a.push_back(T(i)); return _a; } /// Concatenate the contents of a container onto a vector, move variant. @@ -299,7 +299,7 @@ template inline bytes toCompactBigEndian(T _val, unsigned _min = 0) { static_assert(std::is_same::value || !std::numeric_limits::is_signed, "only unsigned types or bigint supported"); //bigint does not carry sign bit on shift - int i = 0; + unsigned i = 0; for (T v = _val; v; ++i, v >>= 8) {} bytes ret(std::max(_min, i), 0); toBigEndian(_val, ret); @@ -379,7 +379,7 @@ void iterateReplacing(std::vector& _vector, F const& _f) { if (!useModified) { - std::move(_vector.begin(), _vector.begin() + i, back_inserter(modifiedVector)); + std::move(_vector.begin(), _vector.begin() + ptrdiff_t(i), back_inserter(modifiedVector)); useModified = true; } modifiedVector += std::move(*r); @@ -406,7 +406,7 @@ void iterateReplacingWindow(std::vector& _vector, F const& _f, std::index_seq { if (!useModified) { - std::move(_vector.begin(), _vector.begin() + i, back_inserter(modifiedVector)); + std::move(_vector.begin(), _vector.begin() + ptrdiff_t(i), back_inserter(modifiedVector)); useModified = true; } modifiedVector += std::move(*r); diff --git a/libsolutil/CommonIO.cpp b/libsolutil/CommonIO.cpp index ec6c33538..cbe68f0d1 100644 --- a/libsolutil/CommonIO.cpp +++ b/libsolutil/CommonIO.cpp @@ -56,7 +56,7 @@ inline T readFile(std::string const& _file) return ret; // do not read empty file (MSVC does not like it) is.seekg(0, is.beg); - ret.resize((length + c_elementSize - 1) / c_elementSize); + ret.resize((static_cast(length) + c_elementSize - 1) / c_elementSize); is.read(const_cast(reinterpret_cast(ret.data())), length); return ret; } @@ -107,8 +107,8 @@ public: DisableConsoleBuffering() { tcgetattr(0, &m_termios); - m_termios.c_lflag &= ~ICANON; - m_termios.c_lflag &= ~ECHO; + m_termios.c_lflag &= ~tcflag_t(ICANON); + m_termios.c_lflag &= ~tcflag_t(ECHO); m_termios.c_cc[VMIN] = 1; m_termios.c_cc[VTIME] = 0; tcsetattr(0, TCSANOW, &m_termios); diff --git a/libsolutil/IpfsHash.cpp b/libsolutil/IpfsHash.cpp index 1f9435748..6913d42f0 100644 --- a/libsolutil/IpfsHash.cpp +++ b/libsolutil/IpfsHash.cpp @@ -62,7 +62,7 @@ string base58Encode(bytes const& _data) string output; while (data) { - output += alphabet[size_t(data % alphabet.size())]; + output += alphabet[static_cast(data % alphabet.size())]; data /= alphabet.size(); } reverse(output.begin(), output.end()); diff --git a/libsolutil/StringUtils.h b/libsolutil/StringUtils.h index 98158aa6d..caea637a2 100644 --- a/libsolutil/StringUtils.h +++ b/libsolutil/StringUtils.h @@ -156,14 +156,14 @@ inline std::string formatNumberReadable( if (_useTruncation) { // return as interior-truncated hex. - int len = str.size(); + size_t len = str.size(); if (len < 24) return str; - int const initialChars = (prefix == HexPrefix::Add) ? 6 : 4; - int const finalChars = 4; - int numSkipped = len - initialChars - finalChars; + size_t const initialChars = (prefix == HexPrefix::Add) ? 6 : 4; + size_t const finalChars = 4; + size_t numSkipped = len - initialChars - finalChars; return str.substr(0, initialChars) + "...{+" + diff --git a/libsolutil/picosha2.h b/libsolutil/picosha2.h index fdfabb5ac..6994455a2 100644 --- a/libsolutil/picosha2.h +++ b/libsolutil/picosha2.h @@ -95,10 +95,10 @@ void hash256_block(RaIter1 message_digest, RaIter2 first, RaIter2 last) { word_t w[64]; std::fill(w, w + 64, 0); for (std::size_t i = 0; i < 16; ++i) { - w[i] = (static_cast(mask_8bit(*(first + i * 4))) << 24) | - (static_cast(mask_8bit(*(first + i * 4 + 1))) << 16) | - (static_cast(mask_8bit(*(first + i * 4 + 2))) << 8) | - (static_cast(mask_8bit(*(first + i * 4 + 3)))); + w[i] = (static_cast(mask_8bit(*(first + long(i) * 4))) << 24) | + (static_cast(mask_8bit(*(first + long(i) * 4 + 1))) << 16) | + (static_cast(mask_8bit(*(first + long(i) * 4 + 2))) << 8) | + (static_cast(mask_8bit(*(first + long(i) * 4 + 3)))); } for (std::size_t i = 16; i < 64; ++i) { w[i] = mask_32bit(ssig1(w[i - 2]) + w[i - 7] + ssig0(w[i - 15]) + @@ -196,10 +196,10 @@ class hash256_one_by_one { std::copy(first, last, std::back_inserter(buffer_)); std::size_t i = 0; for (; i + 64 <= buffer_.size(); i += 64) { - detail::hash256_block(h_, buffer_.begin() + i, - buffer_.begin() + i + 64); + detail::hash256_block(h_, buffer_.begin() + long(i), + buffer_.begin() + long(i) + 64); } - buffer_.erase(buffer_.begin(), buffer_.begin() + i); + buffer_.erase(buffer_.begin(), buffer_.begin() + long(i)); } void finish() { @@ -297,20 +297,20 @@ void hash256_impl(RaIter first, RaIter last, OutIter first2, OutIter last2, int, template void hash256_impl(InputIter first, InputIter last, OutIter first2, - OutIter last2, int buffer_size, std::input_iterator_tag) { + OutIter last2, size_t buffer_size, std::input_iterator_tag) { std::vector buffer(buffer_size); hash256_one_by_one hasher; // hasher.init(); while (first != last) { - int size = buffer_size; - for (int i = 0; i != buffer_size; ++i, ++first) { + size_t size = buffer_size; + for (size_t i = 0; i != buffer_size; ++i, ++first) { if (first == last) { size = i; break; } buffer[i] = *first; } - hasher.process(buffer.begin(), buffer.begin() + size); + hasher.process(buffer.begin(), buffer.begin() + ptrdiff_t(size)); } hasher.finish(); hasher.get_hash_bytes(first2, last2); diff --git a/test/libsolutil/IpfsHash.cpp b/test/libsolutil/IpfsHash.cpp index 9c4fb5633..9d6f0f8fc 100644 --- a/test/libsolutil/IpfsHash.cpp +++ b/test/libsolutil/IpfsHash.cpp @@ -36,10 +36,10 @@ BOOST_AUTO_TEST_CASE(test_small) BOOST_CHECK_EQUAL(ipfsHashBase58({}), "QmbFMke1KXqnYyBBWxB74N4c5SBnJMVAiMNRcGu6x1AwQH"); BOOST_CHECK_EQUAL(ipfsHashBase58("x"), "QmULKig5Fxrs2sC4qt9nNduucXfb92AFYQ6Hi3YRqDmrYC"); BOOST_CHECK_EQUAL(ipfsHashBase58("Solidity\n"), "QmSsm9M7PQRBnyiz1smizk8hZw3URfk8fSeHzeTo3oZidS"); - BOOST_CHECK_EQUAL(ipfsHashBase58(string(size_t(200), char(0))), "QmSXR1N23uWzsANi8wpxMPw5dmmhqBVUAb4hUrHVLpNaMr"); - BOOST_CHECK_EQUAL(ipfsHashBase58(string(size_t(10250), char(0))), "QmVJJBB3gKKBWYC9QTywpH8ZL1bDeTDJ17B63Af5kino9i"); - BOOST_CHECK_EQUAL(ipfsHashBase58(string(size_t(100000), char(0))), "QmYgKa25YqEGpQmmZtPPFMNK3kpqqneHk6nMSEUYryEX1C"); - BOOST_CHECK_EQUAL(ipfsHashBase58(string(size_t(121071), char(0))), "QmdMdRshQmqvyc92N82r7AKYdUF5FRh4DJo6GtrmEk3wgj"); + BOOST_CHECK_EQUAL(ipfsHashBase58(string(200ul, char(0))), "QmSXR1N23uWzsANi8wpxMPw5dmmhqBVUAb4hUrHVLpNaMr"); + BOOST_CHECK_EQUAL(ipfsHashBase58(string(10250ul, char(0))), "QmVJJBB3gKKBWYC9QTywpH8ZL1bDeTDJ17B63Af5kino9i"); + BOOST_CHECK_EQUAL(ipfsHashBase58(string(100000ul, char(0))), "QmYgKa25YqEGpQmmZtPPFMNK3kpqqneHk6nMSEUYryEX1C"); + BOOST_CHECK_EQUAL(ipfsHashBase58(string(121071ul, char(0))), "QmdMdRshQmqvyc92N82r7AKYdUF5FRh4DJo6GtrmEk3wgj"); } BOOST_AUTO_TEST_CASE(test_medium) From 8ab8d5b1b0e2f15213cefb9035dc2261b7cda4d8 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 2 Jun 2020 19:55:33 +0200 Subject: [PATCH 32/36] Do not try compiling via yul if explicitly forbidden. --- test/libsolidity/SemanticTest.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/libsolidity/SemanticTest.cpp b/test/libsolidity/SemanticTest.cpp index 33d40c899..39744fa59 100644 --- a/test/libsolidity/SemanticTest.cpp +++ b/test/libsolidity/SemanticTest.cpp @@ -62,6 +62,8 @@ SemanticTest::SemanticTest(string const& _filename, langutil::EVMVersion _evmVer { m_runWithYul = false; m_runWithoutYul = true; + // Do not try to run via yul if explicitly denied. + m_enforceViaYul = false; } else BOOST_THROW_EXCEPTION(runtime_error("Invalid compileViaYul value: " + choice + ".")); From 41d8420718abab087d4f292fe59e7153e95e625b Mon Sep 17 00:00:00 2001 From: a3d4 Date: Sun, 31 May 2020 05:08:37 +0200 Subject: [PATCH 33/36] Fix tabs in SolcReferenceFormatterHuman --- liblangutil/SourceReferenceFormatterHuman.cpp | 39 ++++++++++++------- libsolutil/UTF8.cpp | 9 ----- libsolutil/UTF8.h | 2 - test/cmdlineTests/message_format_utf16/err | 11 ------ .../message_format_utf16/input.sol | 3 -- test/cmdlineTests/message_format_utf8/err | 35 +++++++++++++++++ .../message_format_utf8/input.sol | 15 +++++++ 7 files changed, 75 insertions(+), 39 deletions(-) delete mode 100644 test/cmdlineTests/message_format_utf16/err delete mode 100644 test/cmdlineTests/message_format_utf16/input.sol create mode 100644 test/cmdlineTests/message_format_utf8/err create mode 100644 test/cmdlineTests/message_format_utf8/input.sol diff --git a/liblangutil/SourceReferenceFormatterHuman.cpp b/liblangutil/SourceReferenceFormatterHuman.cpp index cbf24cf35..b8fdba163 100644 --- a/liblangutil/SourceReferenceFormatterHuman.cpp +++ b/liblangutil/SourceReferenceFormatterHuman.cpp @@ -23,6 +23,7 @@ #include #include #include +#include using namespace std; using namespace solidity; @@ -30,6 +31,20 @@ using namespace solidity::langutil; using namespace solidity::util; using namespace solidity::util::formatting; +namespace +{ + +std::string replaceNonTabs(std::string_view _utf8Input, char _filler) +{ + std::string output; + for (char const c: _utf8Input) + if ((c & 0xc0) != 0x80) + output.push_back(c == '\t' ? '\t' : _filler); + return output; +} + +} + AnsiColorized SourceReferenceFormatterHuman::normalColored() const { return AnsiColorized(m_stream, m_colored, {WHITE}); @@ -85,6 +100,8 @@ void SourceReferenceFormatterHuman::printSourceLocation(SourceReference const& _ frameColored() << "-->"; m_stream << ' ' << _ref.sourceName << ':' << line << ':' << (_ref.position.column + 1) << ":\n"; + string_view text = _ref.text; + if (!_ref.multiline) { int const locationLength = _ref.endColumn - _ref.startColumn; @@ -96,21 +113,15 @@ void SourceReferenceFormatterHuman::printSourceLocation(SourceReference const& _ // line 2: frameColored() << line << " |"; - m_stream << ' ' << _ref.text.substr(0, _ref.startColumn); - highlightColored() << _ref.text.substr(_ref.startColumn, locationLength); - m_stream << _ref.text.substr(_ref.endColumn) << '\n'; + m_stream << ' ' << text.substr(0, _ref.startColumn); + highlightColored() << text.substr(_ref.startColumn, locationLength); + m_stream << text.substr(_ref.endColumn) << '\n'; // line 3: m_stream << leftpad << ' '; frameColored() << '|'; - m_stream << ' '; - - for_each( - _ref.text.cbegin(), - _ref.text.cbegin() + numCodepoints(_ref.text.substr(0, _ref.startColumn)), - [this](char ch) { m_stream << (ch == '\t' ? '\t' : ' '); } - ); - diagColored() << string(numCodepoints(_ref.text.substr(_ref.startColumn, locationLength)), '^'); + m_stream << ' ' << replaceNonTabs(text.substr(0, _ref.startColumn), ' '); + diagColored() << replaceNonTabs(text.substr(_ref.startColumn, locationLength), '^'); m_stream << '\n'; } else @@ -122,13 +133,13 @@ void SourceReferenceFormatterHuman::printSourceLocation(SourceReference const& _ // line 2: frameColored() << line << " |"; - m_stream << ' ' << _ref.text.substr(0, _ref.startColumn); - highlightColored() << _ref.text.substr(_ref.startColumn) << '\n'; + m_stream << ' ' << text.substr(0, _ref.startColumn); + highlightColored() << text.substr(_ref.startColumn) << '\n'; // line 3: m_stream << leftpad << ' '; frameColored() << '|'; - m_stream << ' ' << string(_ref.startColumn, ' '); + m_stream << ' ' << replaceNonTabs(text.substr(0, _ref.startColumn), ' '); diagColored() << "^ (Relevant source part starts here and spans across multiple lines)."; m_stream << '\n'; } diff --git a/libsolutil/UTF8.cpp b/libsolutil/UTF8.cpp index 82c87bb44..a7d55af6c 100644 --- a/libsolutil/UTF8.cpp +++ b/libsolutil/UTF8.cpp @@ -138,13 +138,4 @@ bool validateUTF8(std::string const& _input, size_t& _invalidPosition) return validateUTF8(reinterpret_cast(_input.c_str()), _input.length(), _invalidPosition); } -size_t numCodepoints(std::string const& _utf8EncodedInput) -{ - size_t codepoint = 0; - for (char c: _utf8EncodedInput) - codepoint += (c & 0xc0) != 0x80; - - return codepoint; -} - } diff --git a/libsolutil/UTF8.h b/libsolutil/UTF8.h index fb5ee376c..cd84c3982 100644 --- a/libsolutil/UTF8.h +++ b/libsolutil/UTF8.h @@ -38,6 +38,4 @@ inline bool validateUTF8(std::string const& _input) return validateUTF8(_input, invalidPos); } -size_t numCodepoints(std::string const& _utf8EncodedInput); - } diff --git a/test/cmdlineTests/message_format_utf16/err b/test/cmdlineTests/message_format_utf16/err deleted file mode 100644 index abe92d2e5..000000000 --- a/test/cmdlineTests/message_format_utf16/err +++ /dev/null @@ -1,11 +0,0 @@ -Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: " to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information. ---> message_format_utf16/input.sol - -Warning: Source file does not specify required compiler version! ---> message_format_utf16/input.sol - -Warning: Statement has no effect. - --> message_format_utf16/input.sol:2:58: - | -2 | /* ©©©©ﾩ©©©©© 2017 */ constructor () public { "©©©©ﾩ©©©©©" ; } - | ^^^^^^^^^^^^ diff --git a/test/cmdlineTests/message_format_utf16/input.sol b/test/cmdlineTests/message_format_utf16/input.sol deleted file mode 100644 index 2c1f9007a..000000000 --- a/test/cmdlineTests/message_format_utf16/input.sol +++ /dev/null @@ -1,3 +0,0 @@ -contract Foo { -/* ©©©©ﾩ©©©©© 2017 */ constructor () public { "©©©©ﾩ©©©©©" ; } -} diff --git a/test/cmdlineTests/message_format_utf8/err b/test/cmdlineTests/message_format_utf8/err new file mode 100644 index 000000000..fcd1717b8 --- /dev/null +++ b/test/cmdlineTests/message_format_utf8/err @@ -0,0 +1,35 @@ +Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: " to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information. +--> message_format_utf8/input.sol + +Warning: Source file does not specify required compiler version! +--> message_format_utf8/input.sol + +Warning: Statement has no effect. + --> message_format_utf8/input.sol:2:58: + | +2 | /* ©©©©ﾩ©©©©© 2017 */ constructor () public { "©©©©ﾩ©©©©©" ; } + | ^^^^^^^^^^^^ + +Warning: Statement has no effect. + --> message_format_utf8/input.sol:6:25: + | +6 | "S = π × r²"; + | ^^^^^^^^^^^^ + +Warning: Statement has no effect. + --> message_format_utf8/input.sol:7:39: + | +7 | /* ₀₁₂₃₄⁵⁶⁷⁸⁹ */ "∑ 1/n! ≈ 2.7"; // tabs in-between + | ^^^^^^^^^^^^^^ + +Warning: Statement has no effect. + --> message_format_utf8/input.sol:8:30: + | +8 | /* Ŀŏŗėɯ ïƥŝʉɱ */ "μὴ χεῖρον βέλτιστον"; // tabs in-between and inside + | ^^^ ^^^^^^ ^^^^^^^^^^ + +Warning: Function state mutability can be restricted to pure + --> message_format_utf8/input.sol:12:2: + | +12 | function selector() public returns(uint) { // starts with tab + | ^ (Relevant source part starts here and spans across multiple lines). diff --git a/test/cmdlineTests/message_format_utf8/input.sol b/test/cmdlineTests/message_format_utf8/input.sol new file mode 100644 index 000000000..51585efde --- /dev/null +++ b/test/cmdlineTests/message_format_utf8/input.sol @@ -0,0 +1,15 @@ +contract Foo { +/* ©©©©ﾩ©©©©© 2017 */ constructor () public { "©©©©ﾩ©©©©©" ; } + + function f() public pure { + + "S = π × r²"; +/* ₀₁₂₃₄⁵⁶⁷⁸⁹ */ "∑ 1/n! ≈ 2.7"; // tabs in-between +/* Ŀŏŗėɯ ïƥŝʉɱ */ "μὴ χεῖρον βέλτιστον"; // tabs in-between and inside + + } + + function selector() public returns(uint) { // starts with tab + return 0; + } +} From 3bf236cf8198fc09a0802b95167fccf3c90437d4 Mon Sep 17 00:00:00 2001 From: a3d4 Date: Tue, 2 Jun 2020 02:52:29 +0200 Subject: [PATCH 34/36] Refactor error ID checker script --- .circleci/config.yml | 2 +- liblangutil/Exceptions.h | 2 +- ...{correct_error_ids.py => fix_error_ids.py} | 39 +++++++++++-------- 3 files changed, 24 insertions(+), 19 deletions(-) rename scripts/{correct_error_ids.py => fix_error_ids.py} (89%) diff --git a/.circleci/config.yml b/.circleci/config.yml index 7f2b46a0c..624af2f0e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -323,7 +323,7 @@ jobs: - checkout - run: name: Check for error codes - command: ./scripts/correct_error_ids.py --noconfirm + command: ./scripts/fix_error_ids.py --check-only chk_pylint: docker: diff --git a/liblangutil/Exceptions.h b/liblangutil/Exceptions.h index 42b76a1fe..3fbf7711a 100644 --- a/liblangutil/Exceptions.h +++ b/liblangutil/Exceptions.h @@ -61,7 +61,7 @@ struct InvalidAstError: virtual util::Exception {}; * They are passed as the first parameter of error reporting functions. * Suffix _error helps to find them in the sources. * The struct ErrorId prevents incidental calls like typeError(3141) instead of typeError(3141_error). - * To create a new ID, one can add 0000_error and then run "python ./scripts/correct_error_ids.py" + * To create a new ID, one can add 0000_error and then run "python ./scripts/fix_error_ids.py" * from the root of the repo. */ struct ErrorId { unsigned long long error = 0; }; diff --git a/scripts/correct_error_ids.py b/scripts/fix_error_ids.py similarity index 89% rename from scripts/correct_error_ids.py rename to scripts/fix_error_ids.py index cbbad8264..d9a201115 100755 --- a/scripts/correct_error_ids.py +++ b/scripts/fix_error_ids.py @@ -119,28 +119,19 @@ def find_source_files(top_dir): def main(argv): + check_only = False noconfirm = False - opts, args = getopt.getopt(argv, "", ["noconfirm"]) + opts, args = getopt.getopt(argv, "", ["check-only", "noconfirm"]) for opt, arg in opts: - if opt == '--noconfirm': + if opt == '--check-only': + check_only = True + elif opt == '--noconfirm': noconfirm = True random.seed() cwd = os.getcwd() - if not noconfirm: - answer = input( - f"This script checks and corrects *_error literals in .h and .cpp files\n" - f"in {cwd}, recursively.\n\n" - f"Please commit current changes first, and review the results when the script finishes.\n\n" - f"Do you want to start [Y/N]? " - ) - while len(answer) == 0 or answer not in "YNyn": - answer = input("[Y/N]? ") - if answer not in "yY": - exit(0) - source_file_names = find_source_files(cwd) used_ids = get_used_ids(source_file_names) @@ -160,11 +151,25 @@ def main(argv): if ok: print("No incorrect IDs found") exit(0) - else: - fix_ids(used_ids, source_file_names) - print("Fixing completed") + + if check_only: exit(1) + if not noconfirm: + answer = input( + "\nDo you want to fix incorrect IDs?\n" + "Please commit current changes first, and review the results when the script finishes.\n" + "[Y/N]? " + ) + while len(answer) == 0 or answer not in "YNyn": + answer = input("[Y/N]? ") + if answer not in "yY": + exit(1) + + fix_ids(used_ids, source_file_names) + print("Fixing completed") + exit(2) + if __name__ == "__main__": main(sys.argv[1:]) From 4b6c322279296ef9b0772bd9e572f5d1eb860cba Mon Sep 17 00:00:00 2001 From: Djordje Mijovic Date: Wed, 3 Jun 2020 10:27:23 +0200 Subject: [PATCH 35/36] Fixing various signedness warnings --- solc/CommandLineInterface.cpp | 4 ++-- test/CommonSyntaxTest.cpp | 6 +++--- test/EVMHost.cpp | 2 +- test/ExecutionFramework.cpp | 10 ++++++---- test/Metadata.cpp | 4 ++-- test/tools/fuzzer_common.cpp | 2 +- tools/solidityUpgrade/UpgradeChange.cpp | 7 +++++-- 7 files changed, 20 insertions(+), 15 deletions(-) diff --git a/solc/CommandLineInterface.cpp b/solc/CommandLineInterface.cpp index a8e5cc5b7..1a661b948 100644 --- a/solc/CommandLineInterface.cpp +++ b/solc/CommandLineInterface.cpp @@ -629,8 +629,8 @@ bool CommandLineInterface::parseLibraryOption(string const& _input) serr() << "Colon separator missing in library address specifier \"" << lib << "\"" << endl; return false; } - string libName(lib.begin(), lib.begin() + colon); - string addrString(lib.begin() + colon + 1, lib.end()); + string libName(lib.begin(), lib.begin() + static_cast(colon)); + string addrString(lib.begin() + static_cast(colon) + 1, lib.end()); boost::trim(libName); boost::trim(addrString); if (addrString.substr(0, 2) == "0x") diff --git a/test/CommonSyntaxTest.cpp b/test/CommonSyntaxTest.cpp index e3edc3fb0..cafcc58f7 100644 --- a/test/CommonSyntaxTest.cpp +++ b/test/CommonSyntaxTest.cpp @@ -116,11 +116,11 @@ void CommonSyntaxTest::printSource(ostream& _stream, string const& _linePrefix, for (int i = error.locationStart; i < error.locationEnd; i++) if (isWarning) { - if (sourceFormatting[i] == formatting::RESET) - sourceFormatting[i] = formatting::ORANGE_BACKGROUND_256; + if (sourceFormatting[static_cast(i)] == formatting::RESET) + sourceFormatting[static_cast(i)] = formatting::ORANGE_BACKGROUND_256; } else - sourceFormatting[i] = formatting::RED_BACKGROUND; + sourceFormatting[static_cast(i)] = formatting::RED_BACKGROUND; } _stream << _linePrefix << sourceFormatting.front() << source.front(); diff --git a/test/EVMHost.cpp b/test/EVMHost.cpp index 836d61057..108dae4f1 100644 --- a/test/EVMHost.cpp +++ b/test/EVMHost.cpp @@ -223,7 +223,7 @@ evmc::result EVMHost::call(evmc_message const& _message) noexcept if (message.kind == EVMC_CREATE || message.kind == EVMC_CREATE2) { - result.gas_left -= evmasm::GasCosts::createDataGas * result.output_size; + result.gas_left -= static_cast(evmasm::GasCosts::createDataGas * result.output_size); if (result.gas_left < 0) { result.gas_left = 0; diff --git a/test/ExecutionFramework.cpp b/test/ExecutionFramework.cpp index ab11b2e98..ebe63b265 100644 --- a/test/ExecutionFramework.cpp +++ b/test/ExecutionFramework.cpp @@ -101,7 +101,9 @@ u256 ExecutionFramework::gasPrice() const u256 ExecutionFramework::blockHash(u256 const& _number) const { - return {EVMHost::convertFromEVMC(m_evmHost->get_block_hash(uint64_t(_number & numeric_limits::max())))}; + return {EVMHost::convertFromEVMC( + m_evmHost->get_block_hash(static_cast(_number & numeric_limits::max())) + )}; } u256 ExecutionFramework::blockNumber() const @@ -153,7 +155,7 @@ void ExecutionFramework::sendMessage(bytes const& _data, bool _isCreation, u256 if (m_showMessages) { cout << " out: " << toHex(m_output) << endl; - cout << " result: " << size_t(result.status_code) << endl; + cout << " result: " << static_cast(result.status_code) << endl; cout << " gas used: " << m_gasUsed.str() << endl; } } @@ -180,7 +182,7 @@ void ExecutionFramework::sendEther(Address const& _addr, u256 const& _amount) size_t ExecutionFramework::currentTimestamp() { - return m_evmHost->tx_context.block_timestamp; + return static_cast(m_evmHost->tx_context.block_timestamp); } size_t ExecutionFramework::blockTimestamp(u256 _block) @@ -188,7 +190,7 @@ size_t ExecutionFramework::blockTimestamp(u256 _block) if (_block > blockNumber()) return 0; else - return size_t((currentTimestamp() / blockNumber()) * _block); + return static_cast((currentTimestamp() / blockNumber()) * _block); } Address ExecutionFramework::account(size_t _idx) diff --git a/test/Metadata.cpp b/test/Metadata.cpp index cf61c4c99..cb5892b8a 100644 --- a/test/Metadata.cpp +++ b/test/Metadata.cpp @@ -36,14 +36,14 @@ bytes onlyMetadata(bytes const& _bytecode) unsigned size = _bytecode.size(); if (size < 5) return bytes{}; - size_t metadataSize = (_bytecode[size - 2] << 8) + _bytecode[size - 1]; + size_t metadataSize = (static_cast(_bytecode[size - 2]) << 8ul) + static_cast(_bytecode[size - 1]); if (size < (metadataSize + 2)) return bytes{}; // Sanity check: assume the first byte is a fixed-size CBOR array with 1, 2 or 3 entries unsigned char firstByte = _bytecode[size - metadataSize - 2]; if (firstByte != 0xa1 && firstByte != 0xa2 && firstByte != 0xa3) return bytes{}; - return bytes(_bytecode.end() - metadataSize - 2, _bytecode.end() - 2); + return bytes(_bytecode.end() - static_cast(metadataSize) - 2, _bytecode.end() - 2); } bytes bytecodeSansMetadata(bytes const& _bytecode) diff --git a/test/tools/fuzzer_common.cpp b/test/tools/fuzzer_common.cpp index e4de0f185..fa167c437 100644 --- a/test/tools/fuzzer_common.cpp +++ b/test/tools/fuzzer_common.cpp @@ -156,7 +156,7 @@ void FuzzerUtil::testConstantOptimizer(string const& _input, bool _quiet) assembly.append(n); } for (bool isCreation: {false, true}) - for (unsigned runs: {1, 2, 3, 20, 40, 100, 200, 400, 1000}) + for (unsigned runs: {1u, 2u, 3u, 20u, 40u, 100u, 200u, 400u, 1000u}) { // Make a copy here so that each time we start with the original state. Assembly tmp = assembly; diff --git a/tools/solidityUpgrade/UpgradeChange.cpp b/tools/solidityUpgrade/UpgradeChange.cpp index 8bbd6214c..ff7d6d68f 100644 --- a/tools/solidityUpgrade/UpgradeChange.cpp +++ b/tools/solidityUpgrade/UpgradeChange.cpp @@ -27,7 +27,10 @@ using namespace solidity::tools; void UpgradeChange::apply() { - m_source.replace(m_location.start, m_location.end - m_location.start, m_patch); + m_source.replace( + static_cast(m_location.start), + static_cast(m_location.end - m_location.start), m_patch + ); } void UpgradeChange::log(bool const _shorten) const @@ -56,7 +59,7 @@ void UpgradeChange::log(bool const _shorten) const string line; while (getline(output, line)) { - os << string(leftpad, ' '); + os << string(static_cast(leftpad), ' '); AnsiColorized(os, true, {formatting::BOLD, formatting::BLUE}) << "| "; AnsiColorized(os, true, {}) << line << endl; } From c708a1bec2d5814f8340c24e25bd5698c345b48c Mon Sep 17 00:00:00 2001 From: Christian Parpart Date: Wed, 3 Jun 2020 11:28:01 +0200 Subject: [PATCH 36/36] AsmAnalysis: Fixes a superfluous whitespace in an error string --- libyul/AsmAnalysis.cpp | 2 +- .../syntaxTests/inlineAssembly/istanbul_on_petersburg.sol | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libyul/AsmAnalysis.cpp b/libyul/AsmAnalysis.cpp index 7a8037760..256d2ac38 100644 --- a/libyul/AsmAnalysis.cpp +++ b/libyul/AsmAnalysis.cpp @@ -555,7 +555,7 @@ bool AsmAnalyzer::warnOnInstructions(evmasm::Instruction _instr, SourceLocation + "\" instruction is " + vmKindMessage + " VMs " + - " (you are currently compiling for \"" + + "(you are currently compiling for \"" + m_evmVersion.name() + "\")." ); diff --git a/test/libsolidity/syntaxTests/inlineAssembly/istanbul_on_petersburg.sol b/test/libsolidity/syntaxTests/inlineAssembly/istanbul_on_petersburg.sol index 249a79cc0..a29a2ab35 100644 --- a/test/libsolidity/syntaxTests/inlineAssembly/istanbul_on_petersburg.sol +++ b/test/libsolidity/syntaxTests/inlineAssembly/istanbul_on_petersburg.sol @@ -13,7 +13,7 @@ contract C { // ==== // EVMVersion: =petersburg // ---- -// TypeError: (101-108): The "chainid" instruction is only available for Istanbul-compatible VMs (you are currently compiling for "petersburg"). +// TypeError: (101-108): The "chainid" instruction is only available for Istanbul-compatible VMs (you are currently compiling for "petersburg"). // DeclarationError: (95-110): Variable count does not match number of values (1 vs. 0) -// TypeError: (215-226): The "selfbalance" instruction is only available for Istanbul-compatible VMs (you are currently compiling for "petersburg"). +// TypeError: (215-226): The "selfbalance" instruction is only available for Istanbul-compatible VMs (you are currently compiling for "petersburg"). // DeclarationError: (209-228): Variable count does not match number of values (1 vs. 0)