From 10a1f649680971c63b110d33cffd9aa5b1aaf6a9 Mon Sep 17 00:00:00 2001 From: George Marques Date: Mon, 17 May 2021 10:59:43 -0300 Subject: [PATCH] GDScript: Fix crash caused by uninitialized temp stack slots This adds initialization to every typed temporary stack slot at the beginning of the function call instead of emitting instructions, since those might be in a conditional branch and not be called. --- modules/gdscript/gdscript_byte_codegen.cpp | 12 +++---- modules/gdscript/gdscript_function.h | 2 ++ modules/gdscript/gdscript_vm.cpp | 42 ++++++++++++++++++++++ 3 files changed, 49 insertions(+), 7 deletions(-) diff --git a/modules/gdscript/gdscript_byte_codegen.cpp b/modules/gdscript/gdscript_byte_codegen.cpp index 0da99ccee3..23eb5fb29f 100644 --- a/modules/gdscript/gdscript_byte_codegen.cpp +++ b/modules/gdscript/gdscript_byte_codegen.cpp @@ -129,12 +129,6 @@ uint32_t GDScriptByteCodeGenerator::add_temporary(const GDScriptDataType &p_type int idx = temporaries.size(); pool.push_back(idx); temporaries.push_back(new_temp); - - // First time using this, so adjust to the proper type. - if (temp_type != Variant::NIL) { - Address addr(Address::TEMPORARY, idx, p_type); - write_type_adjust(addr, temp_type); - } } int slot = pool.front()->get(); pool.pop_front(); @@ -189,8 +183,12 @@ GDScriptFunction *GDScriptByteCodeGenerator::write_end() { append(GDScriptFunction::OPCODE_END, 0); for (int i = 0; i < temporaries.size(); i++) { + int stack_index = i + max_locals + RESERVED_STACK; for (int j = 0; j < temporaries[i].bytecode_indices.size(); j++) { - opcodes.write[temporaries[i].bytecode_indices[j]] = (i + max_locals + RESERVED_STACK) | (GDScriptFunction::ADDR_TYPE_STACK << GDScriptFunction::ADDR_BITS); + opcodes.write[temporaries[i].bytecode_indices[j]] = stack_index | (GDScriptFunction::ADDR_TYPE_STACK << GDScriptFunction::ADDR_BITS); + } + if (temporaries[i].type != Variant::NIL) { + function->temporary_slots[stack_index] = temporaries[i].type; } } diff --git a/modules/gdscript/gdscript_function.h b/modules/gdscript/gdscript_function.h index 70b62ced6d..b39ed9cf3a 100644 --- a/modules/gdscript/gdscript_function.h +++ b/modules/gdscript/gdscript_function.h @@ -496,6 +496,8 @@ private: Vector argument_types; GDScriptDataType return_type; + Map temporary_slots; + #ifdef TOOLS_ENABLED Vector arg_names; Vector default_arg_values; diff --git a/modules/gdscript/gdscript_vm.cpp b/modules/gdscript/gdscript_vm.cpp index 4757ec6ca9..9c1bc8217f 100644 --- a/modules/gdscript/gdscript_vm.cpp +++ b/modules/gdscript/gdscript_vm.cpp @@ -152,6 +152,44 @@ String GDScriptFunction::_get_call_error(const Callable::CallError &p_err, const return err_text; } +void (*type_init_function_table[])(Variant *) = { + nullptr, // NIL (shouldn't be called). + &VariantInitializer::init, // BOOL. + &VariantInitializer::init, // INT. + &VariantInitializer::init, // FLOAT. + &VariantInitializer::init, // STRING. + &VariantInitializer::init, // VECTOR2. + &VariantInitializer::init, // VECTOR2I. + &VariantInitializer::init, // RECT2. + &VariantInitializer::init, // RECT2I. + &VariantInitializer::init, // VECTOR3. + &VariantInitializer::init, // VECTOR3I. + &VariantInitializer::init, // TRANSFORM2D. + &VariantInitializer::init, // PLANE. + &VariantInitializer::init, // QUAT. + &VariantInitializer::init, // AABB. + &VariantInitializer::init, // BASIS. + &VariantInitializer::init, // TRANSFORM. + &VariantInitializer::init, // COLOR. + &VariantInitializer::init, // STRING_NAME. + &VariantInitializer::init, // NODE_PATH. + &VariantInitializer::init, // RID. + &VariantTypeAdjust::adjust, // OBJECT. + &VariantInitializer::init, // CALLABLE. + &VariantInitializer::init, // SIGNAL. + &VariantInitializer::init, // DICTIONARY. + &VariantInitializer::init, // ARRAY. + &VariantInitializer::init, // PACKED_BYTE_ARRAY. + &VariantInitializer::init, // PACKED_INT32_ARRAY. + &VariantInitializer::init, // PACKED_INT64_ARRAY. + &VariantInitializer::init, // PACKED_FLOAT32_ARRAY. + &VariantInitializer::init, // PACKED_FLOAT64_ARRAY. + &VariantInitializer::init, // PACKED_STRING_ARRAY. + &VariantInitializer::init, // PACKED_VECTOR2_ARRAY. + &VariantInitializer::init, // PACKED_VECTOR3_ARRAY. + &VariantInitializer::init, // PACKED_COLOR_ARRAY. +}; + #if defined(__GNUC__) #define OPCODES_TABLE \ static const void *switch_table_ops[] = { \ @@ -491,6 +529,10 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a memnew_placement(&stack[ADDR_STACK_CLASS], Variant(script)); + for (const Map::Element *E = temporary_slots.front(); E; E = E->next()) { + type_init_function_table[E->get()](&stack[E->key()]); + } + String err_text; #ifdef DEBUG_ENABLED