From ddad3aacd7971ccb2b9dc92666b130ada1bbae3e Mon Sep 17 00:00:00 2001
From: jjcnn <38888011+jjcnn@users.noreply.github.com>
Date: Tue, 20 Aug 2024 08:54:26 +0200
Subject: [PATCH] No limit on the depth of the return path analysis (#6435)

## Description

Fixes #6369.

The return path analysis performs a breadth-first traversal of the
control flow graph to determine whether every path returns a value.

So far the traversal has been limited to 50 iterations of the outer
while loop, meaning that if the graph has a depth of more than 50 there
are paths that would not be checked. The limit was introduced to ensure
that that the analysis terminated.

This PR removes this limit. Termination is ensured by keeping track of
which nodes have already been visited so that they are not visited
again.


## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.

---------

Co-authored-by: Sophie Dankel <47993817+sdankel@users.noreply.github.com>
---
 .../analyze_return_paths.rs                   |   7 +-
 .../return_path_analysis/Forc.lock            |  16 +
 .../return_path_analysis/Forc.toml            |   9 +
 .../return_path_analysis/json_abi_oracle.json |   1 +
 .../return_path_analysis/src/main.sw          | 525 ++++++++++++++++++
 .../return_path_analysis/test.toml            |   6 +
 6 files changed, 561 insertions(+), 3 deletions(-)
 create mode 100644 test/src/e2e_vm_tests/test_programs/should_fail/return_path_analysis/Forc.lock
 create mode 100644 test/src/e2e_vm_tests/test_programs/should_fail/return_path_analysis/Forc.toml
 create mode 100644 test/src/e2e_vm_tests/test_programs/should_fail/return_path_analysis/json_abi_oracle.json
 create mode 100644 test/src/e2e_vm_tests/test_programs/should_fail/return_path_analysis/src/main.sw
 create mode 100644 test/src/e2e_vm_tests/test_programs/should_fail/return_path_analysis/test.toml

diff --git a/sway-core/src/control_flow_analysis/analyze_return_paths.rs b/sway-core/src/control_flow_analysis/analyze_return_paths.rs
index ea1d932edc4..863e5ccfc69 100644
--- a/sway-core/src/control_flow_analysis/analyze_return_paths.rs
+++ b/sway-core/src/control_flow_analysis/analyze_return_paths.rs
@@ -81,14 +81,14 @@ impl<'cfg> ControlFlowGraph<'cfg> {
         return_ty: &TypeInfo,
     ) -> Vec<CompileError> {
         let mut rovers = vec![entry_point];
+        let mut visited = vec![];
         let mut errors = vec![];
-        let mut max_iterations = 50;
-        while !rovers.is_empty() && rovers[0] != exit_point && max_iterations > 0 {
-            max_iterations -= 1;
+        while !rovers.is_empty() && rovers[0] != exit_point {
             rovers.retain(|idx| *idx != exit_point);
             let mut next_rovers = vec![];
             let mut last_discovered_span;
             for rover in rovers {
+                visited.push(rover);
                 last_discovered_span = match &self.graph[rover] {
                     ControlFlowGraphNode::ProgramNode { node, .. } => Some(node.span.clone()),
                     ControlFlowGraphNode::MethodDeclaration { span, .. } => Some(span.clone()),
@@ -122,6 +122,7 @@ impl<'cfg> ControlFlowGraph<'cfg> {
                 }
                 next_rovers.append(&mut neighbors);
             }
+            next_rovers.retain(|idx| !visited.contains(idx));
             rovers = next_rovers;
         }
 
diff --git a/test/src/e2e_vm_tests/test_programs/should_fail/return_path_analysis/Forc.lock b/test/src/e2e_vm_tests/test_programs/should_fail/return_path_analysis/Forc.lock
new file mode 100644
index 00000000000..3fc7b0496ff
--- /dev/null
+++ b/test/src/e2e_vm_tests/test_programs/should_fail/return_path_analysis/Forc.lock
@@ -0,0 +1,16 @@
+[[package]]
+name = "core"
+source = "path+from-root-EA042F5351E6BA99"
+
+[[package]]
+name = "return_path_analysis"
+source = "member"
+dependencies = [
+    "core",
+    "std",
+]
+
+[[package]]
+name = "std"
+source = "path+from-root-EA042F5351E6BA99"
+dependencies = ["core"]
diff --git a/test/src/e2e_vm_tests/test_programs/should_fail/return_path_analysis/Forc.toml b/test/src/e2e_vm_tests/test_programs/should_fail/return_path_analysis/Forc.toml
new file mode 100644
index 00000000000..20181921672
--- /dev/null
+++ b/test/src/e2e_vm_tests/test_programs/should_fail/return_path_analysis/Forc.toml
@@ -0,0 +1,9 @@
+[project]
+authors = ["Fuel Labs <contact@fuel.sh>"]
+entry = "main.sw"
+license = "Apache-2.0"
+name = "return_path_analysis"
+
+[dependencies]
+core = { path = "../../../../../../sway-lib-core" }
+std = { path = "../../../reduced_std_libs/sway-lib-std-assert" }
\ No newline at end of file
diff --git a/test/src/e2e_vm_tests/test_programs/should_fail/return_path_analysis/json_abi_oracle.json b/test/src/e2e_vm_tests/test_programs/should_fail/return_path_analysis/json_abi_oracle.json
new file mode 100644
index 00000000000..fe51488c706
--- /dev/null
+++ b/test/src/e2e_vm_tests/test_programs/should_fail/return_path_analysis/json_abi_oracle.json
@@ -0,0 +1 @@
+[]
diff --git a/test/src/e2e_vm_tests/test_programs/should_fail/return_path_analysis/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_fail/return_path_analysis/src/main.sw
new file mode 100644
index 00000000000..87ad204b7fc
--- /dev/null
+++ b/test/src/e2e_vm_tests/test_programs/should_fail/return_path_analysis/src/main.sw
@@ -0,0 +1,525 @@
+script;
+
+// Shows that https://github.com/FuelLabs/sway/issues/6369 has been resolved.
+//
+// The depth of the return path analysis was limited to 50, which meant that there was no
+// requirement to return a value after 50 blocks.
+
+fn f() -> bool {
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+    if true {
+	return true;
+    } else {
+	return false;
+    };
+
+    // No value returned here, which is an error.
+}
+
+fn main() {
+    let _ = f();
+}
diff --git a/test/src/e2e_vm_tests/test_programs/should_fail/return_path_analysis/test.toml b/test/src/e2e_vm_tests/test_programs/should_fail/return_path_analysis/test.toml
new file mode 100644
index 00000000000..588a46a1294
--- /dev/null
+++ b/test/src/e2e_vm_tests/test_programs/should_fail/return_path_analysis/test.toml
@@ -0,0 +1,6 @@
+category = "fail"
+
+# check: $()This path must return a value of type "bool" from function "f", but it does not.
+# check: $()Aborting due to 1 error.
+
+