Skip to content

Commit

Permalink
Fix quadratic runtime for duplicate export name detection (#3861)
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 authored Oct 21, 2024
1 parent 87588ca commit 3ad9530
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 24 deletions.
61 changes: 50 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,56 @@ 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 +2816,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 +2863,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
62 changes: 49 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,53 @@ 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 +4131,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 +4203,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 3ad9530

Please sign in to comment.