Skip to content

Commit

Permalink
Fix ALLOCA macro to prevent memory leak.
Browse files Browse the repository at this point in the history 
If you don't use unique_ptr, the memory leak can occur if there is c++ exception.

Signed-off-by: Seonghyun Kim <[email protected]>
  • Loading branch information
@ksh8281
ksh8281 committed Aug 18, 2023
1 parent d52105d commit 06a9e89
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 43 deletions.
14 changes: 10 additions & 4 deletions src/Walrus.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -343,10 +343,16 @@ if (f.type == Type::B) { puts("failed in msvc."); }
static void operator delete(void*) = delete; \
static void operator delete[](void*) = delete;

#define ALLOCA(Type, Result, Bytes, Success) \
Type* Result = (Type*)(LIKELY(Bytes < 512) ? alloca(Bytes) \
: new char[Bytes]); \
bool Success = LIKELY(Bytes < 512) ? true : false;
#define ALLOCA(Type, Result, Bytes) \
std::unique_ptr<uint8_t[]> Result##HolderWhenUsingMalloc; \
size_t bytes##Result = (Bytes); \
Type* Result; \
if (LIKELY(bytes##Result < 512)) { \
Result = (Type*)alloca(bytes##Result); \
} else { \
Result##HolderWhenUsingMalloc = std::unique_ptr<uint8_t[]>(new uint8_t[bytes##Result]); \
Result = (Type*)Result##HolderWhenUsingMalloc.get(); \
}

#if !defined(STACK_GROWS_DOWN) && !defined(STACK_GROWS_UP)
#define STACK_GROWS_DOWN
Expand Down
23 changes: 4 additions & 19 deletions src/interpreter/Interpreter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1359,15 +1359,15 @@ NEVER_INLINE void Interpreter::callOperation(
Function* target = instance->function(code->index());
const FunctionType* ft = target->functionType();
const ValueTypeVector& param = ft->param();
ALLOCA(Value, paramVector, sizeof(Value) * param.size(), isAllocaParam);
ALLOCA(Value, paramVector, sizeof(Value) * param.size());

size_t c = 0;
for (size_t i = 0; i < param.size(); i++) {
paramVector[i] = Value(param[i], bp + code->stackOffsets()[c++]);
}

const ValueTypeVector& result = ft->result();
ALLOCA(Value, resultVector, sizeof(Value) * result.size(), isAllocaResult);
ALLOCA(Value, resultVector, sizeof(Value) * result.size());
size_t codeExtraOffsetsSize = sizeof(ByteCodeStackOffset) * ft->param().size() + sizeof(ByteCodeStackOffset) * ft->result().size();

target->call(state, param.size(), paramVector, resultVector);
Expand All @@ -1377,13 +1377,6 @@ NEVER_INLINE void Interpreter::callOperation(
resultVector[i].writeToMemory(resultStackPointer);
}

if (UNLIKELY(!isAllocaParam)) {
delete[] paramVector;
}
if (UNLIKELY(!isAllocaResult)) {
delete[] resultVector;
}

programCounter += sizeof(Call) + codeExtraOffsetsSize;
}

Expand All @@ -1409,15 +1402,15 @@ NEVER_INLINE void Interpreter::callIndirectOperation(
Trap::throwException(state, "indirect call type mismatch");
}
const ValueTypeVector& param = ft->param();
ALLOCA(Value, paramVector, sizeof(Value) * param.size(), isAllocaParam);
ALLOCA(Value, paramVector, sizeof(Value) * param.size());

size_t c = 0;
for (size_t i = 0; i < param.size(); i++) {
paramVector[i] = Value(param[i], bp + code->stackOffsets()[c++]);
}

const ValueTypeVector& result = ft->result();
ALLOCA(Value, resultVector, sizeof(Value) * result.size(), isAllocaResult);
ALLOCA(Value, resultVector, sizeof(Value) * result.size());
size_t codeExtraOffsetsSize = sizeof(ByteCodeStackOffset) * ft->param().size() + sizeof(ByteCodeStackOffset) * ft->result().size();

target->call(state, param.size(), paramVector, resultVector);
Expand All @@ -1427,14 +1420,6 @@ NEVER_INLINE void Interpreter::callIndirectOperation(
resultVector[i].writeToMemory(resultStackPointer);
}

if (UNLIKELY(!isAllocaParam)) {
delete[] paramVector;
}
if (UNLIKELY(!isAllocaResult)) {
delete[] resultVector;
}

programCounter += sizeof(CallIndirect) + codeExtraOffsetsSize;
}

} // namespace Walrus
6 changes: 1 addition & 5 deletions src/runtime/Function.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ void DefinedFunction::call(ExecutionState& state, const uint32_t argc, Value* ar
{
ExecutionState newState(state, this);
checkStackLimit(newState);
ALLOCA(uint8_t, functionStackBase, m_moduleFunction->requiredStackSize(), isAlloca);
ALLOCA(uint8_t, functionStackBase, m_moduleFunction->requiredStackSize());
uint8_t* functionStackPointer = functionStackBase;

// init parameter space
Expand Down Expand Up @@ -100,10 +100,6 @@ void DefinedFunction::call(ExecutionState& state, const uint32_t argc, Value* ar
for (size_t i = 0; i < resultTypeInfo.size(); i++) {
result[i] = Value(resultTypeInfo[i], functionStackBase + resultOffsets[i]);
}

if (UNLIKELY(!isAlloca)) {
delete[] functionStackBase;
}
}

ImportedFunction* ImportedFunction::createImportedFunction(Store* store,
Expand Down
18 changes: 3 additions & 15 deletions src/runtime/Module.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,16 +226,12 @@ Instance* Module::instantiate(ExecutionState& state, const ExternVector& imports
Walrus::Trap trap;
trap.run([](Walrus::ExecutionState& state, void* d) {
RunData* data = reinterpret_cast<RunData*>(d);
ALLOCA(uint8_t, functionStackBase, data->mf->requiredStackSize(), isAlloca);
ALLOCA(uint8_t, functionStackBase, data->mf->requiredStackSize());

DefinedFunction fakeFunction(data->instance, data->mf);
ExecutionState newState(state, &fakeFunction);
auto resultOffset = Interpreter::interpret(newState, functionStackBase);
data->instance->m_globals[data->index]->setValue(Value(data->type, functionStackBase + resultOffset[0]));

if (UNLIKELY(!isAlloca)) {
delete[] functionStackBase;
}
},
&data);
}
Expand All @@ -261,7 +257,7 @@ Instance* Module::instantiate(ExecutionState& state, const ExternVector& imports
Walrus::Trap trap;
trap.run([](Walrus::ExecutionState& state, void* d) {
RunData* data = reinterpret_cast<RunData*>(d);
ALLOCA(uint8_t, functionStackBase, data->elem->moduleFunction()->requiredStackSize(), isAlloca);
ALLOCA(uint8_t, functionStackBase, data->elem->moduleFunction()->requiredStackSize());

DefinedFunction fakeFunction(data->instance,
data->elem->moduleFunction());
Expand All @@ -270,10 +266,6 @@ Instance* Module::instantiate(ExecutionState& state, const ExternVector& imports
auto resultOffset = Interpreter::interpret(newState, functionStackBase);
Value offset(Value::I32, functionStackBase + resultOffset[0]);
data->index = offset.asI32();

if (UNLIKELY(!isAlloca)) {
delete[] functionStackBase;
}
},
&data);
}
Expand Down Expand Up @@ -312,7 +304,7 @@ Instance* Module::instantiate(ExecutionState& state, const ExternVector& imports
auto result = trap.run([](Walrus::ExecutionState& state, void* d) {
RunData* data = reinterpret_cast<RunData*>(d);
if (data->init->moduleFunction()->currentByteCodeSize()) {
ALLOCA(uint8_t, functionStackBase, data->init->moduleFunction()->requiredStackSize(), isAlloca);
ALLOCA(uint8_t, functionStackBase, data->init->moduleFunction()->requiredStackSize());

DefinedFunction fakeFunction(data->instance,
data->init->moduleFunction());
Expand All @@ -321,10 +313,6 @@ Instance* Module::instantiate(ExecutionState& state, const ExternVector& imports
auto resultOffset = Interpreter::interpret(newState, functionStackBase);
Value offset(Value::I32, functionStackBase + resultOffset[0]);

if (UNLIKELY(!isAlloca)) {
delete[] functionStackBase;
}

Memory* m = data->instance->memory(0);
const auto& initData = data->init->initData();
if (m->sizeInByte() >= initData.size() && (offset.asI32() + initData.size()) <= m->sizeInByte() && offset.asI32() >= 0) {
Expand Down

0 comments on commit 06a9e89

Please sign in to comment.