Skip to content

Commit d3e8989

Browse files
wenyonghwenyongh
authored
wasm loader: Fix pop invalid offset count when stack top is ANY (#3516)
In wasm_loader_pop_frame_offset, when the stack is in polymorphic state and the stack top operand is VALUE_TYPE_ANY, if we popping I64/F64 operand, we should pop one offset but not two offsets. The issue was reported in #3513 and #3514.
1 parent ad5d31b commit d3e8989

5 files changed

Lines changed: 83 additions & 44 deletions

File tree

‎core/iwasm/interpreter/wasm_loader.c‎

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -9370,42 +9370,41 @@ wasm_loader_pop_frame_offset(WASMLoaderContext *ctx, uint8 type,
93709370
char *error_buf, uint32 error_buf_size)
93719371
{
93729372
/* if ctx->frame_csp equals ctx->frame_csp_bottom,
9373-
then current block is the function block */
9373+
then current block is the function block */
93749374
uint32 depth = ctx->frame_csp > ctx->frame_csp_bottom ? 1 : 0;
93759375
BranchBlock *cur_block = ctx->frame_csp - depth;
93769376
int32 available_stack_cell =
93779377
(int32)(ctx->stack_cell_num - cur_block->stack_cell_num);
9378+
uint32 cell_num_to_pop;
93789379

93799380
/* Directly return success if current block is in stack
9380-
* polymorphic state while stack is empty. */
9381+
polymorphic state while stack is empty. */
93819382
if (available_stack_cell <= 0 && cur_block->is_stack_polymorphic)
93829383
return true;
93839384

93849385
if (type == VALUE_TYPE_VOID)
93859386
return true;
93869387

9387-
if (is_32bit_type(type)) {
9388-
/* Check the offset stack bottom to ensure the frame offset
9389-
stack will not go underflow. But we don't thrown error
9390-
and return true here, because the error msg should be
9391-
given in wasm_loader_pop_frame_ref */
9392-
if (!check_offset_pop(ctx, 1))
9393-
return true;
9388+
/* Change type to ANY when the stack top is ANY, so as to avoid
9389+
popping unneeded offsets, e.g. if type is I64/F64, we may pop
9390+
two offsets */
9391+
if (available_stack_cell > 0 && *(ctx->frame_ref - 1) == VALUE_TYPE_ANY)
9392+
type = VALUE_TYPE_ANY;
93949393

9395-
ctx->frame_offset -= 1;
9396-
if ((*(ctx->frame_offset) > ctx->start_dynamic_offset)
9397-
&& (*(ctx->frame_offset) < ctx->max_dynamic_offset))
9398-
ctx->dynamic_offset -= 1;
9399-
}
9400-
else {
9401-
if (!check_offset_pop(ctx, 2))
9402-
return true;
9394+
cell_num_to_pop = wasm_value_type_cell_num(type);
9395+
9396+
/* Check the offset stack bottom to ensure the frame offset
9397+
stack will not go underflow. But we don't thrown error
9398+
and return true here, because the error msg should be
9399+
given in wasm_loader_pop_frame_ref */
9400+
if (!check_offset_pop(ctx, cell_num_to_pop))
9401+
return true;
9402+
9403+
ctx->frame_offset -= cell_num_to_pop;
9404+
if ((*(ctx->frame_offset) > ctx->start_dynamic_offset)
9405+
&& (*(ctx->frame_offset) < ctx->max_dynamic_offset))
9406+
ctx->dynamic_offset -= cell_num_to_pop;
94039407

9404-
ctx->frame_offset -= 2;
9405-
if ((*(ctx->frame_offset) > ctx->start_dynamic_offset)
9406-
&& (*(ctx->frame_offset) < ctx->max_dynamic_offset))
9407-
ctx->dynamic_offset -= 2;
9408-
}
94099408
emit_operand(ctx, *(ctx->frame_offset));
94109409

94119410
(void)error_buf;
@@ -10893,6 +10892,12 @@ wasm_loader_get_custom_section(WASMModule *module, const char *name,
1089310892
}
1089410893
#endif
1089510894

10895+
#if 0
10896+
#define HANDLE_OPCODE(opcode) #opcode
10897+
DEFINE_GOTO_TABLE(const char *, op_mnemonics);
10898+
#undef HANDLE_OPCODE
10899+
#endif
10900+
1089610901
static bool
1089710902
wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
1089810903
uint32 cur_func_idx, char *error_buf,

‎core/iwasm/interpreter/wasm_mini_loader.c‎

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4924,43 +4924,45 @@ wasm_loader_pop_frame_offset(WASMLoaderContext *ctx, uint8 type,
49244924
char *error_buf, uint32 error_buf_size)
49254925
{
49264926
/* if ctx->frame_csp equals ctx->frame_csp_bottom,
4927-
then current block is the function block */
4927+
then current block is the function block */
49284928
uint32 depth = ctx->frame_csp > ctx->frame_csp_bottom ? 1 : 0;
49294929
BranchBlock *cur_block = ctx->frame_csp - depth;
49304930
int32 available_stack_cell =
49314931
(int32)(ctx->stack_cell_num - cur_block->stack_cell_num);
4932+
uint32 cell_num_to_pop;
49324933

49334934
/* Directly return success if current block is in stack
4934-
* polymorphic state while stack is empty. */
4935+
polymorphic state while stack is empty. */
49354936
if (available_stack_cell <= 0 && cur_block->is_stack_polymorphic)
49364937
return true;
49374938

49384939
if (type == VALUE_TYPE_VOID)
49394940
return true;
49404941

4941-
if (is_32bit_type(type)) {
4942-
/* Check the offset stack bottom to ensure the frame offset
4943-
stack will not go underflow. But we don't thrown error
4944-
and return true here, because the error msg should be
4945-
given in wasm_loader_pop_frame_ref */
4946-
if (!check_offset_pop(ctx, 1))
4947-
return true;
4942+
/* Change type to ANY when the stack top is ANY, so as to avoid
4943+
popping unneeded offsets, e.g. if type is I64/F64, we may pop
4944+
two offsets */
4945+
if (available_stack_cell > 0 && *(ctx->frame_ref - 1) == VALUE_TYPE_ANY)
4946+
type = VALUE_TYPE_ANY;
49484947

4949-
ctx->frame_offset -= 1;
4950-
if ((*(ctx->frame_offset) > ctx->start_dynamic_offset)
4951-
&& (*(ctx->frame_offset) < ctx->max_dynamic_offset))
4952-
ctx->dynamic_offset -= 1;
4953-
}
4954-
else {
4955-
if (!check_offset_pop(ctx, 2))
4956-
return true;
4948+
cell_num_to_pop = wasm_value_type_cell_num(type);
4949+
4950+
/* Check the offset stack bottom to ensure the frame offset
4951+
stack will not go underflow. But we don't thrown error
4952+
and return true here, because the error msg should be
4953+
given in wasm_loader_pop_frame_ref */
4954+
if (!check_offset_pop(ctx, cell_num_to_pop))
4955+
return true;
4956+
4957+
ctx->frame_offset -= cell_num_to_pop;
4958+
if ((*(ctx->frame_offset) > ctx->start_dynamic_offset)
4959+
&& (*(ctx->frame_offset) < ctx->max_dynamic_offset))
4960+
ctx->dynamic_offset -= cell_num_to_pop;
49574961

4958-
ctx->frame_offset -= 2;
4959-
if ((*(ctx->frame_offset) > ctx->start_dynamic_offset)
4960-
&& (*(ctx->frame_offset) < ctx->max_dynamic_offset))
4961-
ctx->dynamic_offset -= 2;
4962-
}
49634962
emit_operand(ctx, *(ctx->frame_offset));
4963+
4964+
(void)error_buf;
4965+
(void)error_buf_size;
49644966
return true;
49654967
}
49664968

‎tests/regression/ba-issues/issues/issue-3513/iwasm_poc_04.wasm‎

1.49 KB
Binary file not shown.

‎tests/regression/ba-issues/issues/issue-3514/iwasm_poc_05.wasm‎

1.92 KB
Binary file not shown.

‎tests/regression/ba-issues/running_config.json‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1722,6 +1722,38 @@
17221722
"stdout content": "WASM module load failed: data count and data section have inconsistent lengths",
17231723
"description": "Check data segment count"
17241724
}
1725+
},
1726+
{
1727+
"deprecated": false,
1728+
"ids": [
1729+
3513
1730+
],
1731+
"runtime": "iwasm-default-wasi-disabled",
1732+
"file": "iwasm_poc_04.wasm",
1733+
"mode": "fast-interp",
1734+
"options": "-f main",
1735+
"argument": "",
1736+
"expected return": {
1737+
"ret code": 0,
1738+
"stdout content": "",
1739+
"description": "no sanitizer 'heap-buffer-overflow'"
1740+
}
1741+
},
1742+
{
1743+
"deprecated": false,
1744+
"ids": [
1745+
3514
1746+
],
1747+
"runtime": "iwasm-default-wasi-disabled",
1748+
"file": "iwasm_poc_05.wasm",
1749+
"mode": "fast-interp",
1750+
"options": "-f main",
1751+
"argument": "",
1752+
"expected return": {
1753+
"ret code": 0,
1754+
"stdout content": "",
1755+
"description": "no sanitizer 'heap-buffer-overflow'"
1756+
}
17251757
}
17261758
]
17271759
}

0 commit comments

Comments
 (0)