From 79864f2c2c095642d119bccacb99bf40a39e0bb8 Mon Sep 17 00:00:00 2001 From: domenico Date: Wed, 30 Oct 2024 15:57:54 +0100 Subject: [PATCH 1/5] Add test case --- .../rpgparser/smeup/MULANGT10BaseCodopTest.kt | 10 +++++ .../src/test/resources/smeup/ERRORPGM2.rpgle | 2 + .../test/resources/smeup/MUDRNRAPU00268.rpgle | 2 +- .../test/resources/smeup/MUDRNRAPU00269.rpgle | 38 +++++++++++++++++++ 4 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 rpgJavaInterpreter-core/src/test/resources/smeup/ERRORPGM2.rpgle create mode 100644 rpgJavaInterpreter-core/src/test/resources/smeup/MUDRNRAPU00269.rpgle diff --git a/rpgJavaInterpreter-core/src/test/kotlin/com/smeup/rpgparser/smeup/MULANGT10BaseCodopTest.kt b/rpgJavaInterpreter-core/src/test/kotlin/com/smeup/rpgparser/smeup/MULANGT10BaseCodopTest.kt index 54550c9ff..ba664b18d 100644 --- a/rpgJavaInterpreter-core/src/test/kotlin/com/smeup/rpgparser/smeup/MULANGT10BaseCodopTest.kt +++ b/rpgJavaInterpreter-core/src/test/kotlin/com/smeup/rpgparser/smeup/MULANGT10BaseCodopTest.kt @@ -397,6 +397,16 @@ open class MULANGT10BaseCodopTest : MULANGTTest() { assertEquals(expected, "smeup/MUDRNRAPU00268".outputOf(configuration = smeupConfig)) } + /** + * State of context after a CALL stack failed setting an error indicator + * @see #LS24004538 + */ + @Test + fun executeMUDRNRAPU00269() { + val expected = listOf("ok") + assertEquals(expected, "smeup/MUDRNRAPU00269".outputOf(configuration = smeupConfig)) + } + /** * Comparing number and `*ZEROS` by using `IFxx`. * @see #LS24004528 diff --git a/rpgJavaInterpreter-core/src/test/resources/smeup/ERRORPGM2.rpgle b/rpgJavaInterpreter-core/src/test/resources/smeup/ERRORPGM2.rpgle new file mode 100644 index 000000000..fb2b84d08 --- /dev/null +++ b/rpgJavaInterpreter-core/src/test/resources/smeup/ERRORPGM2.rpgle @@ -0,0 +1,2 @@ + * This program has the sole purpose of calling a program that throws a runtime exception + C CALL 'ERRORPGM' \ No newline at end of file diff --git a/rpgJavaInterpreter-core/src/test/resources/smeup/MUDRNRAPU00268.rpgle b/rpgJavaInterpreter-core/src/test/resources/smeup/MUDRNRAPU00268.rpgle index 1200cf176..4f7873ac4 100644 --- a/rpgJavaInterpreter-core/src/test/resources/smeup/MUDRNRAPU00268.rpgle +++ b/rpgJavaInterpreter-core/src/test/resources/smeup/MUDRNRAPU00268.rpgle @@ -22,7 +22,7 @@ D £TESTDS DS D £TESTPA 10 - * Call of a program that throws an error and setting the error indicator + * Call of a program that throws an error and sets the error indicator C CALL 'ERRORPGM' 35 * If call failed but the error indicator was set diff --git a/rpgJavaInterpreter-core/src/test/resources/smeup/MUDRNRAPU00269.rpgle b/rpgJavaInterpreter-core/src/test/resources/smeup/MUDRNRAPU00269.rpgle new file mode 100644 index 000000000..fa43d3cd5 --- /dev/null +++ b/rpgJavaInterpreter-core/src/test/resources/smeup/MUDRNRAPU00269.rpgle @@ -0,0 +1,38 @@ + V* ============================================================== + V* 30/10/2024 APU002 Creation + V* ============================================================== + O * PROGRAM GOAL + O * Call a program with an error indicator that calls another program + O * that throws an error and ensure the execution context is appropriate + V* ============================================================== + O * JARIKO ANOMALY + O * Before the fix, the error was about the caller not resolving + O * symbols in the correct compilation unit + O * + O * Note: see MUDRNRAPU00268.rpgle for a deeper explanation + O * on how the error was reproduced + V* ============================================================== + + * Test declarations + D £TESTDS DS + D £TESTPA 10 + + * Call of a program that throws an error and sets the error indicator + C CALL 'ERRORPGM2' 35 + + * If call failed but the error indicator was set + * Execution should continue and the context should reference the correct program + C IF P_Test(£TESTPA:'EmiMsg(':'')='Y' + C 'ok' DSPLY + C ENDIF + + * Test procedure + PP_Test B + D P_Test Pi 30000 VARYING + D $A 15 CONST + D $B 1N OPTIONS(*NOPASS) + D $C 1 OPTIONS(*NOPASS) + C RETURN 'Y' + P E + + C SETON LR From 6223bb1ced9d796fd03d2c87f4f7a93540782e58 Mon Sep 17 00:00:00 2001 From: domenico Date: Wed, 30 Oct 2024 16:48:13 +0100 Subject: [PATCH 2/5] Fix stack state in composite call cases --- .../com/smeup/rpgparser/parsing/ast/statements.kt | 15 ++++++++++----- .../main/kotlin/com/smeup/rpgparser/utils/misc.kt | 15 +++++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/parsing/ast/statements.kt b/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/parsing/ast/statements.kt index 2124d2a63..049d7de62 100644 --- a/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/parsing/ast/statements.kt +++ b/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/parsing/ast/statements.kt @@ -29,10 +29,7 @@ import com.smeup.rpgparser.parsing.parsetreetoast.acceptBody import com.smeup.rpgparser.parsing.parsetreetoast.error import com.smeup.rpgparser.parsing.parsetreetoast.isInt import com.smeup.rpgparser.parsing.parsetreetoast.toAst -import com.smeup.rpgparser.utils.ComparisonOperator -import com.smeup.rpgparser.utils.divideAtIndex -import com.smeup.rpgparser.utils.resizeTo -import com.smeup.rpgparser.utils.substringOfLength +import com.smeup.rpgparser.utils.* import com.strumenta.kolasu.model.* import kotlinx.serialization.SerialName import kotlinx.serialization.Serializable @@ -789,9 +786,17 @@ data class CallStmt( if (errorIndicator == null) { throw e } + interpreter.getIndicators()[errorIndicator] = BooleanValue.TRUE MainExecutionContext.getConfiguration().jarikoCallback.onCallPgmError.invoke(popRuntimeErrorEvent()) - MainExecutionContext.getProgramStack().pop() + + /** + * Restore the program stack state to what it was before this call. + * + * Note: MainExecutionContext.getProgramStack().pop() is not enough because entries of programs that + * quit with error indicators must be in the program stack until onCallPgmError is invoked. + */ + MainExecutionContext.getProgramStack().rollbackBefore(programToCall) null } paramValuesAtTheEnd?.forEachIndexed { index, value -> diff --git a/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/utils/misc.kt b/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/utils/misc.kt index dab61cb47..274cdfe8d 100644 --- a/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/utils/misc.kt +++ b/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/utils/misc.kt @@ -17,6 +17,7 @@ package com.smeup.rpgparser.utils import com.smeup.rpgparser.execution.ParsingProgram +import com.smeup.rpgparser.interpreter.RpgProgram import com.smeup.rpgparser.parsing.ast.CompilationUnit import com.strumenta.kolasu.model.Node import com.strumenta.kolasu.model.ancestor @@ -169,6 +170,20 @@ internal fun Stack.peekOrNull(): T? = if (isNotEmpty()) { this.peek() } else null +/** + * Rollback the stack state to the state it was prior of the specified program was called. + * @param name The name of the program + * @return `true` if the program was actually found in the stack and `false` else-wise + */ +internal fun Stack.rollbackBefore(name: String): Boolean { + while (true) { + val previous = this.popIfPresent() ?: break + if (previous.name == name) return true + } + + return false +} + /** * Get the [CompilationUnit] that contains the current [Node] */ From 2e71498f742db18ff53e5bd175d7cedb39409792 Mon Sep 17 00:00:00 2001 From: domenico Date: Wed, 6 Nov 2024 14:22:36 +0100 Subject: [PATCH 3/5] Remove rollbackBefore --- .../main/kotlin/com/smeup/rpgparser/utils/misc.kt | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/utils/misc.kt b/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/utils/misc.kt index 274cdfe8d..dab61cb47 100644 --- a/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/utils/misc.kt +++ b/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/utils/misc.kt @@ -17,7 +17,6 @@ package com.smeup.rpgparser.utils import com.smeup.rpgparser.execution.ParsingProgram -import com.smeup.rpgparser.interpreter.RpgProgram import com.smeup.rpgparser.parsing.ast.CompilationUnit import com.strumenta.kolasu.model.Node import com.strumenta.kolasu.model.ancestor @@ -170,20 +169,6 @@ internal fun Stack.peekOrNull(): T? = if (isNotEmpty()) { this.peek() } else null -/** - * Rollback the stack state to the state it was prior of the specified program was called. - * @param name The name of the program - * @return `true` if the program was actually found in the stack and `false` else-wise - */ -internal fun Stack.rollbackBefore(name: String): Boolean { - while (true) { - val previous = this.popIfPresent() ?: break - if (previous.name == name) return true - } - - return false -} - /** * Get the [CompilationUnit] that contains the current [Node] */ From 6a8f3aac6924204ab090866973e4e21cb8385934 Mon Sep 17 00:00:00 2001 From: domenico Date: Wed, 6 Nov 2024 14:23:03 +0100 Subject: [PATCH 4/5] Delegate program stack managing logic to callers --- .../rpgparser/interpreter/ProgramInterpreter.kt | 9 ++++++++- .../com/smeup/rpgparser/interpreter/program.kt | 6 ++---- .../smeup/rpgparser/parsing/ast/statements.kt | 17 +++++++++-------- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/interpreter/ProgramInterpreter.kt b/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/interpreter/ProgramInterpreter.kt index b84efdde3..dab3581e2 100644 --- a/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/interpreter/ProgramInterpreter.kt +++ b/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/interpreter/ProgramInterpreter.kt @@ -1,8 +1,15 @@ package com.smeup.rpgparser.interpreter +import com.smeup.rpgparser.execution.MainExecutionContext + class ProgramInterpreter(val systemInterface: SystemInterface) { fun execute(rpgProgram: RpgProgram, initialValues: LinkedHashMap) { - rpgProgram.execute(systemInterface = systemInterface, params = initialValues) + MainExecutionContext.getProgramStack().push(rpgProgram) + try { + rpgProgram.execute(systemInterface = systemInterface, params = initialValues) + } finally { + MainExecutionContext.getProgramStack().pop() + } } } diff --git a/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/interpreter/program.kt b/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/interpreter/program.kt index fb213ad78..3df7d4002 100644 --- a/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/interpreter/program.kt +++ b/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/interpreter/program.kt @@ -128,8 +128,8 @@ class RpgProgram(val cu: CompilationUnit, val name: String = " 1) { + MainExecutionContext.getProgramStack()[MainExecutionContext.getProgramStack().size - 2] } else { null } @@ -149,7 +149,6 @@ class RpgProgram(val cu: CompilationUnit, val name: String = " @@ -808,6 +806,9 @@ data class CallStmt( currentParam.result?.let { interpreter.assign(it, value) } } } + + if (program is RpgProgram) + MainExecutionContext.getProgramStack().pop() } override fun getStatementLogRenderer(source: LogSourceProvider, action: String): LazyLogEntry { From 77f38551c942625ce4e01473eaab3e748a77c137 Mon Sep 17 00:00:00 2001 From: domenico Date: Wed, 6 Nov 2024 15:35:09 +0100 Subject: [PATCH 5/5] Add documentation for ambiguous logic in RpgProgram.execute --- .../com/smeup/rpgparser/interpreter/program.kt | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/interpreter/program.kt b/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/interpreter/program.kt index 3df7d4002..079cc62e2 100644 --- a/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/interpreter/program.kt +++ b/rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/interpreter/program.kt @@ -128,8 +128,20 @@ class RpgProgram(val cu: CompilationUnit, val name: String = " 1) { - MainExecutionContext.getProgramStack()[MainExecutionContext.getProgramStack().size - 2] + + /** + * As the RPG program stack is managed outside of this method, it is up to the caller of this method + * to ensure it is in the correct state, that is: + * - `lastIndex` is this RpgProgram + * - `lastIndex - 1` is the RpgProgram that calls this RpgProgram + * + * Note: If these two rules are not followed at this point, do not expect RpgPrograms to behave correctly. + * that means something is wrong with `MainExecutionContext.getProgramStack()` push and pop logic. + */ + val programStack = MainExecutionContext.getProgramStack() + val caller = if (programStack.size > 1) { + val parentProgramIndex = programStack.lastIndex - 1 + programStack[parentProgramIndex] } else { null }