From 54bdc1e1f6a7fb85a5b193c9b8ecf0dcf06949e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20J=2E=20Est=C3=A9banez?= Date: Wed, 17 Oct 2018 20:36:47 +0200 Subject: [PATCH] Fix crash on signal/resume to dangling target Fixes #22443. --- modules/gdscript/gdscript_compiler.cpp | 20 +++++------ modules/gdscript/gdscript_compiler.h | 10 +++--- modules/gdscript/gdscript_function.cpp | 49 ++++++++++++++------------ modules/gdscript/gdscript_function.h | 10 +++--- 4 files changed, 46 insertions(+), 43 deletions(-) diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp index 310c4e21f25..45319c59e73 100644 --- a/modules/gdscript/gdscript_compiler.cpp +++ b/modules/gdscript/gdscript_compiler.cpp @@ -1603,13 +1603,13 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Blo return OK; } -Error GDScriptCompiler::_parse_function(GDScript *p_script, const GDScriptParser::ClassNode *p_class, const GDScriptParser::FunctionNode *p_func, bool p_for_ready) { +Error GDScriptCompiler::_parse_function(Ref p_script, const GDScriptParser::ClassNode *p_class, const GDScriptParser::FunctionNode *p_func, bool p_for_ready) { Vector bytecode; CodeGen codegen; codegen.class_node = p_class; - codegen.script = p_script; + codegen.script = p_script.ptr(); codegen.function_node = p_func; codegen.stack_max = 0; codegen.current_line = 0; @@ -1853,7 +1853,7 @@ Error GDScriptCompiler::_parse_function(GDScript *p_script, const GDScriptParser return OK; } -Error GDScriptCompiler::_parse_class_level(GDScript *p_script, GDScript *p_owner, const GDScriptParser::ClassNode *p_class, bool p_keep_state) { +Error GDScriptCompiler::_parse_class_level(Ref p_script, Ref p_owner, const GDScriptParser::ClassNode *p_class, bool p_keep_state) { if (p_class->owner && p_class->owner->owner) { // Owner is not root @@ -1887,7 +1887,7 @@ Error GDScriptCompiler::_parse_class_level(GDScript *p_script, GDScript *p_owner p_script->initializer = NULL; p_script->subclasses.clear(); - p_script->_owner = p_owner; + p_script->_owner = p_owner.ptr(); p_script->tool = p_class->tool; p_script->name = p_class->name; @@ -1994,7 +1994,7 @@ Error GDScriptCompiler::_parse_class_level(GDScript *p_script, GDScript *p_owner StringName name = p_class->_signals[i].name; - GDScript *c = p_script; + GDScript *c = p_script.ptr(); while (c) { @@ -2054,7 +2054,7 @@ Error GDScriptCompiler::_parse_class_level(GDScript *p_script, GDScript *p_owner return OK; } -Error GDScriptCompiler::_parse_class_blocks(GDScript *p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state) { +Error GDScriptCompiler::_parse_class_blocks(Ref p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state) { //parse methods bool has_initializer = false; @@ -2159,7 +2159,7 @@ Error GDScriptCompiler::_parse_class_blocks(GDScript *p_script, const GDScriptPa return OK; } -void GDScriptCompiler::_make_scripts(const GDScript *p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state) { +void GDScriptCompiler::_make_scripts(const Ref p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state) { Map > old_subclasses; @@ -2178,20 +2178,20 @@ void GDScriptCompiler::_make_scripts(const GDScript *p_script, const GDScriptPar subclass.instance(); } - subclass->_owner = const_cast(p_script); + subclass->_owner = const_cast(p_script.ptr()); class_map.insert(name, subclass); _make_scripts(subclass.ptr(), p_class->subclasses[i], p_keep_state); } } -Error GDScriptCompiler::compile(const GDScriptParser *p_parser, GDScript *p_script, bool p_keep_state) { +Error GDScriptCompiler::compile(const GDScriptParser *p_parser, Ref p_script, bool p_keep_state) { err_line = -1; err_column = -1; error = ""; parser = p_parser; - main_script = p_script; + main_script = p_script.ptr(); const GDScriptParser::Node *root = parser->get_parse_tree(); ERR_FAIL_COND_V(root->type != GDScriptParser::Node::TYPE_CLASS, ERR_INVALID_DATA); diff --git a/modules/gdscript/gdscript_compiler.h b/modules/gdscript/gdscript_compiler.h index 55f5e2b48eb..23c6450aa7a 100644 --- a/modules/gdscript/gdscript_compiler.h +++ b/modules/gdscript/gdscript_compiler.h @@ -148,17 +148,17 @@ class GDScriptCompiler { int _parse_assign_right_expression(CodeGen &codegen, const GDScriptParser::OperatorNode *p_expression, int p_stack_level); int _parse_expression(CodeGen &codegen, const GDScriptParser::Node *p_expression, int p_stack_level, bool p_root = false, bool p_initializer = false); Error _parse_block(CodeGen &codegen, const GDScriptParser::BlockNode *p_block, int p_stack_level = 0, int p_break_addr = -1, int p_continue_addr = -1); - Error _parse_function(GDScript *p_script, const GDScriptParser::ClassNode *p_class, const GDScriptParser::FunctionNode *p_func, bool p_for_ready = false); - Error _parse_class_level(GDScript *p_script, GDScript *p_owner, const GDScriptParser::ClassNode *p_class, bool p_keep_state); - Error _parse_class_blocks(GDScript *p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state); - void _make_scripts(const GDScript *p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state); + Error _parse_function(Ref p_script, const GDScriptParser::ClassNode *p_class, const GDScriptParser::FunctionNode *p_func, bool p_for_ready = false); + Error _parse_class_level(Ref p_script, Ref p_owner, const GDScriptParser::ClassNode *p_class, bool p_keep_state); + Error _parse_class_blocks(Ref p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state); + void _make_scripts(const Ref p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state); int err_line; int err_column; StringName source; String error; public: - Error compile(const GDScriptParser *p_parser, GDScript *p_script, bool p_keep_state = false); + Error compile(const GDScriptParser *p_parser, Ref p_script, bool p_keep_state = false); String get_error() const; int get_error_line() const; diff --git a/modules/gdscript/gdscript_function.cpp b/modules/gdscript/gdscript_function.cpp index f09ff224e89..31115a4bd90 100644 --- a/modules/gdscript/gdscript_function.cpp +++ b/modules/gdscript/gdscript_function.cpp @@ -275,7 +275,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a #endif uint32_t alloca_size = 0; - GDScript *_class; + GDScript *script; int ip = 0; int line = _initial_line; @@ -286,7 +286,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a line = p_state->line; ip = p_state->ip; alloca_size = p_state->stack.size(); - _class = p_state->_class; + script = p_state->script.ptr(); p_instance = p_state->instance; defarg = p_state->defarg; self = p_state->self; @@ -368,9 +368,9 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a } else { self = p_instance->owner; } - _class = p_instance->script.ptr(); + script = p_instance->script.ptr(); } else { - _class = _script; + script = this->_script.ptr(); } } @@ -395,7 +395,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a #define GET_VARIANT_PTR(m_v, m_code_ofs) \ Variant *m_v; \ - m_v = _get_variant(_code_ptr[ip + m_code_ofs], p_instance, _class, self, stack, err_text); \ + m_v = _get_variant(_code_ptr[ip + m_code_ofs], p_instance, script, self, stack, err_text); \ if (unlikely(!m_v)) \ OPCODE_BREAK; @@ -404,7 +404,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a #define CHECK_SPACE(m_space) #define GET_VARIANT_PTR(m_v, m_code_ofs) \ Variant *m_v; \ - m_v = _get_variant(_code_ptr[ip + m_code_ofs], p_instance, _class, self, stack, err_text); + m_v = _get_variant(_code_ptr[ip + m_code_ofs], p_instance, script, self, stack, err_text); #endif @@ -1185,7 +1185,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a GET_VARIANT_PTR(dst, argc + 3); - const GDScript *gds = _script; + const GDScript *gds = _script.ptr(); const Map::Element *E = NULL; while (gds->base.ptr()) { @@ -1256,11 +1256,10 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a gdfs->state.stack_size = _stack_size; gdfs->state.self = self; gdfs->state.alloca_size = alloca_size; - gdfs->state._class = _class; + gdfs->state.script = _script; gdfs->state.ip = ip + ipofs; gdfs->state.line = line; gdfs->state.instance_id = (p_instance && p_instance->get_owner()) ? p_instance->get_owner()->get_instance_id() : 0; - gdfs->state.script_id = _class->get_instance_id(); //gdfs->state.result_pos=ip+ipofs-1; gdfs->state.defarg = defarg; gdfs->state.instance = p_instance; @@ -1549,8 +1548,8 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a String err_file; if (p_instance) err_file = p_instance->script->path; - else if (_class) - err_file = _class->path; + else if (script) + err_file = script->path; if (err_file == "") err_file = ""; String err_func = name; @@ -1765,14 +1764,19 @@ GDScriptFunction::~GDScriptFunction() { Variant GDScriptFunctionState::_signal_callback(const Variant **p_args, int p_argcount, Variant::CallError &r_error) { #ifdef DEBUG_ENABLED + // Checking this first since it's faster + if (!state.script.is_valid()) { + ERR_EXPLAIN("Resumed after yield, but script is gone"); + ERR_FAIL_V(Variant()); + } + if (state.instance_id && !ObjectDB::get_instance(state.instance_id)) { ERR_EXPLAIN("Resumed after yield, but class instance is gone"); ERR_FAIL_V(Variant()); } - - if (state.script_id && !ObjectDB::get_instance(state.script_id)) { - ERR_EXPLAIN("Resumed after yield, but script is gone"); - ERR_FAIL_V(Variant()); +#else + if (!is_valid()) { + return Variant(); } #endif @@ -1841,12 +1845,12 @@ bool GDScriptFunctionState::is_valid(bool p_extended_check) const { return false; if (p_extended_check) { + //script gone? (checking this first since it's faster) + if (!state.script.is_valid()) + return false; //class instance gone? if (state.instance_id && !ObjectDB::get_instance(state.instance_id)) return false; - //script gone? - if (state.script_id && !ObjectDB::get_instance(state.script_id)) - return false; } return true; @@ -1856,13 +1860,14 @@ Variant GDScriptFunctionState::resume(const Variant &p_arg) { ERR_FAIL_COND_V(!function, Variant()); #ifdef DEBUG_ENABLED - if (state.instance_id && !ObjectDB::get_instance(state.instance_id)) { - ERR_EXPLAIN("Resumed after yield, but class instance is gone"); + // Checking this first since it's faster + if (!state.script.is_valid()) { + ERR_EXPLAIN("Resumed after yield, but script is gone"); ERR_FAIL_V(Variant()); } - if (state.script_id && !ObjectDB::get_instance(state.script_id)) { - ERR_EXPLAIN("Resumed after yield, but script is gone"); + if (state.instance_id && !ObjectDB::get_instance(state.instance_id)) { + ERR_EXPLAIN("Resumed after yield, but class instance is gone"); ERR_FAIL_V(Variant()); } #endif diff --git a/modules/gdscript/gdscript_function.h b/modules/gdscript/gdscript_function.h index bfb6d673f16..a47070de4f8 100644 --- a/modules/gdscript/gdscript_function.h +++ b/modules/gdscript/gdscript_function.h @@ -225,7 +225,7 @@ private: bool _static; MultiplayerAPI::RPCMode rpc_mode; - GDScript *_script; + Ref _script; StringName name; Vector constants; @@ -272,15 +272,13 @@ private: public: struct CallState { - ObjectID instance_id; //by debug only - ObjectID script_id; - + ObjectID instance_id; GDScriptInstance *instance; Vector stack; int stack_size; Variant self; uint32_t alloca_size; - GDScript *_class; + Ref script; int ip; int line; int defarg; @@ -299,7 +297,7 @@ public: int get_default_argument_addr(int p_idx) const; GDScriptDataType get_return_type() const; GDScriptDataType get_argument_type(int p_idx) const; - GDScript *get_script() const { return _script; } + GDScript *get_script() const { return const_cast(_script.ptr()); } StringName get_source() const { return source; } void debug_get_stack_member_state(int p_line, List > *r_stackvars) const;