From fb97d308d62e39abb444f849af588d55df6282b8 Mon Sep 17 00:00:00 2001 From: Frotty Date: Sat, 29 Aug 2026 19:41:38 +0200 Subject: [PATCH] Make local-player analysis stack safe --- .../wurstio/CompiletimeFunctionRunner.java | 12 +- .../peeeq/wurstio/gui/WurstErrorWindow.java | 16 +- .../optimizer/LocalPlayerContextAnalyzer.java | 206 +++++++++++------- .../CompiletimeFunctionRunnerTests.java | 18 ++ .../wurstio/gui/WurstErrorWindowTests.java | 16 ++ .../wurstscript/tests/OptimizerTests.java | 31 +++ 6 files changed, 221 insertions(+), 78 deletions(-) create mode 100644 de.peeeq.wurstscript/src/test/java/de/peeeq/wurstio/CompiletimeFunctionRunnerTests.java create mode 100644 de.peeeq.wurstscript/src/test/java/de/peeeq/wurstio/gui/WurstErrorWindowTests.java diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/CompiletimeFunctionRunner.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/CompiletimeFunctionRunner.java index e9eab1d40..359c625ea 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/CompiletimeFunctionRunner.java +++ b/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,7 +233,9 @@ 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())) { @@ -242,6 +243,11 @@ private void sendErrors(Element origin, String msg, Throwable ex) { } } + 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 */ diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/gui/WurstErrorWindow.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/gui/WurstErrorWindow.java index cde6d8605..09b4ac678 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/gui/WurstErrorWindow.java +++ b/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()) { diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java index 33f36ca39..fcb1055f0 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java +++ b/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 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 work) { + List 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 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 work) { indexedElements.add(arguments); + List 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 containsExit = new IdentityHashMap<>(); + Deque 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, diff --git a/de.peeeq.wurstscript/src/test/java/de/peeeq/wurstio/CompiletimeFunctionRunnerTests.java b/de.peeeq.wurstscript/src/test/java/de/peeeq/wurstio/CompiletimeFunctionRunnerTests.java new file mode 100644 index 000000000..3bc6bae32 --- /dev/null +++ b/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"); + } +} diff --git a/de.peeeq.wurstscript/src/test/java/de/peeeq/wurstio/gui/WurstErrorWindowTests.java b/de.peeeq.wurstscript/src/test/java/de/peeeq/wurstio/gui/WurstErrorWindowTests.java new file mode 100644 index 000000000..17389f5a1 --- /dev/null +++ b/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("")); + assertTrue(WurstErrorWindow.isSyntheticSource(null)); + assertFalse(WurstErrorWindow.isSyntheticSource("wurst/Package.wurst")); + } +} diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java index 8ace50f65..3e4582bb1 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java @@ -2375,6 +2375,37 @@ public void reverseOrderedCallChainUsesLocalPlayerWorklist() { "GetLocalPlayer usage must propagate through the complete call chain"); } + @Test(timeOut = 10_000) + public void deeplyNestedImDoesNotOverflowLocalPlayerAnalysis() { + Element trace = Ast.NoExpr(); + ImStmts nested = JassIm.ImStmts(); + for (int i = 0; i < 20_000; i++) { + nested = JassIm.ImStmts(JassIm.ImIf(trace, JassIm.ImBoolVal(true), + nested, JassIm.ImStmts())); + } + ImFunction main = JassIm.ImFunction( + trace, + "main", + JassIm.ImTypeVars(), + JassIm.ImVars(), + JassIm.ImVoid(), + JassIm.ImVars(), + JassIm.ImStmts(JassIm.ImLoop(trace, nested)), + Collections.emptyList() + ); + ImProg prog = JassIm.ImProg( + trace, + JassIm.ImVars(), + JassIm.ImFunctions(main), + JassIm.ImMethods(), + JassIm.ImClasses(), + JassIm.ImTypeClassFuncs(), + new java.util.HashMap<>() + ); + + new LocalPlayerContextAnalyzer(prog); + } + private static int countOccurrences(String text, String needle) { int count = 0; int from = 0;