From 53f9a114408c9f73594a942f7b1b8289fa4f1376 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Mon, 21 Dec 2020 21:16:35 +0100 Subject: [PATCH] prepare_report: Handle internal compiler errors in Standard JSON as errors, not missing bytecode --- scripts/bytecodecompare/prepare_report.js | 17 ++++++- scripts/bytecodecompare/prepare_report.py | 11 +++- .../code_generation_error_cli_output.txt | 8 +++ .../code_generation_error_json_output.json | 50 +++++++++++++++++++ .../fixtures/stack_too_deep_cli_output.txt | 2 + .../fixtures/stack_too_deep_json_output.json | 23 +++++++++ .../unimplemented_feature_cli_output.txt | 5 ++ .../unimplemented_feature_json_output.json | 23 +++++++++ .../test_bytecodecompare_prepare_report.py | 39 +++++++++++++++ 9 files changed, 176 insertions(+), 2 deletions(-) create mode 100644 test/scripts/fixtures/code_generation_error_cli_output.txt create mode 100644 test/scripts/fixtures/code_generation_error_json_output.json create mode 100644 test/scripts/fixtures/stack_too_deep_cli_output.txt create mode 100644 test/scripts/fixtures/stack_too_deep_json_output.json create mode 100644 test/scripts/fixtures/unimplemented_feature_cli_output.txt create mode 100644 test/scripts/fixtures/unimplemented_feature_json_output.json diff --git a/scripts/bytecodecompare/prepare_report.js b/scripts/bytecodecompare/prepare_report.js index 1d1b04060..83cfc13e0 100755 --- a/scripts/bytecodecompare/prepare_report.js +++ b/scripts/bytecodecompare/prepare_report.js @@ -24,10 +24,25 @@ for (const optimize of [false, true]) const result = JSON.parse(compiler.compile(JSON.stringify(input))) + let internalCompilerError = false + if ('errors' in result) + { + for (const error of result['errors']) + // JSON interface still returns contract metadata in case of an internal compiler error while + // CLI interface does not. To make reports comparable we must force this case to be detected as + // an error in both cases. + if (['UnimplementedFeatureError', 'CompilerError', 'CodeGenerationError'].includes(error['type'])) + { + internalCompilerError = true + break + } + } + if ( !('contracts' in result) || Object.keys(result['contracts']).length === 0 || - Object.keys(result['contracts']).every(file => Object.keys(result['contracts'][file]).length === 0) + Object.keys(result['contracts']).every(file => Object.keys(result['contracts'][file]).length === 0) || + internalCompilerError ) // NOTE: do not exit here because this may be run on source which cannot be compiled console.log(filename + ': ') diff --git a/scripts/bytecodecompare/prepare_report.py b/scripts/bytecodecompare/prepare_report.py index 64fee0704..054aa18b5 100755 --- a/scripts/bytecodecompare/prepare_report.py +++ b/scripts/bytecodecompare/prepare_report.py @@ -66,10 +66,19 @@ def load_source(path: Union[Path, str]) -> str: def parse_standard_json_output(source_file_name: Path, standard_json_output: str) -> FileReport: decoded_json_output = json.loads(standard_json_output.strip()) + # JSON interface still returns contract metadata in case of an internal compiler error while + # CLI interface does not. To make reports comparable we must force this case to be detected as + # an error in both cases. + internal_compiler_error = any( + error['type'] in ['UnimplementedFeatureError', 'CompilerError', 'CodeGenerationError'] + for error in decoded_json_output.get('errors', {}) + ) + if ( 'contracts' not in decoded_json_output or len(decoded_json_output['contracts']) == 0 or - all(len(file_results) == 0 for file_name, file_results in decoded_json_output['contracts'].items()) + all(len(file_results) == 0 for file_name, file_results in decoded_json_output['contracts'].items()) or + internal_compiler_error ): return FileReport(file_name=source_file_name, contract_reports=None) diff --git a/test/scripts/fixtures/code_generation_error_cli_output.txt b/test/scripts/fixtures/code_generation_error_cli_output.txt new file mode 100644 index 000000000..2badab1da --- /dev/null +++ b/test/scripts/fixtures/code_generation_error_cli_output.txt @@ -0,0 +1,8 @@ +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. +--> test_1c3426238b8296745d8d8bd0ff995ab65a51992b568dc7c5ce73c3f59b107825_no_assignments_sol.sol + +Warning: Source file does not specify required compiler version! Consider adding "pragma solidity ^0.8.0;" +--> test_1c3426238b8296745d8d8bd0ff995ab65a51992b568dc7c5ce73c3f59b107825_no_assignments_sol.sol + +Error: Some immutables were read from but never assigned, possibly because of optimization. + diff --git a/test/scripts/fixtures/code_generation_error_json_output.json b/test/scripts/fixtures/code_generation_error_json_output.json new file mode 100644 index 000000000..949bf33ea --- /dev/null +++ b/test/scripts/fixtures/code_generation_error_json_output.json @@ -0,0 +1,50 @@ +{ + "contracts": { + "syntaxTests/immutable/no_assignments.sol": { + "C": { + "metadata": "{\"compiler\":{\"version\":\"0.8.0+commit.c7dfd78e\"},\"language\":\"Solidity\",\"output\":{\"abi\":[{\"inputs\":[],\"stateMutability\":\"nonpayable\",\"type\":\"constructor\"},{\"inputs\":[],\"name\":\"f\",\"outputs\":[{\"internalType\":\"uint256\",\"name\":\"\",\"type\":\"uint256\"}],\"stateMutability\":\"view\",\"type\":\"function\"}],\"devdoc\":{\"kind\":\"dev\",\"methods\":{},\"version\":1},\"userdoc\":{\"kind\":\"user\",\"methods\":{},\"version\":1}},\"settings\":{\"compilationTarget\":{\"syntaxTests/immutable/no_assignments.sol\":\"C\"},\"evmVersion\":\"istanbul\",\"libraries\":{},\"metadata\":{\"bytecodeHash\":\"ipfs\"},\"optimizer\":{\"enabled\":true,\"runs\":200},\"remappings\":[]},\"sources\":{\"syntaxTests/immutable/no_assignments.sol\":{\"keccak256\":\"0xbafaec265150d52cd293787144247b1a0a782adf3cb89296fb4f0eb05dc25739\",\"urls\":[\"bzz-raw://7c01dbb8146347c8cf62469d57a0e290f7ef1b871426d86d995315160db665c0\",\"dweb:/ipfs/QmPrYtxVbFCFeXwnhcHoBgbg546EqMzQCT5kK7wLc3rat8\"]}},\"version\":1}" + } + } + }, + "errors": [ + { + "component": "general", + "errorCode": "1878", + "formattedMessage": "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.\n--> syntaxTests/immutable/no_assignments.sol\n\n", + "message": "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.", + "severity": "warning", + "sourceLocation": { + "end": -1, + "file": "syntaxTests/immutable/no_assignments.sol", + "start": -1 + }, + "type": "Warning" + }, + { + "component": "general", + "errorCode": "3420", + "formattedMessage": "Warning: Source file does not specify required compiler version! Consider adding \"pragma solidity ^0.8.0;\"\n--> syntaxTests/immutable/no_assignments.sol\n\n", + "message": "Source file does not specify required compiler version! Consider adding \"pragma solidity ^0.8.0;\"", + "severity": "warning", + "sourceLocation": { + "end": -1, + "file": "syntaxTests/immutable/no_assignments.sol", + "start": -1 + }, + "type": "Warning" + }, + { + "component": "general", + "errorCode": "1284", + "formattedMessage": "CodeGenerationError: Some immutables were read from but never assigned, possibly because of optimization.\n\n", + "message": "Some immutables were read from but never assigned, possibly because of optimization.", + "severity": "error", + "type": "CodeGenerationError" + } + ], + "sources": { + "syntaxTests/immutable/no_assignments.sol": { + "id": 0 + } + } +} diff --git a/test/scripts/fixtures/stack_too_deep_cli_output.txt b/test/scripts/fixtures/stack_too_deep_cli_output.txt new file mode 100644 index 000000000..d0ff06d47 --- /dev/null +++ b/test/scripts/fixtures/stack_too_deep_cli_output.txt @@ -0,0 +1,2 @@ +Compiler error: Stack too deep when compiling inline assembly: Variable value0 is 1 slot(s) too deep inside the stack. + diff --git a/test/scripts/fixtures/stack_too_deep_json_output.json b/test/scripts/fixtures/stack_too_deep_json_output.json new file mode 100644 index 000000000..4a259aa35 --- /dev/null +++ b/test/scripts/fixtures/stack_too_deep_json_output.json @@ -0,0 +1,23 @@ +{ + "contracts": { + "syntaxTests/tupleAssignments/large_component_count.sol": { + "C": { + "metadata": "{\"compiler\":{\"version\":\"0.8.0+commit.c7dfd78e\"},\"language\":\"Solidity\",\"output\":{\"abi\":[{\"inputs\":[],\"name\":\"f\",\"outputs\":[{\"internalType\":\"uint256\",\"name\":\"\",\"type\":\"uint256\"},{\"internalType\":\"uint256\",\"name\":\"\",\"type\":\"uint256\"},{\"internalType\":\"bytes32\",\"name\":\"\",\"type\":\"bytes32\"}],\"stateMutability\":\"pure\",\"type\":\"function\"},{\"inputs\":[],\"name\":\"g\",\"outputs\":[{\"internalType\":\"uint256\",\"name\":\"\",\"type\":\"uint256\"},{\"internalType\":\"uint256\",\"name\":\"\",\"type\":\"uint256\"},{\"internalType\":\"uint256\",\"name\":\"\",\"type\":\"uint256\"},{\"internalType\":\"uint256\",\"name\":\"\",\"type\":\"uint256\"},{\"internalType\":\"uint256\",\"name\":\"\",\"type\":\"uint256\"},{\"internalType\":\"uint256\",\"name\":\"\",\"type\":\"uint256\"},{\"internalType\":\"uint256\",\"name\":\"\",\"type\":\"uint256\"},{\"internalType\":\"uint256\",\"name\":\"\",\"type\":\"uint256\"},{\"internalType\":\"uint256\",\"name\":\"\",\"type\":\"uint256\"},{\"internalType\":\"uint256\",\"name\":\"\",\"type\":\"uint256\"},{\"internalType\":\"uint256\",\"name\":\"\",\"type\":\"uint256\"},{\"internalType\":\"uint256\",\"name\":\"\",\"type\":\"uint256\"},{\"internalType\":\"uint256\",\"name\":\"\",\"type\":\"uint256\"}],\"stateMutability\":\"pure\",\"type\":\"function\"}],\"devdoc\":{\"kind\":\"dev\",\"methods\":{},\"version\":1},\"userdoc\":{\"kind\":\"user\",\"methods\":{},\"version\":1}},\"settings\":{\"compilationTarget\":{\"syntaxTests/tupleAssignments/large_component_count.sol\":\"C\"},\"evmVersion\":\"istanbul\",\"libraries\":{},\"metadata\":{\"bytecodeHash\":\"ipfs\"},\"optimizer\":{\"enabled\":false,\"runs\":200},\"remappings\":[]},\"sources\":{\"syntaxTests/tupleAssignments/large_component_count.sol\":{\"keccak256\":\"0xb5478857c30ab2e7cf6b0fdcad0fdc70f4715129a7dd3f3f1dda5b3892a83846\",\"urls\":[\"bzz-raw://abc14e4a2a61618b712c4ac3ba0cab07d9214d0637af78aa7648e3d2f89eb725\",\"dweb:/ipfs/QmTqrQzwWbZgdAck6ma9xY3sL1mGiw61Nsg36P21mZdxLG\"]}},\"version\":1}" + } + } + }, + "errors": [ + { + "component": "general", + "formattedMessage": "CompilerError: Stack too deep when compiling inline assembly: Variable value0 is 1 slot(s) too deep inside the stack.\n\n", + "message": "Compiler error (/solidity/libyul/backends/evm/AsmCodeGen.cpp:248):Stack too deep when compiling inline assembly: Variable value0 is 1 slot(s) too deep inside the stack.", + "severity": "error", + "type": "CompilerError" + } + ], + "sources": { + "syntaxTests/tupleAssignments/large_component_count.sol": { + "id": 0 + } + } +} diff --git a/test/scripts/fixtures/unimplemented_feature_cli_output.txt b/test/scripts/fixtures/unimplemented_feature_cli_output.txt new file mode 100644 index 000000000..2278b832d --- /dev/null +++ b/test/scripts/fixtures/unimplemented_feature_cli_output.txt @@ -0,0 +1,5 @@ +Unimplemented feature: +/solidity/libsolidity/codegen/CompilerUtils.cpp(771): Throw in function void solidity::frontend::CompilerUtils::convertType(const solidity::frontend::Type&, const solidity::frontend::Type&, bool, bool, bool) +Dynamic exception type: boost::wrapexcept +std::exception::what: Not yet implemented - FixedPointType. +[solidity::util::tag_comment*] = Not yet implemented - FixedPointType. diff --git a/test/scripts/fixtures/unimplemented_feature_json_output.json b/test/scripts/fixtures/unimplemented_feature_json_output.json new file mode 100644 index 000000000..0a1e9db27 --- /dev/null +++ b/test/scripts/fixtures/unimplemented_feature_json_output.json @@ -0,0 +1,23 @@ +{ + "contracts": { + "syntaxTests/nameAndTypeResolution/317_fixed_type_valid_explicit_conversions.sol": { + "test": { + "metadata": "{\"compiler\":{\"version\":\"0.8.0+commit.c7dfd78e\"},\"language\":\"Solidity\",\"output\":{\"abi\":[{\"inputs\":[],\"name\":\"f\",\"outputs\":[],\"stateMutability\":\"nonpayable\",\"type\":\"function\"}],\"devdoc\":{\"kind\":\"dev\",\"methods\":{},\"version\":1},\"userdoc\":{\"kind\":\"user\",\"methods\":{},\"version\":1}},\"settings\":{\"compilationTarget\":{\"syntaxTests/nameAndTypeResolution/317_fixed_type_valid_explicit_conversions.sol\":\"test\"},\"evmVersion\":\"istanbul\",\"libraries\":{},\"metadata\":{\"bytecodeHash\":\"ipfs\"},\"optimizer\":{\"enabled\":false,\"runs\":200},\"remappings\":[]},\"sources\":{\"syntaxTests/nameAndTypeResolution/317_fixed_type_valid_explicit_conversions.sol\":{\"keccak256\":\"0x44b85b2db00441b574d40d11a6be684517d4312de0f6ef0550e02aea33e2a05f\",\"urls\":[\"bzz-raw://c7a9416a8634c5bd1451742f09d633acab37a6e0db1441d0bb7ea4e8f4af214b\",\"dweb:/ipfs/QmZNMQcZu9wnD6sFH2c7PymZReYeroBXDyKkJ5YEhWtCjc\"]}},\"version\":1}" + } + } + }, + "errors": [ + { + "component": "general", + "formattedMessage": "UnimplementedFeatureError: Not yet implemented - FixedPointType.\n\n", + "message": "Unimplemented feature (/solidity/libsolidity/codegen/CompilerUtils.cpp:771):Not yet implemented - FixedPointType.", + "severity": "error", + "type": "UnimplementedFeatureError" + } + ], + "sources": { + "syntaxTests/nameAndTypeResolution/317_fixed_type_valid_explicit_conversions.sol": { + "id": 0 + } + } +} diff --git a/test/scripts/test_bytecodecompare_prepare_report.py b/test/scripts/test_bytecodecompare_prepare_report.py index 1c22dd75f..885548e9d 100644 --- a/test/scripts/test_bytecodecompare_prepare_report.py +++ b/test/scripts/test_bytecodecompare_prepare_report.py @@ -32,6 +32,15 @@ LIBRARY_INHERITED2_SOL_CLI_OUTPUT = load_fixture('library_inherited2_sol_cli_out UNKNOWN_PRAGMA_SOL_JSON_OUTPUT = load_fixture('unknown_pragma_sol_json_output.json') UNKNOWN_PRAGMA_SOL_CLI_OUTPUT = load_fixture('unknown_pragma_sol_cli_output.txt') +UNIMPLEMENTED_FEATURE_JSON_OUTPUT = load_fixture('unimplemented_feature_json_output.json') +UNIMPLEMENTED_FEATURE_CLI_OUTPUT = load_fixture('unimplemented_feature_cli_output.txt') + +STACK_TOO_DEEP_JSON_OUTPUT = load_fixture('stack_too_deep_json_output.json') +STACK_TOO_DEEP_CLI_OUTPUT = load_fixture('stack_too_deep_cli_output.txt') + +CODE_GENERATION_ERROR_JSON_OUTPUT = load_fixture('code_generation_error_json_output.json') +CODE_GENERATION_ERROR_CLI_OUTPUT = load_fixture('code_generation_error_cli_output.txt') + class PrepareReportTestBase(unittest.TestCase): def setUp(self): @@ -294,6 +303,21 @@ class TestParseStandardJSONOutput(PrepareReportTestBase): self.assertEqual(parse_standard_json_output(Path('contract.sol'), compiler_output), expected_report) + def test_parse_standard_json_output_should_report_error_on_unimplemented_feature_error(self): + expected_report = FileReport(file_name=Path('file.sol'), contract_reports=None) + + self.assertEqual(parse_standard_json_output(Path('file.sol'), UNIMPLEMENTED_FEATURE_JSON_OUTPUT), expected_report) + + def test_parse_standard_json_output_should_report_error_on_stack_too_deep_error(self): + expected_report = FileReport(file_name=Path('file.sol'), contract_reports=None) + + self.assertEqual(parse_standard_json_output(Path('file.sol'), STACK_TOO_DEEP_JSON_OUTPUT), expected_report) + + def test_parse_standard_json_output_should_report_error_on_code_generation_error(self): + expected_report = FileReport(file_name=Path('file.sol'), contract_reports=None) + + self.assertEqual(parse_standard_json_output(Path('file.sol'), CODE_GENERATION_ERROR_JSON_OUTPUT), expected_report) + class TestParseCLIOutput(PrepareReportTestBase): def test_parse_cli_output(self): @@ -378,3 +402,18 @@ class TestParseCLIOutput(PrepareReportTestBase): ) self.assertEqual(parse_cli_output(Path('syntaxTests/scoping/library_inherited2.sol'), compiler_output), expected_report) + + def test_parse_cli_output_should_report_error_on_unimplemented_feature_error(self): + expected_report = FileReport(file_name=Path('file.sol'), contract_reports=None) + + self.assertEqual(parse_cli_output(Path('file.sol'), UNIMPLEMENTED_FEATURE_CLI_OUTPUT), expected_report) + + def test_parse_cli_output_should_report_error_on_stack_too_deep_error(self): + expected_report = FileReport(file_name=Path('file.sol'), contract_reports=None) + + self.assertEqual(parse_cli_output(Path('file.sol'), STACK_TOO_DEEP_CLI_OUTPUT), expected_report) + + def test_parse_cli_output_should_report_error_on_code_generation_error(self): + expected_report = FileReport(file_name=Path('file.sol'), contract_reports=None) + + self.assertEqual(parse_cli_output(Path('file.sol'), CODE_GENERATION_ERROR_CLI_OUTPUT), expected_report)