From 2e528ef38298544d040dddce1aa3110c651eaf6b Mon Sep 17 00:00:00 2001
From: George Marques <george@gmarqu.es>
Date: Sun, 22 Nov 2020 12:24:40 -0300
Subject: GDScript: Fix mishandling of stack pointers

- Replace the for loop temporaries by locals. They cause conflicts with
  the stack when being popped, while locals are properly handled in the
  scope.
- Change the interface for the codegen so the for loop list doesn't live
  through the whole block if it's a temporary.
- Keep track of the actual amount of local variables in the stack. Using
  the size of the map is misleading in cases where multiple locals have
  the same name (which is allowed when there's no shadowing).
- Added a few debug checks for temporaries, to avoid them being wrongly
  manipulated in the future. They should not live more than a line of
  code.
- Rearrange some of compiler code to make sure the temporaries don't
  live across blocks.
---
 modules/gdscript/gdscript_byte_codegen.h | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

(limited to 'modules/gdscript/gdscript_byte_codegen.h')

diff --git a/modules/gdscript/gdscript_byte_codegen.h b/modules/gdscript/gdscript_byte_codegen.h
index a546920fb2..0173b7f820 100644
--- a/modules/gdscript/gdscript_byte_codegen.h
+++ b/modules/gdscript/gdscript_byte_codegen.h
@@ -43,6 +43,7 @@ class GDScriptByteCodeGenerator : public GDScriptCodeGenerator {
 	Vector<int> opcodes;
 	List<Map<StringName, int>> stack_id_stack;
 	Map<StringName, int> stack_identifiers;
+	List<int> stack_identifiers_counts;
 	Map<StringName, int> local_constants;
 
 	List<GDScriptFunction::StackDebug> stack_debug;
@@ -51,11 +52,16 @@ class GDScriptByteCodeGenerator : public GDScriptCodeGenerator {
 
 	int current_stack_size = 0;
 	int current_temporaries = 0;
+	int current_locals = 0;
 	int current_line = 0;
 	int stack_max = 0;
 	int instr_args_max = 0;
 	int ptrcall_max = 0;
 
+#ifdef DEBUG_ENABLED
+	List<int> temp_stack;
+#endif
+
 	HashMap<Variant, int, VariantHasher, VariantComparator> constant_map;
 	Map<StringName, int> name_map;
 #ifdef TOOLS_ENABLED
@@ -72,8 +78,12 @@ class GDScriptByteCodeGenerator : public GDScriptCodeGenerator {
 	Map<Variant::ValidatedConstructor, int> constructors_map;
 	Map<MethodBind *, int> method_bind_map;
 
-	List<int> if_jmp_addrs; // List since this can be nested.
+	// Lists since these can be nested.
+	List<int> if_jmp_addrs;
 	List<int> for_jmp_addrs;
+	List<Address> for_iterator_variables;
+	List<Address> for_counter_variables;
+	List<Address> for_container_variables;
 	List<int> while_jmp_addrs;
 	List<int> continue_addrs;
 
@@ -89,6 +99,7 @@ class GDScriptByteCodeGenerator : public GDScriptCodeGenerator {
 	List<List<int>> match_continues_to_patch;
 
 	void add_stack_identifier(const StringName &p_id, int p_stackpos) {
+		current_locals++;
 		stack_identifiers[p_id] = p_stackpos;
 		if (debug_stack) {
 			block_identifiers[p_id] = p_stackpos;
@@ -102,17 +113,26 @@ class GDScriptByteCodeGenerator : public GDScriptCodeGenerator {
 	}
 
 	void push_stack_identifiers() {
+		stack_identifiers_counts.push_back(current_locals);
 		stack_id_stack.push_back(stack_identifiers);
 		if (debug_stack) {
-			block_identifier_stack.push_back(block_identifiers);
+			Map<StringName, int> block_ids(block_identifiers);
+			block_identifier_stack.push_back(block_ids);
 			block_identifiers.clear();
 		}
 	}
 
 	void pop_stack_identifiers() {
+		current_locals = stack_identifiers_counts.back()->get();
+		stack_identifiers_counts.pop_back();
 		stack_identifiers = stack_id_stack.back()->get();
-		current_stack_size = stack_identifiers.size() + current_temporaries;
 		stack_id_stack.pop_back();
+#ifdef DEBUG_ENABLED
+		if (current_temporaries != 0) {
+			ERR_PRINT("Leaving block with non-zero temporary variables: " + itos(current_temporaries));
+		}
+#endif
+		current_stack_size = current_locals;
 
 		if (debug_stack) {
 			for (Map<StringName, int>::Element *E = block_identifiers.front(); E; E = E->next()) {
@@ -397,7 +417,9 @@ public:
 	virtual void write_if(const Address &p_condition) override;
 	virtual void write_else() override;
 	virtual void write_endif() override;
-	virtual void write_for(const Address &p_variable, const Address &p_list) override;
+	virtual void start_for(const GDScriptDataType &p_iterator_type, const GDScriptDataType &p_list_type) override;
+	virtual void write_for_assignment(const Address &p_variable, const Address &p_list) override;
+	virtual void write_for() override;
 	virtual void write_endfor() override;
 	virtual void start_while_condition() override;
 	virtual void write_while(const Address &p_condition) override;
-- 
cgit v1.2.3