Skip to content

Commit 24344de

Browse files
YuriSizovYuriSizov
authored
Merge pull request #73448 from RandomShaper/fix_sticky_stack
Avoid GDScript bookkeeping from referencing objects longer than necessary
2 parents 6bb4b00 + 5e9400f commit 24344de

2 files changed

Lines changed: 101 additions & 31 deletions

File tree

‎modules/gdscript/gdscript_byte_codegen.cpp‎

Lines changed: 73 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,11 @@ void GDScriptByteCodeGenerator::pop_temporary() {
143143
ERR_FAIL_COND(used_temporaries.is_empty());
144144
int slot_idx = used_temporaries.back()->get();
145145
const StackSlot &slot = temporaries[slot_idx];
146+
if (slot.type == Variant::OBJECT) {
147+
// Avoid keeping in the stack long-lived references to objects,
148+
// which may prevent RefCounted objects from being freed.
149+
write_assign_false(Address(Address::TEMPORARY, slot_idx));
150+
}
146151
temporaries_pool[slot.type].push_back(slot_idx);
147152
used_temporaries.pop_back();
148153
}
@@ -954,7 +959,7 @@ void GDScriptByteCodeGenerator::write_cast(const Address &p_target, const Addres
954959
append(index);
955960
}
956961

957-
GDScriptCodeGenerator::Address GDScriptByteCodeGenerator::get_call_target(const GDScriptCodeGenerator::Address &p_target, Variant::Type p_type) {
962+
GDScriptByteCodeGenerator::CallTarget GDScriptByteCodeGenerator::get_call_target(const GDScriptCodeGenerator::Address &p_target, Variant::Type p_type) {
958963
if (p_target.mode == Address::NIL) {
959964
GDScriptDataType type;
960965
if (p_type != Variant::NIL) {
@@ -963,10 +968,9 @@ GDScriptCodeGenerator::Address GDScriptByteCodeGenerator::get_call_target(const
963968
type.builtin_type = p_type;
964969
}
965970
uint32_t addr = add_temporary(type);
966-
pop_temporary();
967-
return Address(Address::TEMPORARY, addr, type);
971+
return CallTarget(Address(Address::TEMPORARY, addr, type), true, this);
968972
} else {
969-
return p_target;
973+
return CallTarget(p_target, false, this);
970974
}
971975
}
972976

@@ -976,19 +980,23 @@ void GDScriptByteCodeGenerator::write_call(const Address &p_target, const Addres
976980
append(p_arguments[i]);
977981
}
978982
append(p_base);
979-
append(get_call_target(p_target));
983+
CallTarget ct = get_call_target(p_target);
984+
append(ct.target);
980985
append(p_arguments.size());
981986
append(p_function_name);
987+
ct.cleanup();
982988
}
983989

984990
void GDScriptByteCodeGenerator::write_super_call(const Address &p_target, const StringName &p_function_name, const Vector<Address> &p_arguments) {
985991
append_opcode_and_argcount(GDScriptFunction::OPCODE_CALL_SELF_BASE, 1 + p_arguments.size());
986992
for (int i = 0; i < p_arguments.size(); i++) {
987993
append(p_arguments[i]);
988994
}
989-
append(get_call_target(p_target));
995+
CallTarget ct = get_call_target(p_target);
996+
append(ct.target);
990997
append(p_arguments.size());
991998
append(p_function_name);
999+
ct.cleanup();
9921000
}
9931001

9941002
void GDScriptByteCodeGenerator::write_call_async(const Address &p_target, const Address &p_base, const StringName &p_function_name, const Vector<Address> &p_arguments) {
@@ -997,9 +1005,11 @@ void GDScriptByteCodeGenerator::write_call_async(const Address &p_target, const
9971005
append(p_arguments[i]);
9981006
}
9991007
append(p_base);
1000-
append(get_call_target(p_target));
1008+
CallTarget ct = get_call_target(p_target);
1009+
append(ct.target);
10011010
append(p_arguments.size());
10021011
append(p_function_name);
1012+
ct.cleanup();
10031013
}
10041014

10051015
void GDScriptByteCodeGenerator::write_call_gdscript_utility(const Address &p_target, const StringName &p_function, const Vector<Address> &p_arguments) {
@@ -1008,9 +1018,11 @@ void GDScriptByteCodeGenerator::write_call_gdscript_utility(const Address &p_tar
10081018
for (int i = 0; i < p_arguments.size(); i++) {
10091019
append(p_arguments[i]);
10101020
}
1011-
append(get_call_target(p_target));
1021+
CallTarget ct = get_call_target(p_target);
1022+
append(ct.target);
10121023
append(p_arguments.size());
10131024
append(gds_function);
1025+
ct.cleanup();
10141026
#ifdef DEBUG_ENABLED
10151027
add_debug_name(gds_utilities_names, get_gds_utility_pos(gds_function), p_function);
10161028
#endif
@@ -1034,18 +1046,19 @@ void GDScriptByteCodeGenerator::write_call_utility(const Address &p_target, cons
10341046

10351047
if (is_validated) {
10361048
Variant::Type result_type = Variant::has_utility_function_return_value(p_function) ? Variant::get_utility_function_return_type(p_function) : Variant::NIL;
1037-
Address target = get_call_target(p_target, result_type);
1038-
Variant::Type temp_type = temporaries[target.address].type;
1049+
CallTarget ct = get_call_target(p_target, result_type);
1050+
Variant::Type temp_type = temporaries[ct.target.address].type;
10391051
if (result_type != temp_type) {
1040-
write_type_adjust(target, result_type);
1052+
write_type_adjust(ct.target, result_type);
10411053
}
10421054
append_opcode_and_argcount(GDScriptFunction::OPCODE_CALL_UTILITY_VALIDATED, 1 + p_arguments.size());
10431055
for (int i = 0; i < p_arguments.size(); i++) {
10441056
append(p_arguments[i]);
10451057
}
1046-
append(target);
1058+
append(ct.target);
10471059
append(p_arguments.size());
10481060
append(Variant::get_validated_utility_function(p_function));
1061+
ct.cleanup();
10491062
#ifdef DEBUG_ENABLED
10501063
add_debug_name(utilities_names, get_utility_pos(Variant::get_validated_utility_function(p_function)), p_function);
10511064
#endif
@@ -1054,9 +1067,11 @@ void GDScriptByteCodeGenerator::write_call_utility(const Address &p_target, cons
10541067
for (int i = 0; i < p_arguments.size(); i++) {
10551068
append(p_arguments[i]);
10561069
}
1057-
append(get_call_target(p_target));
1070+
CallTarget ct = get_call_target(p_target);
1071+
append(ct.target);
10581072
append(p_arguments.size());
10591073
append(p_function);
1074+
ct.cleanup();
10601075
}
10611076
}
10621077

@@ -1085,21 +1100,23 @@ void GDScriptByteCodeGenerator::write_call_builtin_type(const Address &p_target,
10851100
for (int i = 0; i < p_arguments.size(); i++) {
10861101
append(p_arguments[i]);
10871102
}
1088-
append(get_call_target(p_target));
1103+
CallTarget ct = get_call_target(p_target);
1104+
append(ct.target);
10891105
append(p_type);
10901106
append(p_method);
10911107
append(p_arguments.size());
1108+
ct.cleanup();
10921109
} else {
10931110
write_call(p_target, p_base, p_method, p_arguments);
10941111
}
10951112
return;
10961113
}
10971114

10981115
Variant::Type result_type = Variant::get_builtin_method_return_type(p_type, p_method);
1099-
Address target = get_call_target(p_target, result_type);
1100-
Variant::Type temp_type = temporaries[target.address].type;
1116+
CallTarget ct = get_call_target(p_target, result_type);
1117+
Variant::Type temp_type = temporaries[ct.target.address].type;
11011118
if (result_type != temp_type) {
1102-
write_type_adjust(target, result_type);
1119+
write_type_adjust(ct.target, result_type);
11031120
}
11041121

11051122
append_opcode_and_argcount(GDScriptFunction::OPCODE_CALL_BUILTIN_TYPE_VALIDATED, 2 + p_arguments.size());
@@ -1108,9 +1125,11 @@ void GDScriptByteCodeGenerator::write_call_builtin_type(const Address &p_target,
11081125
append(p_arguments[i]);
11091126
}
11101127
append(p_base);
1111-
append(target);
1128+
append(ct.target);
11121129
append(p_arguments.size());
11131130
append(Variant::get_validated_builtin_method(p_type, p_method));
1131+
ct.cleanup();
1132+
11141133
#ifdef DEBUG_ENABLED
11151134
add_debug_name(builtin_methods_names, get_builtin_method_pos(Variant::get_validated_builtin_method(p_type, p_method)), p_method);
11161135
#endif
@@ -1135,9 +1154,11 @@ void GDScriptByteCodeGenerator::write_call_native_static(const Address &p_target
11351154
for (int i = 0; i < p_arguments.size(); i++) {
11361155
append(p_arguments[i]);
11371156
}
1138-
append(get_call_target(p_target));
1157+
CallTarget ct = get_call_target(p_target);
1158+
append(ct.target);
11391159
append(method);
11401160
append(p_arguments.size());
1161+
ct.cleanup();
11411162
return;
11421163
}
11431164
}
@@ -1147,10 +1168,12 @@ void GDScriptByteCodeGenerator::write_call_method_bind(const Address &p_target,
11471168
for (int i = 0; i < p_arguments.size(); i++) {
11481169
append(p_arguments[i]);
11491170
}
1171+
CallTarget ct = get_call_target(p_target);
11501172
append(p_base);
1151-
append(get_call_target(p_target));
1173+
append(ct.target);
11521174
append(p_arguments.size());
11531175
append(p_method);
1176+
ct.cleanup();
11541177
}
11551178

11561179
void GDScriptByteCodeGenerator::write_call_ptrcall(const Address &p_target, const Address &p_base, MethodBind *p_method, const Vector<Address> &p_arguments) {
@@ -1212,9 +1235,11 @@ void GDScriptByteCodeGenerator::write_call_ptrcall(const Address &p_target, cons
12121235
append(p_arguments[i]);
12131236
}
12141237
append(p_base);
1215-
append(get_call_target(p_target));
1238+
CallTarget ct = get_call_target(p_target);
1239+
append(ct.target);
12161240
append(p_arguments.size());
12171241
append(p_method);
1242+
ct.cleanup();
12181243
if (is_ptrcall) {
12191244
alloc_ptrcall(p_arguments.size());
12201245
}
@@ -1228,9 +1253,11 @@ void GDScriptByteCodeGenerator::write_call_self(const Address &p_target, const S
12281253
append(p_arguments[i]);
12291254
}
12301255
append(GDScriptFunction::ADDR_TYPE_STACK << GDScriptFunction::ADDR_BITS);
1231-
append(get_call_target(p_target));
1256+
CallTarget ct = get_call_target(p_target);
1257+
append(ct.target);
12321258
append(p_arguments.size());
12331259
append(p_function_name);
1260+
ct.cleanup();
12341261
}
12351262

12361263
void GDScriptByteCodeGenerator::write_call_self_async(const Address &p_target, const StringName &p_function_name, const Vector<Address> &p_arguments) {
@@ -1239,9 +1266,11 @@ void GDScriptByteCodeGenerator::write_call_self_async(const Address &p_target, c
12391266
append(p_arguments[i]);
12401267
}
12411268
append(GDScriptFunction::ADDR_SELF);
1242-
append(get_call_target(p_target));
1269+
CallTarget ct = get_call_target(p_target);
1270+
append(ct.target);
12431271
append(p_arguments.size());
12441272
append(p_function_name);
1273+
ct.cleanup();
12451274
}
12461275

12471276
void GDScriptByteCodeGenerator::write_call_script_function(const Address &p_target, const Address &p_base, const StringName &p_function_name, const Vector<Address> &p_arguments) {
@@ -1250,9 +1279,11 @@ void GDScriptByteCodeGenerator::write_call_script_function(const Address &p_targ
12501279
append(p_arguments[i]);
12511280
}
12521281
append(p_base);
1253-
append(get_call_target(p_target));
1282+
CallTarget ct = get_call_target(p_target);
1283+
append(ct.target);
12541284
append(p_arguments.size());
12551285
append(p_function_name);
1286+
ct.cleanup();
12561287
}
12571288

12581289
void GDScriptByteCodeGenerator::write_lambda(const Address &p_target, GDScriptFunction *p_function, const Vector<Address> &p_captures, bool p_use_self) {
@@ -1261,9 +1292,11 @@ void GDScriptByteCodeGenerator::write_lambda(const Address &p_target, GDScriptFu
12611292
append(p_captures[i]);
12621293
}
12631294

1264-
append(get_call_target(p_target));
1295+
CallTarget ct = get_call_target(p_target);
1296+
append(ct.target);
12651297
append(p_captures.size());
12661298
append(p_function);
1299+
ct.cleanup();
12671300
}
12681301

12691302
void GDScriptByteCodeGenerator::write_construct(const Address &p_target, Variant::Type p_type, const Vector<Address> &p_arguments) {
@@ -1300,9 +1333,11 @@ void GDScriptByteCodeGenerator::write_construct(const Address &p_target, Variant
13001333
for (int i = 0; i < p_arguments.size(); i++) {
13011334
append(p_arguments[i]);
13021335
}
1303-
append(get_call_target(p_target));
1336+
CallTarget ct = get_call_target(p_target);
1337+
append(ct.target);
13041338
append(p_arguments.size());
13051339
append(Variant::get_validated_constructor(p_type, valid_constructor));
1340+
ct.cleanup();
13061341
#ifdef DEBUG_ENABLED
13071342
add_debug_name(constructors_names, get_constructor_pos(Variant::get_validated_constructor(p_type, valid_constructor)), Variant::get_type_name(p_type));
13081343
#endif
@@ -1314,39 +1349,47 @@ void GDScriptByteCodeGenerator::write_construct(const Address &p_target, Variant
13141349
for (int i = 0; i < p_arguments.size(); i++) {
13151350
append(p_arguments[i]);
13161351
}
1317-
append(get_call_target(p_target));
1352+
CallTarget ct = get_call_target(p_target);
1353+
append(ct.target);
13181354
append(p_arguments.size());
13191355
append(p_type);
1356+
ct.cleanup();
13201357
}
13211358

13221359
void GDScriptByteCodeGenerator::write_construct_array(const Address &p_target, const Vector<Address> &p_arguments) {
13231360
append_opcode_and_argcount(GDScriptFunction::OPCODE_CONSTRUCT_ARRAY, 1 + p_arguments.size());
13241361
for (int i = 0; i < p_arguments.size(); i++) {
13251362
append(p_arguments[i]);
13261363
}
1327-
append(get_call_target(p_target));
1364+
CallTarget ct = get_call_target(p_target);
1365+
append(ct.target);
13281366
append(p_arguments.size());
1367+
ct.cleanup();
13291368
}
13301369

13311370
void GDScriptByteCodeGenerator::write_construct_typed_array(const Address &p_target, const GDScriptDataType &p_element_type, const Vector<Address> &p_arguments) {
13321371
append_opcode_and_argcount(GDScriptFunction::OPCODE_CONSTRUCT_TYPED_ARRAY, 2 + p_arguments.size());
13331372
for (int i = 0; i < p_arguments.size(); i++) {
13341373
append(p_arguments[i]);
13351374
}
1336-
append(get_call_target(p_target));
1375+
CallTarget ct = get_call_target(p_target);
1376+
append(ct.target);
13371377
append(get_constant_pos(p_element_type.script_type) | (GDScriptFunction::ADDR_TYPE_CONSTANT << GDScriptFunction::ADDR_BITS));
13381378
append(p_arguments.size());
13391379
append(p_element_type.builtin_type);
13401380
append(p_element_type.native_type);
1381+
ct.cleanup();
13411382
}
13421383

13431384
void GDScriptByteCodeGenerator::write_construct_dictionary(const Address &p_target, const Vector<Address> &p_arguments) {
13441385
append_opcode_and_argcount(GDScriptFunction::OPCODE_CONSTRUCT_DICTIONARY, 1 + p_arguments.size());
13451386
for (int i = 0; i < p_arguments.size(); i++) {
13461387
append(p_arguments[i]);
13471388
}
1348-
append(get_call_target(p_target));
1389+
CallTarget ct = get_call_target(p_target);
1390+
append(ct.target);
13491391
append(p_arguments.size() / 2); // This is number of key-value pairs, so only half of actual arguments.
1392+
ct.cleanup();
13501393
}
13511394

13521395
void GDScriptByteCodeGenerator::write_await(const Address &p_target, const Address &p_operand) {

‎modules/gdscript/gdscript_byte_codegen.h‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,33 @@ class GDScriptByteCodeGenerator : public GDScriptCodeGenerator {
4848

4949
const static int RESERVED_STACK = 3; // For self, class, and nil.
5050

51+
struct CallTarget {
52+
Address target;
53+
bool is_new_temporary = false;
54+
GDScriptByteCodeGenerator *codegen = nullptr;
55+
#ifdef DEV_ENABLED
56+
bool cleaned = false;
57+
#endif
58+
59+
void cleanup() {
60+
DEV_ASSERT(!cleaned);
61+
if (is_new_temporary) {
62+
codegen->pop_temporary();
63+
}
64+
#ifdef DEV_ENABLED
65+
cleaned = true;
66+
#endif
67+
}
68+
69+
CallTarget(Address p_target, bool p_is_new_temporary, GDScriptByteCodeGenerator *p_codegen) :
70+
target(p_target),
71+
is_new_temporary(p_is_new_temporary),
72+
codegen(p_codegen) {}
73+
~CallTarget() { DEV_ASSERT(cleaned); }
74+
CallTarget(const CallTarget &) = delete;
75+
CallTarget &operator=(CallTarget &) = delete;
76+
};
77+
5178
bool ended = false;
5279
GDScriptFunction *function = nullptr;
5380
bool debug_stack = false;
@@ -326,7 +353,7 @@ class GDScriptByteCodeGenerator : public GDScriptCodeGenerator {
326353
}
327354
}
328355

329-
Address get_call_target(const Address &p_target, Variant::Type p_type = Variant::NIL);
356+
CallTarget get_call_target(const Address &p_target, Variant::Type p_type = Variant::NIL);
330357

331358
int address_of(const Address &p_address) {
332359
switch (p_address.mode) {

0 commit comments

Comments
 (0)