Merge pull request #14328 from ethereum/fix-incomplete-ast-in-standar…

…d-json-on-analysis-fail

Fix incomplete AST in standard json on analysis fail

Changelog.md

@@ -20,6 +20,7 @@ Bugfixes:
  * Commandline Interface: It is no longer possible to specify both ``--optimize-yul`` and ``--no-optimize-yul`` at the same time.
  * SMTChecker: Fix encoding of side-effects inside ``if`` and ``ternary conditional``statements in the BMC engine.
  * SMTChecker: Fix false negative when a verification target can be violated only by trusted external call from another public function.
+ * Standard JSON Interface: Fix an incomplete AST being returned when analysis is interrupted by certain kinds of fatal errors.
  * Yul Optimizer: Ensure that the assignment of memory slots for variables moved to memory does not depend on AST IDs that may depend on whether additional files are included during compilation.
  * Yul Optimizer: Fix optimized IR being unnecessarily passed through the Yul optimizer again before bytecode generation.
 

libsolidity/interface/StandardCompiler.cpp

@@ -1317,15 +1317,21 @@ Json::Value StandardCompiler::compileSolidity(StandardCompiler::InputsAndSetting
  ));
  }
 
+ bool parsingSuccess = compilerStack.state() >= CompilerStack::State::Parsed;
  bool analysisPerformed = compilerStack.state() >= CompilerStack::State::AnalysisPerformed;
- bool const compilationSuccess = compilerStack.state() == CompilerStack::State::CompilationSuccessful;
+ bool compilationSuccess = compilerStack.state() == CompilerStack::State::CompilationSuccessful;
 
  if (compilerStack.hasError() && !_inputsAndSettings.parserErrorRecovery)
  analysisPerformed = false;
 
+ // If analysis fails, the artifacts inside CompilerStack are potentially incomplete and must not be returned.
+ // Note that not completing analysis due to stopAfter does not count as a failure. It's neither failure nor success.
+ bool analysisFailed = !analysisPerformed && _inputsAndSettings.stopAfter >= CompilerStack::State::AnalysisPerformed;
+ bool compilationFailed = !compilationSuccess && binariesRequested;
+
  /// Inconsistent state - stop here to receive error reports from users
  if (
- ((binariesRequested && !compilationSuccess) || !analysisPerformed) &&
+ (compilationFailed || !analysisPerformed) &&
  (errors.empty() && _inputsAndSettings.stopAfter >= CompilerStack::State::AnalysisPerformed)
  )
  return formatFatalError(Error::Type::InternalCompilerError, "No error reported, but compilation failed.");
@@ -1343,7 +1349,7 @@ Json::Value StandardCompiler::compileSolidity(StandardCompiler::InputsAndSetting
 
  output["sources"] = Json::objectValue;
  unsigned sourceIndex = 0;
- if (compilerStack.state() >= CompilerStack::State::Parsed && (!compilerStack.hasError() || _inputsAndSettings.parserErrorRecovery))
+ if (parsingSuccess && !analysisFailed && (!compilerStack.hasError() || _inputsAndSettings.parserErrorRecovery))
  for (string const& sourceName: compilerStack.sourceNames())
  {
  Json::Value sourceResult = Json::objectValue;

test/cmdlineTests/standard_empty_file_name/output.json

@@ -18,11 +18,5 @@
  "type": "DeclarationError"
  }
  ],
- "sources":
- {
- "":
- {
- "id": 0
- }
- }
+ "sources": {}
 }

test/cmdlineTests/standard_outputs_on_analysis_error_fatal/args

@@ -0,0 +1 @@
+--allow-paths .

test/cmdlineTests/standard_outputs_on_analysis_error_fatal/in.sol

@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-3.0
+pragma solidity *;
+
+contract C {
+ // This will trigger a fatal error at the analysis stage, of the kind that terminates analysis
+ // immediately without letting the current step finish.
+ constructor(uint[] storage) {}
+}

test/cmdlineTests/standard_outputs_on_analysis_error_fatal/input.json

@@ -0,0 +1,14 @@
+{
+ "language": "Solidity",
+ "sources": {
+ "C": {"urls": ["standard_outputs_on_analysis_error_fatal/in.sol"]}
+ },
+ "settings": {
+ "outputSelection": {
+ "*": {
+ "*": ["*"],
+ "": ["*"]
+ }
+ }
+ }
+}

test/cmdlineTests/standard_outputs_on_analysis_error_fatal/output.json

@@ -0,0 +1,26 @@
+{
+ "errors":
+ [
+ {
+ "component": "general",
+ "errorCode": "3644",
+ "formattedMessage": "TypeError: This parameter has a type that can only be used internally. You can make the contract abstract to avoid this problem.
+ --> C:7:17:
+ |
+7 | constructor(uint[] storage) {}
+ | ^^^^^^^^^^^^^^
+
+",
+ "message": "This parameter has a type that can only be used internally. You can make the contract abstract to avoid this problem.",
+ "severity": "error",
+ "sourceLocation":
+ {
+ "end": 258,
+ "file": "C",
+ "start": 244
+ },
+ "type": "TypeError"
+ }
+ ],
+ "sources": {}
+}

test/cmdlineTests/standard_outputs_on_analysis_error_fatal_after_current_step/args

@@ -0,0 +1 @@
+--allow-paths .

test/cmdlineTests/standard_outputs_on_analysis_error_fatal_after_current_step/in.sol

@@ -0,0 +1,6 @@
+// SPDX-License-Identifier: GPL-3.0
+pragma solidity *;
+
+// This will trigger a fatal error at the analysis stage, of the kind that lets the current
+// analysis steps finish but terminates analysis after immediately after that step.
+function f(uint immutable x) {}

test/cmdlineTests/standard_outputs_on_analysis_error_fatal_after_current_step/input.json

@@ -0,0 +1,14 @@
+{
+ "language": "Solidity",
+ "sources": {
+ "C": {"urls": ["standard_outputs_on_analysis_error_fatal_after_current_step/in.sol"]}
+ },
+ "settings": {
+ "outputSelection": {
+ "*": {
+ "*": ["*"],
+ "": ["*"]
+ }
+ }
+ }
+}

test/cmdlineTests/standard_outputs_on_analysis_error_fatal_after_current_step/output.json

@@ -0,0 +1,26 @@
+{
+ "errors":
+ [
+ {
+ "component": "general",
+ "errorCode": "8297",
+ "formattedMessage": "DeclarationError: The \"immutable\" keyword can only be used for state variables.
+ --> C:6:12:
+ |
+6 | function f(uint immutable x) {}
+ | ^^^^^^^^^^^^^^^^
+
+",
+ "message": "The \"immutable\" keyword can only be used for state variables.",
+ "severity": "error",
+ "sourceLocation":
+ {
+ "end": 259,
+ "file": "C",
+ "start": 243
+ },
+ "type": "DeclarationError"
+ }
+ ],
+ "sources": {}
+}

test/cmdlineTests/standard_outputs_on_analysis_error_non_fatal/args

@@ -0,0 +1 @@
+--allow-paths .

test/cmdlineTests/standard_outputs_on_analysis_error_non_fatal/in.sol

@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: GPL-3.0
+pragma solidity *;
+
+contract C {
+ // This will trigger a non-fatal error at the analysis stage.
+ // With this kind of error we still run subsequent analysis stages.
+ uint x;
+ string y = x;
+}

test/cmdlineTests/standard_outputs_on_analysis_error_non_fatal/input.json

@@ -0,0 +1,14 @@
+{
+ "language": "Solidity",
+ "sources": {
+ "C": {"urls": ["standard_outputs_on_analysis_error_non_fatal/in.sol"]}
+ },
+ "settings": {
+ "outputSelection": {
+ "*": {
+ "*": ["*"],
+ "": ["*"]
+ }
+ }
+ }
+}

test/cmdlineTests/standard_outputs_on_analysis_error_non_fatal/output.json

@@ -0,0 +1,26 @@
+{
+ "errors":
+ [
+ {
+ "component": "general",
+ "errorCode": "7407",
+ "formattedMessage": "TypeError: Type uint256 is not implicitly convertible to expected type string storage ref.
+ --> C:8:16:
+ |
+8 | string y = x;
+ | ^
+
+",
+ "message": "Type uint256 is not implicitly convertible to expected type string storage ref.",
+ "severity": "error",
+ "sourceLocation":
+ {
+ "end": 235,
+ "file": "C",
+ "start": 234
+ },
+ "type": "TypeError"
+ }
+ ],
+ "sources": {}
+}

test/cmdlineTests/standard_outputs_on_compilation_error/args

@@ -0,0 +1 @@
+--allow-paths .

test/cmdlineTests/standard_outputs_on_compilation_error/in.sol

@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-3.0
+pragma solidity *;
+
+contract C {
+ // This will trigger an error at the compilation stage.
+ // CodeGenerationError due to immutable initialization in constructor being optimized out.
+ uint immutable public x;
+
+ constructor() {
+ x = 0;
+ while (true) {}
+ }
+}

test/cmdlineTests/standard_outputs_on_compilation_error/input.json

@@ -0,0 +1,15 @@
+{
+ "language": "Solidity",
+ "sources": {
+ "C": {"urls": ["standard_outputs_on_compilation_error/in.sol"]}
+ },
+ "settings": {
+ "optimizer": {"enabled": true},
+ "outputSelection": {
+ "*": {
+ "*": ["*"],
+ "": ["*"]
+ }
+ }
+ }
+}