Skip to content

Commit

Permalink
Fix quadratic runtime for duplicate export name detection
Browse files Browse the repository at this point in the history 
Previously, the loader would check the name of a new export against all
existing exports, leading to a quadratic running time.

This change makes the loader parse the entire export section. The
exports are then sorted by name, then adjacent exports are checked for
uniqueness.
  • Loading branch information
@sjamesr
sjamesr committed Oct 21, 2024
1 parent 48eaa22 commit 980593e
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 24 deletions.
62 changes: 51 additions & 11 deletions core/iwasm/aot/aot_loader.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@
*/

#include "aot_runtime.h"
#include "bh_common.h"
#include "bh_log.h"
#include "aot_reloc.h"
#include "bh_platform.h"
#include "../common/wasm_runtime_common.h"
#include "../common/wasm_native.h"
#include "../common/wasm_loader_common.h"
Expand Down Expand Up @@ -2753,14 +2752,57 @@ destroy_exports(AOTExport *exports)
wasm_runtime_free(exports);
}

static int
cmp_export_name(const void *a, const void *b)
{
return strcmp(*(char **)a, *(char **)b);
}

static bool
check_duplicate_exports(AOTModule *module, char *error_buf,
uint32 error_buf_size)
{
uint32 i;
bool result = false;
char *names_buf[32], **names = names_buf;
if (module->export_count > 32) {
names = loader_malloc(module->export_count * sizeof(char *), error_buf,
error_buf_size);
}

if (!names) {
return result;
}

for (i = 0; i < module->export_count; i++) {
names[i] = module->exports[i].name;
}

qsort(names, module->export_count, sizeof(char *), cmp_export_name);

for (i = 1; i < module->export_count; i++) {
if (!strcmp(names[i], names[i - 1])) {
set_error_buf(error_buf, error_buf_size, "duplicate export name");
goto cleanup;
}
}

result = true;
cleanup:
if (module->export_count > 32) {
wasm_runtime_free(names);
}
return result;
}

static bool
load_exports(const uint8 **p_buf, const uint8 *buf_end, AOTModule *module,
bool is_load_from_file_buf, char *error_buf, uint32 error_buf_size)
{
const uint8 *buf = *p_buf;
AOTExport *exports;
uint64 size;
uint32 i, j;
uint32 i;

/* Allocate memory */
size = sizeof(AOTExport) * (uint64)module->export_count;
Expand All @@ -2775,14 +2817,6 @@ load_exports(const uint8 **p_buf, const uint8 *buf_end, AOTModule *module,
read_uint8(buf, buf_end, exports[i].kind);
read_string(buf, buf_end, exports[i].name);

for (j = 0; j < i; j++) {
if (!strcmp(exports[i].name, exports[j].name)) {
set_error_buf(error_buf, error_buf_size,
"duplicate export name");
return false;
}
}

/* Check export kind and index */
switch (exports[i].kind) {
case EXPORT_KIND_FUNC:
Expand Down Expand Up @@ -2830,6 +2864,12 @@ load_exports(const uint8 **p_buf, const uint8 *buf_end, AOTModule *module,
}
}

if (module->export_count > 0) {
if (!check_duplicate_exports(module, error_buf, error_buf_size)) {
return false;
}
}

*p_buf = buf;
return true;
fail:
Expand Down
63 changes: 50 additions & 13 deletions core/iwasm/interpreter/wasm_loader.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
*/

#include "wasm_loader.h"
#include "bh_common.h"
#include "bh_log.h"
#include "bh_platform.h"
#include "wasm.h"
#include "wasm_opcode.h"
#include "wasm_runtime.h"
Expand Down Expand Up @@ -3184,6 +3183,12 @@ load_memory(const uint8 **p_buf, const uint8 *buf_end, WASMMemory *memory,
return false;
}

static int
cmp_export_name(const void *a, const void *b)
{
return strcmp(*(char **)a, *(char **)b);
}

static bool
load_import_section(const uint8 *buf, const uint8 *buf_end, WASMModule *module,
bool is_load_from_file_buf, bool no_resolve,
Expand Down Expand Up @@ -4054,17 +4059,54 @@ load_global_section(const uint8 *buf, const uint8 *buf_end, WASMModule *module,
return false;
}

static bool
check_duplicate_exports(WASMModule *module, char *error_buf,
uint32 error_buf_size)
{
uint32 i;
bool result = false;
char *names_buf[32], **names = names_buf;

if (module->export_count > 32) {
names = loader_malloc(module->export_count * sizeof(char *), error_buf,
error_buf_size);
}

if (!names) {
return result;
}

for (i = 0; i < module->export_count; i++) {
names[i] = module->exports[i].name;
}

qsort(names, module->export_count, sizeof(char *), cmp_export_name);

for (i = 1; i < module->export_count; i++) {
if (!strcmp(names[i], names[i - 1])) {
set_error_buf(error_buf, error_buf_size, "duplicate export name");
goto cleanup;
}
}

result = true;
cleanup:
if (module->export_count > 32) {
wasm_runtime_free(names);
}
return result;
}

static bool
load_export_section(const uint8 *buf, const uint8 *buf_end, WASMModule *module,
bool is_load_from_file_buf, char *error_buf,
uint32 error_buf_size)
{
const uint8 *p = buf, *p_end = buf_end;
uint32 export_count, i, j, index;
uint32 export_count, i, index;
uint64 total_size;
uint32 str_len;
WASMExport *export;
const char *name;

read_leb_uint32(p, p_end, export_count);

Expand All @@ -4090,15 +4132,6 @@ load_export_section(const uint8 *buf, const uint8 *buf_end, WASMModule *module,
read_leb_uint32(p, p_end, str_len);
CHECK_BUF(p, p_end, str_len);

for (j = 0; j < i; j++) {
name = module->exports[j].name;
if (strlen(name) == str_len && memcmp(name, p, str_len) == 0) {
set_error_buf(error_buf, error_buf_size,
"duplicate export name");
return false;
}
}

if (!(export->name = wasm_const_str_list_insert(
p, str_len, module, is_load_from_file_buf, error_buf,
error_buf_size))) {
Expand Down Expand Up @@ -4171,6 +4204,10 @@ load_export_section(const uint8 *buf, const uint8 *buf_end, WASMModule *module,
return false;
}
}

if (!check_duplicate_exports(module, error_buf, error_buf_size)) {
return false;
}
}

if (p != p_end) {
Expand Down

0 comments on commit 980593e

Please sign in to comment.