From 781cba1c3ed3010461c4de152422258ceedc9e0b Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Thu, 29 Feb 2024 16:04:17 +0100 Subject: [PATCH 1/8] This fixes #1920 * stackoverflow errors were not reported properly. Instead the interpreter triggered many more stackoverflow errors while trying to report on them. * fixed this by capturing the deepest overflow exception and wrapping the current runtime stack in a cheap exception object. This object is then thrown an caught by the REPL loop which prints the proper exception stack trace. * We lost the ability to catch a stackOverflow() exception in Rascal code with this. This is problematic since there are tools that use that (drAmbiguity) in case of expected eternal recursions. So for now this is PR and I would like to hear if anybody has ideas on how to fix this properly without loss of this important feature. --- .../exceptions/RascalStackOverflowError.java | 46 +++++++++++++++++++ .../interpreter/result/RascalFunction.java | 13 ++++++ .../rascalmpl/repl/RascalInterpreterREPL.java | 5 ++ .../semantics/dynamic/Expression.java | 12 +++-- 4 files changed, 72 insertions(+), 4 deletions(-) create mode 100644 src/org/rascalmpl/exceptions/RascalStackOverflowError.java diff --git a/src/org/rascalmpl/exceptions/RascalStackOverflowError.java b/src/org/rascalmpl/exceptions/RascalStackOverflowError.java new file mode 100644 index 00000000000..80c65cccb42 --- /dev/null +++ b/src/org/rascalmpl/exceptions/RascalStackOverflowError.java @@ -0,0 +1,46 @@ +/******************************************************************************* + * Copyright (c) 2024 CWI + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + + * * Jurgen J. Vinju - Jurgen.Vinju@cwi.nl - CWI +*******************************************************************************/ +package org.rascalmpl.exceptions; + +import org.rascalmpl.ast.AbstractAST; +import org.rascalmpl.interpreter.IEvaluatorContext; +import org.rascalmpl.interpreter.env.Environment; + +/** + * This class captures the runtime state of the interpreter at the moment + * we detect a java StackOverflowError. Since we can not use any stack depth + * at that moment to create a real Throw exception, we only copy the deepest + * environment and wrap it in here. Later on the level of the REPL, when the + * stack is completely unrolled, we can convert the stack trace to a Rascal trace. + */ +public class RascalStackOverflowError extends RuntimeException { + private static final long serialVersionUID = -3947588548271683963L; + private final Environment deepestEnvironment; + private final AbstractAST currentAST; + + public RascalStackOverflowError(AbstractAST current, Environment deepest) { + this.deepestEnvironment = deepest; + this.currentAST = current; + } + + public Throw makeThrow() { + StackTrace trace = new StackTrace(); + Environment env = deepestEnvironment; + + while (env != null) { + trace.add(env.getLocation(), env.getName()); + env = env.getCallerScope(); + } + + return RuntimeExceptionFactory.stackOverflow(currentAST, trace); + } +} diff --git a/src/org/rascalmpl/interpreter/result/RascalFunction.java b/src/org/rascalmpl/interpreter/result/RascalFunction.java index 7eabbc774e7..b1a2c57c2b3 100644 --- a/src/org/rascalmpl/interpreter/result/RascalFunction.java +++ b/src/org/rascalmpl/interpreter/result/RascalFunction.java @@ -40,6 +40,7 @@ import org.rascalmpl.ast.Statement; import org.rascalmpl.ast.Type.Structured; import org.rascalmpl.exceptions.ImplementationError; +import org.rascalmpl.exceptions.RuntimeExceptionFactory; import org.rascalmpl.interpreter.Accumulator; import org.rascalmpl.interpreter.IEvaluator; import org.rascalmpl.interpreter.IEvaluatorContext; @@ -303,6 +304,18 @@ public Result call(Type[] actualStaticTypes, IValue[] actuals, Map 0) { + // trace.add(env.getLocation(), env.getName()); + // env = env.getCallerScope(); + // } + + throw new RuntimeException("hello"); + } } matchers[0].initMatch(makeResult(actualStaticTypesTuple.getFieldType(0), actuals[0], ctx)); diff --git a/src/org/rascalmpl/repl/RascalInterpreterREPL.java b/src/org/rascalmpl/repl/RascalInterpreterREPL.java index 5333e75a9b6..d9d50b88316 100644 --- a/src/org/rascalmpl/repl/RascalInterpreterREPL.java +++ b/src/org/rascalmpl/repl/RascalInterpreterREPL.java @@ -19,6 +19,7 @@ import java.util.function.Function; import java.util.stream.Collectors; +import org.rascalmpl.exceptions.RascalStackOverflowError; import org.rascalmpl.exceptions.StackTrace; import org.rascalmpl.exceptions.Throw; import org.rascalmpl.ideservices.IDEServices; @@ -150,6 +151,10 @@ public IRascalResult evalStatement(String statement, String lastLine) throws Int parseErrorMessage(eval.getErrorPrinter(), lastLine, "prompt", pe, indentedPrettyPrinter); return null; } + catch (RascalStackOverflowError e) { + throwMessage(eval.getErrorPrinter(), e.makeThrow(), indentedPrettyPrinter); + return null; + } catch (StaticError e) { staticErrorMessage(eval.getErrorPrinter(),e, indentedPrettyPrinter); return null; diff --git a/src/org/rascalmpl/semantics/dynamic/Expression.java b/src/org/rascalmpl/semantics/dynamic/Expression.java index ba5f629189d..d4dce5267a6 100644 --- a/src/org/rascalmpl/semantics/dynamic/Expression.java +++ b/src/org/rascalmpl/semantics/dynamic/Expression.java @@ -32,7 +32,9 @@ import org.rascalmpl.ast.Parameters; import org.rascalmpl.ast.Statement; import org.rascalmpl.exceptions.ImplementationError; +import org.rascalmpl.exceptions.RascalStackOverflowError; import org.rascalmpl.exceptions.RuntimeExceptionFactory; +import org.rascalmpl.exceptions.StackTrace; import org.rascalmpl.exceptions.Throw; import org.rascalmpl.interpreter.IEvaluator; import org.rascalmpl.interpreter.IEvaluatorContext; @@ -542,12 +544,14 @@ public Result interpret(IEvaluator> eval) { catch (Failure | MatchFailed e) { throw RuntimeExceptionFactory.callFailed(eval.getValueFactory().list(actuals), eval.getCurrentAST(), eval.getStackTrace()); } + catch (StackOverflowError e) { + // this should not use so much stack as to trigger another StackOverflowError + throw new RascalStackOverflowError(this, eval.getCurrentEnvt()); + } return res; } - catch (StackOverflowError e) { - e.printStackTrace(); - throw RuntimeExceptionFactory.stackOverflow(this, eval.getStackTrace()); - } + finally {} + } @Override From b55cebf36cfba78e2914cbbdf2192dec3656f23d Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Thu, 29 Feb 2024 16:31:19 +0100 Subject: [PATCH 2/8] removed diagnostics code and added some info to the overflow error --- .../exceptions/RascalStackOverflowError.java | 12 +++++++++++- .../interpreter/result/RascalFunction.java | 13 +------------ src/org/rascalmpl/semantics/dynamic/Expression.java | 2 +- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/org/rascalmpl/exceptions/RascalStackOverflowError.java b/src/org/rascalmpl/exceptions/RascalStackOverflowError.java index 80c65cccb42..2abd8326f7c 100644 --- a/src/org/rascalmpl/exceptions/RascalStackOverflowError.java +++ b/src/org/rascalmpl/exceptions/RascalStackOverflowError.java @@ -26,10 +26,12 @@ public class RascalStackOverflowError extends RuntimeException { private static final long serialVersionUID = -3947588548271683963L; private final Environment deepestEnvironment; private final AbstractAST currentAST; + private final int depth; - public RascalStackOverflowError(AbstractAST current, Environment deepest) { + public RascalStackOverflowError(AbstractAST current, Environment deepest, int depth) { this.deepestEnvironment = deepest; this.currentAST = current; + this.depth = depth; } public Throw makeThrow() { @@ -43,4 +45,12 @@ public Throw makeThrow() { return RuntimeExceptionFactory.stackOverflow(currentAST, trace); } + + public Environment getEnvironment() { + return deepestEnvironment; + } + + public int getDepth() { + return depth; + } } diff --git a/src/org/rascalmpl/interpreter/result/RascalFunction.java b/src/org/rascalmpl/interpreter/result/RascalFunction.java index b1a2c57c2b3..629c3f4eeb3 100644 --- a/src/org/rascalmpl/interpreter/result/RascalFunction.java +++ b/src/org/rascalmpl/interpreter/result/RascalFunction.java @@ -40,6 +40,7 @@ import org.rascalmpl.ast.Statement; import org.rascalmpl.ast.Type.Structured; import org.rascalmpl.exceptions.ImplementationError; +import org.rascalmpl.exceptions.RascalStackOverflowError; import org.rascalmpl.exceptions.RuntimeExceptionFactory; import org.rascalmpl.interpreter.Accumulator; import org.rascalmpl.interpreter.IEvaluator; @@ -304,18 +305,6 @@ public Result call(Type[] actualStaticTypes, IValue[] actuals, Map 0) { - // trace.add(env.getLocation(), env.getName()); - // env = env.getCallerScope(); - // } - - throw new RuntimeException("hello"); - } } matchers[0].initMatch(makeResult(actualStaticTypesTuple.getFieldType(0), actuals[0], ctx)); diff --git a/src/org/rascalmpl/semantics/dynamic/Expression.java b/src/org/rascalmpl/semantics/dynamic/Expression.java index d4dce5267a6..fbb35caf223 100644 --- a/src/org/rascalmpl/semantics/dynamic/Expression.java +++ b/src/org/rascalmpl/semantics/dynamic/Expression.java @@ -546,7 +546,7 @@ public Result interpret(IEvaluator> eval) { } catch (StackOverflowError e) { // this should not use so much stack as to trigger another StackOverflowError - throw new RascalStackOverflowError(this, eval.getCurrentEnvt()); + throw new RascalStackOverflowError(this, eval.getCurrentEnvt(), eval.getCallNesting()); } return res; } From d089ff646550a3b3a162b24b0ee70995d91d2197 Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Thu, 29 Feb 2024 16:32:06 +0100 Subject: [PATCH 3/8] depth was always 0 --- .../rascalmpl/exceptions/RascalStackOverflowError.java | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/org/rascalmpl/exceptions/RascalStackOverflowError.java b/src/org/rascalmpl/exceptions/RascalStackOverflowError.java index 2abd8326f7c..cecee677b0d 100644 --- a/src/org/rascalmpl/exceptions/RascalStackOverflowError.java +++ b/src/org/rascalmpl/exceptions/RascalStackOverflowError.java @@ -12,7 +12,6 @@ package org.rascalmpl.exceptions; import org.rascalmpl.ast.AbstractAST; -import org.rascalmpl.interpreter.IEvaluatorContext; import org.rascalmpl.interpreter.env.Environment; /** @@ -26,12 +25,10 @@ public class RascalStackOverflowError extends RuntimeException { private static final long serialVersionUID = -3947588548271683963L; private final Environment deepestEnvironment; private final AbstractAST currentAST; - private final int depth; - public RascalStackOverflowError(AbstractAST current, Environment deepest, int depth) { + public RascalStackOverflowError(AbstractAST current, Environment deepest) { this.deepestEnvironment = deepest; this.currentAST = current; - this.depth = depth; } public Throw makeThrow() { @@ -49,8 +46,4 @@ public Throw makeThrow() { public Environment getEnvironment() { return deepestEnvironment; } - - public int getDepth() { - return depth; - } } From 767ea0b5b26fa965a6c30c7772896c87cdc4e0ec Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Thu, 29 Feb 2024 16:36:51 +0100 Subject: [PATCH 4/8] cleanup imports --- src/org/rascalmpl/interpreter/result/RascalFunction.java | 2 -- src/org/rascalmpl/semantics/dynamic/Expression.java | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/org/rascalmpl/interpreter/result/RascalFunction.java b/src/org/rascalmpl/interpreter/result/RascalFunction.java index 629c3f4eeb3..7eabbc774e7 100644 --- a/src/org/rascalmpl/interpreter/result/RascalFunction.java +++ b/src/org/rascalmpl/interpreter/result/RascalFunction.java @@ -40,8 +40,6 @@ import org.rascalmpl.ast.Statement; import org.rascalmpl.ast.Type.Structured; import org.rascalmpl.exceptions.ImplementationError; -import org.rascalmpl.exceptions.RascalStackOverflowError; -import org.rascalmpl.exceptions.RuntimeExceptionFactory; import org.rascalmpl.interpreter.Accumulator; import org.rascalmpl.interpreter.IEvaluator; import org.rascalmpl.interpreter.IEvaluatorContext; diff --git a/src/org/rascalmpl/semantics/dynamic/Expression.java b/src/org/rascalmpl/semantics/dynamic/Expression.java index fbb35caf223..2711138f309 100644 --- a/src/org/rascalmpl/semantics/dynamic/Expression.java +++ b/src/org/rascalmpl/semantics/dynamic/Expression.java @@ -34,7 +34,6 @@ import org.rascalmpl.exceptions.ImplementationError; import org.rascalmpl.exceptions.RascalStackOverflowError; import org.rascalmpl.exceptions.RuntimeExceptionFactory; -import org.rascalmpl.exceptions.StackTrace; import org.rascalmpl.exceptions.Throw; import org.rascalmpl.interpreter.IEvaluator; import org.rascalmpl.interpreter.IEvaluatorContext; @@ -546,7 +545,7 @@ public Result interpret(IEvaluator> eval) { } catch (StackOverflowError e) { // this should not use so much stack as to trigger another StackOverflowError - throw new RascalStackOverflowError(this, eval.getCurrentEnvt(), eval.getCallNesting()); + throw new RascalStackOverflowError(this, eval.getCurrentEnvt()); } return res; } From 68b422b1ad336524e093f75898f69ff2d20f8550 Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Sat, 9 Mar 2024 13:23:47 +0100 Subject: [PATCH 5/8] try-catch can now convert a java StackOverflowError to a Rascal StackOverflow exception just in time for the ctach block --- .../semantics/dynamic/Statement.java | 38 ++++++++++++++++++- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/src/org/rascalmpl/semantics/dynamic/Statement.java b/src/org/rascalmpl/semantics/dynamic/Statement.java index 8bedcc2bcd5..a06932a967e 100644 --- a/src/org/rascalmpl/semantics/dynamic/Statement.java +++ b/src/org/rascalmpl/semantics/dynamic/Statement.java @@ -28,6 +28,7 @@ import org.rascalmpl.ast.QualifiedName; import org.rascalmpl.ast.Target; import org.rascalmpl.ast.Type; +import org.rascalmpl.exceptions.RuntimeExceptionFactory; import org.rascalmpl.interpreter.Accumulator; import org.rascalmpl.interpreter.AssignableEvaluator; import org.rascalmpl.interpreter.IEvaluator; @@ -941,7 +942,7 @@ static public Result evalStatementTry(IEvaluator> eval, o IValue eValue = e.getException(); boolean handled = false; - +r for (Catch c : handlers) { if (c.isDefault()) { res = c.getBody().interpret(eval); @@ -957,13 +958,46 @@ static public Result evalStatementTry(IEvaluator> eval, o if (!handled) throw e; - } finally { + } + catch (StackOverflowError e) { + // and now we pretend as if a Rascal stackoverflow has been thrown, such that + // it can be caugt in this catch block if necessary: + boolean handled = false; + + for (Catch c : handlers) { + if (c.hasPattern() && isCatchStackOverflow(c.getPattern())) { + IValue pseudo = RuntimeExceptionFactory.stackOverflow().getException(); + + if (Cases.matchAndEval(makeResult(pseudo.getType(), pseudo, eval), c.getPattern().buildMatcher(eval, false), c.getBody(), eval)) { + handled = true; + break; + } + } + } + + if (!handled) { + throw e; + } + } + finally { if (finallyBody != null) { finallyBody.interpret(eval); } } return res; } + + private static boolean isCatchStackOverflow(org.rascalmpl.ast.Expression pattern) { + if (pattern.isVariableBecomes() || pattern.isTypedVariableBecomes()) { + return isCatchStackOverflow(pattern.getPattern()); + } + else if (pattern.isCallOrTree()) { + return pattern.getArguments().isEmpty() && "StackOverflow".equals(Names.name(pattern.getName())); + } + else { + return false; + } + } } static public class TryFinally extends From 6595ea81d2abb6611d402095933f3ab6407afef8 Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Mon, 11 Mar 2024 11:23:09 +0100 Subject: [PATCH 6/8] fixed catch of stackoverflow to create the right stack trace --- src/org/rascalmpl/semantics/dynamic/Statement.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/org/rascalmpl/semantics/dynamic/Statement.java b/src/org/rascalmpl/semantics/dynamic/Statement.java index a06932a967e..c3af803f3b2 100644 --- a/src/org/rascalmpl/semantics/dynamic/Statement.java +++ b/src/org/rascalmpl/semantics/dynamic/Statement.java @@ -28,6 +28,7 @@ import org.rascalmpl.ast.QualifiedName; import org.rascalmpl.ast.Target; import org.rascalmpl.ast.Type; +import org.rascalmpl.exceptions.RascalStackOverflowError; import org.rascalmpl.exceptions.RuntimeExceptionFactory; import org.rascalmpl.interpreter.Accumulator; import org.rascalmpl.interpreter.AssignableEvaluator; @@ -959,14 +960,14 @@ static public Result evalStatementTry(IEvaluator> eval, o if (!handled) throw e; } - catch (StackOverflowError e) { - // and now we pretend as if a Rascal stackoverflow has been thrown, such that + catch (RascalStackOverflowError e) { + // and now we pretend as if a real Stackoverflow() value has been thrown, such that // it can be caugt in this catch block if necessary: boolean handled = false; for (Catch c : handlers) { if (c.hasPattern() && isCatchStackOverflow(c.getPattern())) { - IValue pseudo = RuntimeExceptionFactory.stackOverflow().getException(); + IValue pseudo = e.makeThrow().getException(); if (Cases.matchAndEval(makeResult(pseudo.getType(), pseudo, eval), c.getPattern().buildMatcher(eval, false), c.getBody(), eval)) { handled = true; @@ -976,6 +977,7 @@ static public Result evalStatementTry(IEvaluator> eval, o } if (!handled) { + // we rethrow because higher up the stack may be another catch block throw e; } } From e91d0680f67a93a76e917c6f34235b008f638845 Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Thu, 21 Nov 2024 12:02:51 +0100 Subject: [PATCH 7/8] added test and fixed bugs that the test triggered --- .../library/lang/rascal/tests/basic/Functions.rsc | 11 +++++++++++ src/org/rascalmpl/semantics/dynamic/Statement.java | 13 +++++++------ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/org/rascalmpl/library/lang/rascal/tests/basic/Functions.rsc b/src/org/rascalmpl/library/lang/rascal/tests/basic/Functions.rsc index fec17b3fa7d..3e5d6930ff4 100644 --- a/src/org/rascalmpl/library/lang/rascal/tests/basic/Functions.rsc +++ b/src/org/rascalmpl/library/lang/rascal/tests/basic/Functions.rsc @@ -588,3 +588,14 @@ test bool innerAndOuterFunctionUseSameParameterName4(){ return outer("a") == 3; } + +test bool stackoverflow() { + int f(int i) = f(i); + + try { + f(1); + return false; + } + catch StackOverflow(): + return true; +} \ No newline at end of file diff --git a/src/org/rascalmpl/semantics/dynamic/Statement.java b/src/org/rascalmpl/semantics/dynamic/Statement.java index c3af803f3b2..cd859683c21 100644 --- a/src/org/rascalmpl/semantics/dynamic/Statement.java +++ b/src/org/rascalmpl/semantics/dynamic/Statement.java @@ -29,7 +29,6 @@ import org.rascalmpl.ast.Target; import org.rascalmpl.ast.Type; import org.rascalmpl.exceptions.RascalStackOverflowError; -import org.rascalmpl.exceptions.RuntimeExceptionFactory; import org.rascalmpl.interpreter.Accumulator; import org.rascalmpl.interpreter.AssignableEvaluator; import org.rascalmpl.interpreter.IEvaluator; @@ -943,7 +942,7 @@ static public Result evalStatementTry(IEvaluator> eval, o IValue eValue = e.getException(); boolean handled = false; -r + for (Catch c : handlers) { if (c.isDefault()) { res = c.getBody().interpret(eval); @@ -994,11 +993,13 @@ private static boolean isCatchStackOverflow(org.rascalmpl.ast.Expression pattern return isCatchStackOverflow(pattern.getPattern()); } else if (pattern.isCallOrTree()) { - return pattern.getArguments().isEmpty() && "StackOverflow".equals(Names.name(pattern.getName())); - } - else { - return false; + var called = pattern.getExpression(); + if (called.hasName()) { + return pattern.getArguments().isEmpty() && "StackOverflow".equals(Names.name(called.getName())); + } } + + return false; } } From 5004cedfdd35ba35955ff46215a595068dbdd814 Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Fri, 22 Nov 2024 09:38:46 +0100 Subject: [PATCH 8/8] fixed catch of stackoverflow --- src/org/rascalmpl/semantics/dynamic/Statement.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/org/rascalmpl/semantics/dynamic/Statement.java b/src/org/rascalmpl/semantics/dynamic/Statement.java index cd859683c21..d9c472817de 100644 --- a/src/org/rascalmpl/semantics/dynamic/Statement.java +++ b/src/org/rascalmpl/semantics/dynamic/Statement.java @@ -994,8 +994,9 @@ private static boolean isCatchStackOverflow(org.rascalmpl.ast.Expression pattern } else if (pattern.isCallOrTree()) { var called = pattern.getExpression(); - if (called.hasName()) { - return pattern.getArguments().isEmpty() && "StackOverflow".equals(Names.name(called.getName())); + if (called.isQualifiedName()) { + var qname = called.getQualifiedName(); + return pattern.getArguments().isEmpty() && "StackOverflow".equals(Names.consName(qname)); } }