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

…-standard-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": {
+			"*": {
+				"*": ["*"],
+				"": ["*"]
+			}
+		}
+	}
+}