Make local-player analysis stack safe (#1268)

Author: Frotty

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/CompiletimeFunctionRunner.java

@@ -157,8 +157,7 @@ public void run() {
             de.peeeq.wurstscript.jassIm.Element s = interpreter.getLastStatement();
             Element origin = s == null ? null : s.attrTrace();
             if (origin != null) {
-                String msg = e.getMessage();
-                sendErrors(origin, msg, e);
+                sendErrors(origin, describeFailure(e), e);
             } else {
                 throw new Error("could not get origin", e);
             }
@@ -234,14 +233,21 @@ private boolean isUnitTestMode() {
     }
 
     private void sendErrors(Element origin, String msg, Throwable ex) {
-        gui.sendError(new CompileError(origin.attrSource(), msg, CompileError.ErrorType.ERROR, ex));
+        gui.sendError(new CompileError(origin.attrSource(),
+                msg == null || msg.isBlank() ? describeFailure(ex) : msg,
+                CompileError.ErrorType.ERROR, ex));
 
         // stackframe messages ...
         for (ILStackFrame sf : Utils.iterateReverse(interpreter.getStackFrames().getStackFrames())) {
             gui.sendError(sf.makeCompileError());
         }
     }
 
+    static String describeFailure(Throwable failure) {
+        String message = failure.getMessage();
+        return message == null || message.isBlank() ? failure.getClass().getSimpleName() : message;
+    }
+
     /**
      * Run actions that must be run after all other code
      */

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/gui/WurstErrorWindow.java

@@ -240,8 +240,15 @@ private void viewErrorDetail(CompileError err) {
         setVisible(true);
         this.errorDetailsPanel.setText(err.getMessage());
 
-        File errFile = new File(err.getSource().getFile());
-        File workspaceErrFile = new File(workspaceRoot + "/" + err.getSource().getFile());
+        String sourceFile = err.getSource().getFile();
+        if (isSyntheticSource(sourceFile)) {
+            currentFile = null;
+            codeArea.setText("No source file is available for this compiler-generated error.");
+            return;
+        }
+
+        File errFile = new File(sourceFile);
+        File workspaceErrFile = new File(workspaceRoot + "/" + sourceFile);
 
         if (!errFile.exists() && workspaceErrFile.exists()) {
             errFile = workspaceErrFile;
@@ -314,6 +321,11 @@ private void viewErrorDetail(CompileError err) {
         }
     }
 
+    static boolean isSyntheticSource(String sourceFile) {
+        return sourceFile == null || sourceFile.isBlank()
+                || sourceFile.startsWith("<") && sourceFile.endsWith(">");
+    }
+
     //	@Override
     public void sendError(CompileError elem) {
         if (errorListModel.isEmpty()) {

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java

@@ -189,55 +189,69 @@ private boolean methodReturnsLocalPlayerDependentValue(ImMethod method) {
         return false;
     }
 
-    private void indexElement(Element element, ImFunction owner, Object controlContext) {
-        indexedElements.add(element);
+    private record IndexTask(Element element, Object controlContext, boolean afterChildren) {
+    }
 
-        Object branchControl = null;
-        if (element instanceof ImIf) {
-            ImIf ifStmt = (ImIf) element;
-            branchControl = new Fact(FactKind.CONTROL, ifStmt);
-            addDependency(ifStmt.getCondition(), branchControl);
-            addEnclosingControlDependency(controlContext, branchControl);
-        }
+    private record ReturnTask(Element element, boolean afterChildren) {
+    }
 
-        Object loopControl = null;
-        if (element instanceof ImLoop) {
-            ImLoop loop = (ImLoop) element;
-            loopControl = new Fact(FactKind.CONTROL, loop);
-            addEnclosingControlDependency(controlContext, loopControl);
-            addLoopExitDependencies(loop.getBody(), loopControl);
-        }
+    private record LoopExitTask(Element element, boolean afterChildren) {
+    }
 
-        if (element instanceof ImStmts) {
-            indexStatementSequence((ImStmts) element, owner, controlContext);
-            return;
-        }
+    private void indexElement(Element root, ImFunction owner, Object controlContext) {
+        Deque<IndexTask> work = new ArrayDeque<>();
+        work.addFirst(new IndexTask(root, controlContext, false));
+        while (!work.isEmpty()) {
+            IndexTask task = work.removeFirst();
+            Element element = task.element();
+            if (task.afterChildren()) {
+                indexElementAfterChildren(element, owner, task.controlContext());
+                continue;
+            }
 
-        for (int i = 0; i < element.size(); i++) {
-            Element child = element.get(i);
-            if (element instanceof ImOperatorCall
-                && ((ImOperatorCall) element).getOp().isLazy()
-                && child == ((ImOperatorCall) element).getArguments()) {
-                indexShortCircuitArguments(
-                    ((ImOperatorCall) element).getArguments(),
-                    owner,
-                    controlContext);
-                addDependency(child, element);
+            indexedElements.add(element);
+            Object branchControl = null;
+            if (element instanceof ImIf ifStmt) {
+                branchControl = new Fact(FactKind.CONTROL, ifStmt);
+                addDependency(ifStmt.getCondition(), branchControl);
+                addEnclosingControlDependency(task.controlContext(), branchControl);
+            }
+
+            Object loopControl = null;
+            if (element instanceof ImLoop loop) {
+                loopControl = new Fact(FactKind.CONTROL, loop);
+                addEnclosingControlDependency(task.controlContext(), loopControl);
+                addLoopExitDependencies(loop.getBody(), loopControl);
+            }
+
+            if (element instanceof ImStmts statements) {
+                scheduleStatementSequence(statements, task.controlContext(), work);
                 continue;
             }
-            Object childControl = controlContext;
-            if (element instanceof ImIf
-                && (child == ((ImIf) element).getThenBlock()
-                || child == ((ImIf) element).getElseBlock())) {
-                childControl = branchControl;
-            } else if (element instanceof ImLoop
-                && child == ((ImLoop) element).getBody()) {
-                childControl = loopControl;
+
+            work.addFirst(new IndexTask(element, task.controlContext(), true));
+            for (int i = element.size() - 1; i >= 0; i--) {
+                Element child = element.get(i);
+                addDependency(child, element);
+                if (element instanceof ImOperatorCall operator
+                    && operator.getOp().isLazy()
+                    && child == operator.getArguments()) {
+                    scheduleShortCircuitArguments(operator.getArguments(), task.controlContext(), work);
+                    continue;
+                }
+                Object childControl = task.controlContext();
+                if (element instanceof ImIf ifStmt
+                    && (child == ifStmt.getThenBlock() || child == ifStmt.getElseBlock())) {
+                    childControl = branchControl;
+                } else if (element instanceof ImLoop loop && child == loop.getBody()) {
+                    childControl = loopControl;
+                }
+                work.addFirst(new IndexTask(child, childControl, false));
             }
-            indexElement(child, owner, childControl);
-            addDependency(child, element);
         }
+    }
 
+    private void indexElementAfterChildren(Element element, ImFunction owner, Object controlContext) {
         if (element instanceof ImVarAccess) {
             addDependency(variableFact(((ImVarAccess) element).getVar()), element);
         } else if (element instanceof ImVarArrayAccess) {
@@ -273,13 +287,14 @@ private void indexElement(Element element, ImFunction owner, Object controlConte
         }
     }
 
-    private void indexStatementSequence(ImStmts statements,
-                                        ImFunction owner,
-                                        Object controlContext) {
+    private void scheduleStatementSequence(ImStmts statements,
+                                           Object controlContext,
+                                           Deque<IndexTask> work) {
+        List<IndexTask> tasks = new ArrayList<>(statements.size());
         Object continuationControl = controlContext;
         for (ImStmt statement : statements) {
-            indexElement(statement, owner, continuationControl);
             addDependency(statement, statements);
+            tasks.add(new IndexTask(statement, continuationControl, false));
 
             if (containsFunctionReturn(statement)) {
                 Fact followingStatementControl =
@@ -291,36 +306,60 @@ private void indexStatementSequence(ImStmts statements,
                 continuationControl = followingStatementControl;
             }
         }
+        for (int i = tasks.size() - 1; i >= 0; i--) {
+            work.addFirst(tasks.get(i));
+        }
     }
 
-    private boolean containsFunctionReturn(Element element) {
-        Boolean cached = containsReturnCache.get(element);
+    private boolean containsFunctionReturn(Element root) {
+        Boolean cached = containsReturnCache.get(root);
         if (cached != null) {
             return cached;
         }
-        if (element instanceof ImReturn) {
-            containsReturnCache.put(element, true);
-            return true;
-        }
-        for (int i = 0; i < element.size(); i++) {
-            if (containsFunctionReturn(element.get(i))) {
+        Deque<ReturnTask> work = new ArrayDeque<>();
+        work.addFirst(new ReturnTask(root, false));
+        while (!work.isEmpty()) {
+            ReturnTask task = work.removeFirst();
+            Element element = task.element();
+            if (containsReturnCache.containsKey(element)) {
+                continue;
+            }
+            if (element instanceof ImReturn) {
                 containsReturnCache.put(element, true);
-                return true;
+                continue;
+            }
+            if (!task.afterChildren()) {
+                work.addFirst(new ReturnTask(element, true));
+                for (int i = element.size() - 1; i >= 0; i--) {
+                    Element child = element.get(i);
+                    if (!containsReturnCache.containsKey(child)) {
+                        work.addFirst(new ReturnTask(child, false));
+                    }
+                }
+                continue;
             }
+            boolean containsReturn = false;
+            for (int i = 0; i < element.size(); i++) {
+                if (Boolean.TRUE.equals(containsReturnCache.get(element.get(i)))) {
+                    containsReturn = true;
+                    break;
+                }
+            }
+            containsReturnCache.put(element, containsReturn);
         }
-        containsReturnCache.put(element, false);
-        return false;
+        return Boolean.TRUE.equals(containsReturnCache.get(root));
     }
 
-    private void indexShortCircuitArguments(ImExprs arguments,
-                                            ImFunction owner,
-                                            Object controlContext) {
+    private void scheduleShortCircuitArguments(ImExprs arguments,
+                                               Object controlContext,
+                                               Deque<IndexTask> work) {
         indexedElements.add(arguments);
+        List<IndexTask> tasks = new ArrayList<>(arguments.size());
         Object operandControl = controlContext;
         for (int i = 0; i < arguments.size(); i++) {
             ImExpr argument = arguments.get(i);
-            indexElement(argument, owner, operandControl);
             addDependency(argument, arguments);
+            tasks.add(new IndexTask(argument, operandControl, false));
 
             if (i + 1 < arguments.size()) {
                 Fact followingOperandControl =
@@ -332,6 +371,9 @@ private void indexShortCircuitArguments(ImExprs arguments,
                 operandControl = followingOperandControl;
             }
         }
+        for (int i = tasks.size() - 1; i >= 0; i--) {
+            work.addFirst(tasks.get(i));
+        }
     }
 
     private void indexFunctionCall(ImFunctionCall call, ImFunction owner, Object controlContext) {
@@ -402,22 +444,40 @@ private void addEnclosingControlDependency(Object controlContext, Object depende
         }
     }
 
-    private boolean addLoopExitDependencies(Element element, Object loopControl) {
-        if (element instanceof ImExitwhen) {
-            addDependency(((ImExitwhen) element).getCondition(), loopControl);
-            return true;
-        } else if (element instanceof ImLoop || element instanceof ImVarargLoop) {
-            return false;
-        }
+    private boolean addLoopExitDependencies(Element root, Object loopControl) {
+        Map<Element, Boolean> containsExit = new IdentityHashMap<>();
+        Deque<LoopExitTask> work = new ArrayDeque<>();
+        work.addFirst(new LoopExitTask(root, false));
+        while (!work.isEmpty()) {
+            LoopExitTask task = work.removeFirst();
+            Element element = task.element();
+            if (element instanceof ImExitwhen exitwhen) {
+                addDependency(exitwhen.getCondition(), loopControl);
+                containsExit.put(element, true);
+                continue;
+            }
+            if (element instanceof ImLoop || element instanceof ImVarargLoop) {
+                containsExit.put(element, false);
+                continue;
+            }
+            if (!task.afterChildren()) {
+                work.addFirst(new LoopExitTask(element, true));
+                for (int i = element.size() - 1; i >= 0; i--) {
+                    work.addFirst(new LoopExitTask(element.get(i), false));
+                }
+                continue;
+            }
 
-        boolean containsExit = false;
-        for (int i = 0; i < element.size(); i++) {
-            containsExit |= addLoopExitDependencies(element.get(i), loopControl);
-        }
-        if (containsExit && element instanceof ImIf) {
-            addDependency(((ImIf) element).getCondition(), loopControl);
+            boolean elementContainsExit = false;
+            for (int i = 0; i < element.size(); i++) {
+                elementContainsExit |= Boolean.TRUE.equals(containsExit.get(element.get(i)));
+            }
+            if (elementContainsExit && element instanceof ImIf ifStmt) {
+                addDependency(ifStmt.getCondition(), loopControl);
+            }
+            containsExit.put(element, elementContainsExit);
         }
-        return containsExit;
+        return Boolean.TRUE.equals(containsExit.get(root));
     }
 
     private boolean collectMethodImplementations(ImMethod method,

de.peeeq.wurstscript/src/test/java/de/peeeq/wurstio/CompiletimeFunctionRunnerTests.java

@@ -0,0 +1,18 @@
+package de.peeeq.wurstio;
+
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+
+public class CompiletimeFunctionRunnerTests {
+
+    @Test
+    public void throwableWithoutMessageUsesItsType() {
+        assertEquals(CompiletimeFunctionRunner.describeFailure(new StackOverflowError()), "StackOverflowError");
+    }
+
+    @Test
+    public void throwableMessageIsPreserved() {
+        assertEquals(CompiletimeFunctionRunner.describeFailure(new RuntimeException("details")), "details");
+    }
+}

de.peeeq.wurstscript/src/test/java/de/peeeq/wurstio/gui/WurstErrorWindowTests.java

@@ -0,0 +1,16 @@
+package de.peeeq.wurstio.gui;
+
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
+
+public class WurstErrorWindowTests {
+
+    @Test
+    public void syntheticSourceIsNotTreatedAsAFile() {
+        assertTrue(WurstErrorWindow.isSyntheticSource("<source of NoExpr not found>"));
+        assertTrue(WurstErrorWindow.isSyntheticSource(null));
+        assertFalse(WurstErrorWindow.isSyntheticSource("wurst/Package.wurst"));
+    }
+}