compiler: fix for/break miscompilation

When patching jump targets for break statments while compiling for-loop
statments, jump beyond the instructions popping intermediate loop variables
off the stack to fix a stack position mismatch between compiler and vm.

Before that change, local loop body variables got popped twice, breaking
the expected stack layout.

Signed-off-by: Jo-Philipp Wich <jo@mein.io>

2 files changed, 43 insertions, 5 deletions

diff --git a/compiler.c b/compiler.c
index 1b0d1c4..ba0b699 100644
--- a/compiler.c
+++ b/compiler.c
@@ -2128,9 +2128,6 @@ uc_compiler_compile_for_in(uc_compiler *compiler, bool local, uc_token *kvar, uc
 	/* back patch conditional jump */
 	uc_compiler_set_jmpaddr(compiler, test_jmp, chunk->count);
 
-	/* patch up break/continue */
-	uc_compiler_backpatch(compiler, chunk->count, skip_jmp + 5);
-
 	/* pop loop variables */
 	uc_compiler_emit_insn(compiler, 0, I_POP);
 
@@ -2138,6 +2135,9 @@ uc_compiler_compile_for_in(uc_compiler *compiler, bool local, uc_token *kvar, uc
 		uc_compiler_emit_insn(compiler, 0, I_POP);
 
 	uc_compiler_leave_scope(compiler);
+
+	/* patch up break/continue */
+	uc_compiler_backpatch(compiler, chunk->count, skip_jmp + 5);
 }
 
 static void
@@ -2232,10 +2232,10 @@ uc_compiler_compile_for_count(uc_compiler *compiler, bool local, uc_token *var)
 	if (test_off)
 		uc_compiler_set_jmpaddr(compiler, test_off, chunk->count);
 
+	uc_compiler_leave_scope(compiler);
+
 	/* patch up break/continue */
 	uc_compiler_backpatch(compiler, chunk->count, incr_off);
-
-	uc_compiler_leave_scope(compiler);
 }
 
 static void
diff --git a/tests/03_bugs/10_break_stack_mismatch b/tests/03_bugs/10_break_stack_mismatch
new file mode 100644
index 0000000..ae16dac
--- /dev/null
+++ b/tests/03_bugs/10_break_stack_mismatch
@@ -0,0 +1,38 @@
+When emitting jump instructions for breaking out of for-loops, the compiler
+incorrectly set the jump target before the pop instruction clearing the
+intermediate loop variables. Since the break instruction itself already
+compiles to a series of pop instructions reverting the stack to it's the
+pre-loop state, intermediate values got popped twice, leading to a stack
+layout mismatch between compiler and VM, resulting in wrong local variable
+values or segmentation faults at runtime.
+
+-- Testcase --
+{%
+	let x = 1;
+
+	for (let y in [2])
+		break;
+
+	print(x, "\n");
+%}
+-- End --
+
+-- Expect stdout --
+1
+-- End --
+
+
+-- Testcase --
+{%
+	let x = 1;
+
+	for (let y = 0; y < 1; y++)
+		break;
+
+	print(x, "\n");
+%}
+-- End --
+
+-- Expect stdout --
+1
+-- End --