Skip to content

Commit 3be5c62

Browse files
adazem009adazem009
authored
Merge pull request #655 from scratchcpp/fix_string_leaks
Fix string leaks
2 parents 6eafec6 + 594df72 commit 3be5c62

40 files changed

+677
-432
lines changed

‎include/scratchcpp/list.h‎

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ class LIBSCRATCHCPP_EXPORT List : public Entity
153153
}
154154

155155
/*! Joins the list items with spaces or without any separator if there are only digits and returns the result as StringPtr. */
156-
inline StringPtr *toStringPtr() const
156+
inline void toStringPtr(StringPtr *dst) const
157157
{
158158
veque::veque<StringPtr *> strings;
159159
size_t size = 0;
@@ -163,7 +163,9 @@ class LIBSCRATCHCPP_EXPORT List : public Entity
163163

164164
for (i = 0; i < m_size; i++) {
165165
const ValueData *item = &m_dataPtr->operator[](i);
166-
strings.push_back(value_toStringPtr(item));
166+
StringPtr *str = string_pool_new();
167+
value_toStringPtr(item, str);
168+
strings.push_back(str);
167169
size += strings.back()->size;
168170

169171
if (value_isValidNumber(item) && !value_isBool(item) && strings.back()->size > 0) {
@@ -185,54 +187,54 @@ class LIBSCRATCHCPP_EXPORT List : public Entity
185187
}
186188
}
187189

188-
StringPtr *ret = string_pool_new();
189-
ret->size = 0;
190+
dst->size = 0;
190191

191192
if (digits) {
192-
string_alloc(ret, size);
193+
string_alloc(dst, size);
193194

194195
for (i = 0; i < strings.size(); i++) {
195-
memcpy(ret->data + ret->size, strings[i]->data, strings[i]->size * sizeof(char16_t));
196-
ret->size += strings[i]->size;
196+
memcpy(dst->data + dst->size, strings[i]->data, strings[i]->size * sizeof(char16_t));
197+
dst->size += strings[i]->size;
197198
string_pool_free(strings[i]);
198199
}
199200

200201
for (; i < m_size; i++) {
201-
StringPtr *item = value_toStringPtr(&m_dataPtr->operator[](i));
202+
StringPtr *item = string_pool_new();
203+
value_toStringPtr(&m_dataPtr->operator[](i), item);
202204
size += item->size + 1;
203-
string_alloc(ret, size);
204-
memcpy(ret->data + ret->size, item->data, item->size * sizeof(char16_t));
205-
ret->size += item->size;
205+
string_alloc(dst, size);
206+
memcpy(dst->data + dst->size, item->data, item->size * sizeof(char16_t));
207+
dst->size += item->size;
206208
string_pool_free(item);
207209
}
208210
} else {
209211
size += strings.size() - 1;
210-
string_alloc(ret, size);
212+
string_alloc(dst, size);
211213

212214
for (i = 0; i < strings.size(); i++) {
213-
memcpy(ret->data + ret->size, strings[i]->data, strings[i]->size * sizeof(char16_t));
214-
ret->size += strings[i]->size;
215+
memcpy(dst->data + dst->size, strings[i]->data, strings[i]->size * sizeof(char16_t));
216+
dst->size += strings[i]->size;
215217
string_pool_free(strings[i]);
216218

217219
if (i + 1 < m_size)
218-
ret->data[ret->size++] = u' ';
220+
dst->data[dst->size++] = u' ';
219221
}
220222

221223
for (; i < m_size; i++) {
222-
StringPtr *item = value_toStringPtr(&m_dataPtr->operator[](i));
224+
StringPtr *item = string_pool_new();
225+
value_toStringPtr(&m_dataPtr->operator[](i), item);
223226
size += item->size + 1;
224-
string_alloc(ret, size);
225-
memcpy(ret->data + ret->size, item->data, item->size * sizeof(char16_t));
226-
ret->size += item->size;
227+
string_alloc(dst, size);
228+
memcpy(dst->data + dst->size, item->data, item->size * sizeof(char16_t));
229+
dst->size += item->size;
227230
string_pool_free(item);
228231

229232
if (i + 1 < m_size)
230-
ret->data[ret->size++] = u' ';
233+
dst->data[dst->size++] = u' ';
231234
}
232235
}
233236

234-
ret->data[ret->size] = u'\0';
235-
return ret;
237+
dst->data[dst->size] = u'\0';
236238
}
237239

238240
std::string toString() const;

‎include/scratchcpp/string_pool.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ struct StringPtr;
1111

1212
extern "C"
1313
{
14-
LIBSCRATCHCPP_EXPORT StringPtr *string_pool_new(bool internal = false);
14+
LIBSCRATCHCPP_EXPORT StringPtr *string_pool_new();
1515
LIBSCRATCHCPP_EXPORT void string_pool_free(StringPtr *str);
1616
}
1717

‎include/scratchcpp/thread.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ class LIBSCRATCHCPP_EXPORT Thread
2020
public:
2121
Thread(Target *target, IEngine *engine, Script *script);
2222
Thread(const Thread &) = delete;
23-
~Thread();
2423

2524
Target *target() const;
2625
IEngine *engine() const;

‎include/scratchcpp/value_functions.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,14 +75,14 @@ extern "C"
7575
LIBSCRATCHCPP_EXPORT double value_toDouble(const ValueData *v);
7676
LIBSCRATCHCPP_EXPORT bool value_toBool(const ValueData *v);
7777
LIBSCRATCHCPP_EXPORT void value_toString(const ValueData *v, std::string *dst);
78-
LIBSCRATCHCPP_EXPORT StringPtr *value_toStringPtr(const ValueData *v);
78+
LIBSCRATCHCPP_EXPORT void value_toStringPtr(const ValueData *v, StringPtr *dst);
7979
LIBSCRATCHCPP_EXPORT void value_toUtf16(const ValueData *v, std::u16string *dst);
8080
LIBSCRATCHCPP_EXPORT Rgb value_toRgba(const ValueData *v);
8181
LIBSCRATCHCPP_EXPORT const void *value_toPointer(const ValueData *v);
8282

8383
LIBSCRATCHCPP_EXPORT bool value_doubleIsInt(double v);
8484

85-
LIBSCRATCHCPP_EXPORT StringPtr *value_doubleToStringPtr(double v);
85+
LIBSCRATCHCPP_EXPORT void value_doubleToStringPtr(double v, StringPtr *dst);
8686
LIBSCRATCHCPP_EXPORT const StringPtr *value_boolToStringPtr(bool v);
8787
LIBSCRATCHCPP_EXPORT double value_stringToDouble(const StringPtr *s);
8888
LIBSCRATCHCPP_EXPORT double value_stringToDoubleWithCheck(const StringPtr *s, bool *ok);

‎src/blocks/looksblocks.cpp‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -561,25 +561,21 @@ extern "C" double looks_backdrop_number(ExecutionContext *ctx)
561561
return ctx->engine()->stage()->costumeIndex() + 1;
562562
}
563563

564-
extern "C" StringPtr *looks_backdrop_name(ExecutionContext *ctx)
564+
extern "C" void looks_backdrop_name(StringPtr *ret, ExecutionContext *ctx)
565565
{
566566
const std::string &name = ctx->engine()->stage()->currentCostume()->name();
567-
StringPtr *ret = string_pool_new();
568567
string_assign_cstring(ret, name.c_str());
569-
return ret;
570568
}
571569

572570
extern "C" double looks_costume_number(Target *target)
573571
{
574572
return target->costumeIndex() + 1;
575573
}
576574

577-
extern "C" StringPtr *looks_costume_name(Target *target)
575+
extern "C" void looks_costume_name(StringPtr *ret, Target *target)
578576
{
579577
const std::string &name = target->currentCostume()->name();
580-
StringPtr *ret = string_pool_new();
581578
string_assign_cstring(ret, name.c_str());
582-
return ret;
583579
}
584580

585581
extern "C" bool looks_backdrop_promise(ExecutionContext *ctx)

‎src/blocks/sensingblocks.cpp‎

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -522,11 +522,9 @@ extern "C" void sensing_askandwait(ExecutionContext *ctx, const StringPtr *quest
522522
ctx->thread()->setPromise(std::make_shared<Promise>());
523523
}
524524

525-
extern "C" StringPtr *sensing_answer(ExecutionContext *ctx)
525+
extern "C" void sensing_answer(StringPtr *ret, ExecutionContext *ctx)
526526
{
527-
StringPtr *ret = string_pool_new();
528527
string_assign(ret, ctx->engine()->answer());
529-
return ret;
530528
}
531529

532530
extern "C" bool sensing_keypressed(ExecutionContext *ctx, const StringPtr *key)
@@ -595,12 +593,10 @@ extern "C" double sensing_costume_number_of_target(Target *target)
595593
return target->costumeIndex() + 1;
596594
}
597595

598-
extern "C" StringPtr *sensing_costume_name_of_target(Target *target)
596+
extern "C" void sensing_costume_name_of_target(StringPtr *ret, Target *target)
599597
{
600598
const std::string &name = target->currentCostume()->name();
601-
StringPtr *ret = string_pool_new();
602599
string_assign_cstring(ret, name.c_str());
603-
return ret;
604600
}
605601

606602
extern "C" double sensing_size_of_sprite(Sprite *sprite)
@@ -659,17 +655,13 @@ extern "C" double sensing_costume_number_of_sprite_with_check(Target *target)
659655
return 0.0;
660656
}
661657

662-
extern "C" StringPtr *sensing_costume_name_of_sprite_with_check(Target *target)
658+
extern "C" void sensing_costume_name_of_sprite_with_check(StringPtr *ret, Target *target)
663659
{
664-
StringPtr *ret = string_pool_new();
665-
666660
if (target && !target->isStage()) {
667661
const std::string &name = target->currentCostume()->name();
668662
string_assign_cstring(ret, name.c_str());
669663
} else
670664
string_assign_cstring(ret, "0");
671-
672-
return ret;
673665
}
674666

675667
extern "C" double sensing_size_of_sprite_with_check(Target *target)
@@ -690,17 +682,13 @@ extern "C" double sensing_backdrop_number_of_stage_with_check(Target *target)
690682
return 0.0;
691683
}
692684

693-
extern "C" StringPtr *sensing_backdrop_name_of_stage_with_check(Target *target)
685+
extern "C" void sensing_backdrop_name_of_stage_with_check(StringPtr *ret, Target *target)
694686
{
695-
StringPtr *ret = string_pool_new();
696-
697687
if (target && target->isStage()) {
698688
const std::string &name = target->currentCostume()->name();
699689
string_assign_cstring(ret, name.c_str());
700690
} else
701691
string_assign_cstring(ret, "");
702-
703-
return ret;
704692
}
705693

706694
extern "C" double sensing_volume_of_target_with_check(Target *target)

‎src/engine/internal/llvm/instructions/control.cpp‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,6 @@ LLVMInstruction *Control::buildBeginIf(LLVMInstruction *ins)
119119
m_builder.SetInsertPoint(statement.body);
120120

121121
m_utils.ifStatements().push_back(statement);
122-
m_utils.pushScopeLevel();
123122

124123
return ins->next;
125124
}
@@ -135,7 +134,6 @@ LLVMInstruction *Control::buildBeginElse(LLVMInstruction *ins)
135134
// Jump to the branch after the if statement
136135
assert(!statement.afterIf);
137136
statement.afterIf = llvm::BasicBlock::Create(llvmCtx, "", function);
138-
m_utils.freeScopeHeap();
139137
m_builder.CreateBr(statement.afterIf);
140138

141139
// Create else branch
@@ -157,7 +155,6 @@ LLVMInstruction *Control::buildEndIf(LLVMInstruction *ins)
157155
auto &ifStatements = m_utils.ifStatements();
158156
assert(!ifStatements.empty());
159157
LLVMIfStatement &statement = ifStatements.back();
160-
m_utils.freeScopeHeap();
161158

162159
// Jump to the branch after the if statement
163160
if (!statement.afterIf)
@@ -176,7 +173,6 @@ LLVMInstruction *Control::buildEndIf(LLVMInstruction *ins)
176173
m_builder.SetInsertPoint(statement.afterIf);
177174

178175
ifStatements.pop_back();
179-
m_utils.popScopeLevel();
180176

181177
return ins->next;
182178
}
@@ -236,7 +232,6 @@ LLVMInstruction *Control::buildBeginRepeatLoop(LLVMInstruction *ins)
236232
m_builder.SetInsertPoint(body);
237233

238234
m_utils.loops().push_back(loop);
239-
m_utils.pushScopeLevel();
240235

241236
return ins->next;
242237
}
@@ -274,7 +269,6 @@ LLVMInstruction *Control::buildBeginWhileLoop(LLVMInstruction *ins)
274269

275270
// Switch to body branch
276271
m_builder.SetInsertPoint(body);
277-
m_utils.pushScopeLevel();
278272

279273
return ins->next;
280274
}
@@ -300,7 +294,6 @@ LLVMInstruction *Control::buildBeginRepeatUntilLoop(LLVMInstruction *ins)
300294

301295
// Switch to body branch
302296
m_builder.SetInsertPoint(body);
303-
m_utils.pushScopeLevel();
304297

305298
return ins->next;
306299
}
@@ -331,14 +324,12 @@ LLVMInstruction *Control::buildEndLoop(LLVMInstruction *ins)
331324
}
332325

333326
// Jump to the condition branch
334-
m_utils.freeScopeHeap();
335327
m_builder.CreateBr(loop.conditionBranch);
336328

337329
// Switch to the branch after the loop
338330
m_builder.SetInsertPoint(loop.afterLoop);
339331

340332
loops.pop_back();
341-
m_utils.popScopeLevel();
342333

343334
return ins->next;
344335
}
@@ -351,7 +342,6 @@ LLVMInstruction *Control::buildStop(LLVMInstruction *ins)
351342

352343
LLVMInstruction *Control::buildStopWithoutSync(LLVMInstruction *ins)
353344
{
354-
m_utils.freeScopeHeap();
355345
m_builder.CreateBr(m_utils.endBranch());
356346
llvm::BasicBlock *nextBranch = llvm::BasicBlock::Create(m_utils.llvmCtx(), "", m_utils.function());
357347
m_builder.SetInsertPoint(nextBranch);

‎src/engine/internal/llvm/instructions/functions.cpp‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,15 @@ LLVMInstruction *Functions::buildFunctionCall(LLVMInstruction *ins)
3232
// Variables must be synchronized because the function can read them
3333
m_utils.syncVariables();
3434

35+
// Strings are returned through an output parameter
36+
llvm::Value *stringRet = nullptr;
37+
38+
if (ins->functionReturnReg && ins->functionReturnReg->type() == Compiler::StaticType::String) {
39+
stringRet = m_utils.addStringAlloca();
40+
types.push_back(m_utils.getType(Compiler::StaticType::String, false));
41+
args.push_back(stringRet);
42+
}
43+
3544
// Add execution context arg
3645
if (ins->functionCtxArg) {
3746
types.push_back(llvm::PointerType::get(llvm::Type::getInt8Ty(m_utils.llvmCtx()), 0));
@@ -54,15 +63,14 @@ LLVMInstruction *Functions::buildFunctionCall(LLVMInstruction *ins)
5463
llvm::Value *ret = m_builder.CreateCall(m_utils.functions().resolveFunction(ins->functionName, llvm::FunctionType::get(retType, types, false)), args);
5564

5665
if (ins->functionReturnReg) {
57-
if (m_utils.isSingleType(ins->functionReturnReg->type()))
66+
if (ins->functionReturnReg->type() == Compiler::StaticType::String)
67+
ins->functionReturnReg->value = stringRet;
68+
else if (m_utils.isSingleType(ins->functionReturnReg->type()))
5869
ins->functionReturnReg->value = ret;
5970
else {
6071
ins->functionReturnReg->value = m_utils.addAlloca(retType);
6172
m_builder.CreateStore(ret, ins->functionReturnReg->value);
6273
}
63-
64-
if (ins->functionReturnReg->type() == Compiler::StaticType::String)
65-
m_utils.freeStringLater(ins->functionReturnReg->value);
6674
}
6775

6876
return ins->next;

‎src/engine/internal/llvm/instructions/lists.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -270,8 +270,8 @@ LLVMInstruction *Lists::buildGetListContents(LLVMInstruction *ins)
270270
{
271271
assert(ins->args.size() == 0);
272272
const LLVMListPtr &listPtr = m_utils.listPtr(ins->targetList);
273-
llvm::Value *ptr = m_builder.CreateCall(m_utils.functions().resolve_list_to_string(), listPtr.ptr);
274-
m_utils.freeStringLater(ptr);
273+
llvm::Value *ptr = m_utils.addStringAlloca();
274+
m_builder.CreateCall(m_utils.functions().resolve_list_to_string(), { listPtr.ptr, ptr });
275275
ins->functionReturnReg->value = ptr;
276276

277277
return ins->next;

‎src/engine/internal/llvm/instructions/procedures.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ LLVMInstruction *Procedures::buildCallProcedure(LLVMInstruction *ins)
3838

3939
assert(ins->procedurePrototype);
4040
assert(ins->args.size() == ins->procedurePrototype->argumentTypes().size());
41-
m_utils.freeScopeHeap();
4241
m_utils.syncVariables();
4342

4443
std::string name = m_utils.scriptFunctionName(ins->procedurePrototype);

0 commit comments

Comments
 (0)