Skip to content

Commit

Permalink
Fix fast-interp polymorphic stack processing (#2974)
Browse files Browse the repository at this point in the history
Fix issue #2951, #2952 and #2953.
  • Loading branch information
xujuntwt95329 authored Jan 4, 2024
1 parent a275190 commit f96257b
Show file tree
Hide file tree
Showing 2 changed files with 219 additions and 73 deletions.
145 changes: 110 additions & 35 deletions core/iwasm/interpreter/wasm_loader.c
Original file line number Diff line number Diff line change
Expand Up @@ -4940,6 +4940,9 @@ typedef struct BranchBlock {
BranchBlockPatch *patch_list;
/* This is used to save params frame_offset of of if block */
int16 *param_frame_offsets;
/* This is used to store available param num for if/else branch, so the else
* opcode can know how many parameters should be copied to the stack */
uint32 available_param_num;
#endif

/* Indicate the operand stack is in polymorphic state.
Expand Down Expand Up @@ -6857,15 +6860,18 @@ check_block_stack(WASMLoaderContext *loader_ctx, BranchBlock *block,
* 1) POP original parameter out;
* 2) Push and copy original values to dynamic space.
* The copy instruction format:
* Part a: param count
* Part a: available param count
* Part b: all param total cell num
* Part c: each param's cell_num, src offset and dst offset
* Part d: each param's src offset
* Part e: each param's dst offset
* Note: if the stack is in polymorphic state, the actual copied parameters may
* be fewer than the defined number in block type
*/
static bool
copy_params_to_dynamic_space(WASMLoaderContext *loader_ctx, bool is_if_block,
char *error_buf, uint32 error_buf_size)
uint32 *p_available_param_count, char *error_buf,
uint32 error_buf_size)
{
bool ret = false;
int16 *frame_offset = NULL;
Expand All @@ -6877,35 +6883,47 @@ copy_params_to_dynamic_space(WASMLoaderContext *loader_ctx, bool is_if_block,
BlockType *block_type = &block->block_type;
WASMType *wasm_type = block_type->u.type;
uint32 param_count = block_type->u.type->param_count;
uint32 available_param_count = 0;
int16 condition_offset = 0;
bool disable_emit = false;
int16 operand_offset = 0;
uint64 size;

if (is_if_block)
condition_offset = *loader_ctx->frame_offset;

/* POP original parameter out */
for (i = 0; i < param_count; i++) {
int32 available_stack_cell =
(int32)(loader_ctx->stack_cell_num - block->stack_cell_num);

uint64 size = (uint64)param_count * (sizeof(*cells) + sizeof(*src_offsets));
if (available_stack_cell <= 0 && block->is_stack_polymorphic)
break;

POP_OFFSET_TYPE(wasm_type->types[param_count - i - 1]);
wasm_loader_emit_backspace(loader_ctx, sizeof(int16));
}
available_param_count = i;

size =
(uint64)available_param_count * (sizeof(*cells) + sizeof(*src_offsets));

/* For if block, we also need copy the condition operand offset. */
if (is_if_block)
size += sizeof(*cells) + sizeof(*src_offsets);

/* Allocate memory for the emit data */
if (!(emit_data = loader_malloc(size, error_buf, error_buf_size)))
if ((size > 0)
&& !(emit_data = loader_malloc(size, error_buf, error_buf_size)))
return false;

cells = emit_data;
src_offsets = (int16 *)(cells + param_count);

if (is_if_block)
condition_offset = *loader_ctx->frame_offset;

/* POP original parameter out */
for (i = 0; i < param_count; i++) {
POP_OFFSET_TYPE(wasm_type->types[param_count - i - 1]);
wasm_loader_emit_backspace(loader_ctx, sizeof(int16));
}
frame_offset = loader_ctx->frame_offset;

/* Get each param's cell num and src offset */
for (i = 0; i < param_count; i++) {
for (i = 0; i < available_param_count; i++) {
cell = (uint8)wasm_value_type_cell_num(wasm_type->types[i]);
cells[i] = cell;
src_offsets[i] = *frame_offset;
Expand All @@ -6915,34 +6933,41 @@ copy_params_to_dynamic_space(WASMLoaderContext *loader_ctx, bool is_if_block,
/* emit copy instruction */
emit_label(EXT_OP_COPY_STACK_VALUES);
/* Part a) */
emit_uint32(loader_ctx, is_if_block ? param_count + 1 : param_count);
emit_uint32(loader_ctx, is_if_block ? available_param_count + 1
: available_param_count);
/* Part b) */
emit_uint32(loader_ctx, is_if_block ? wasm_type->param_cell_num + 1
: wasm_type->param_cell_num);
/* Part c) */
for (i = 0; i < param_count; i++)
for (i = 0; i < available_param_count; i++)
emit_byte(loader_ctx, cells[i]);
if (is_if_block)
emit_byte(loader_ctx, 1);

/* Part d) */
for (i = 0; i < param_count; i++)
for (i = 0; i < available_param_count; i++)
emit_operand(loader_ctx, src_offsets[i]);
if (is_if_block)
emit_operand(loader_ctx, condition_offset);

/* Part e) */
/* Push to dynamic space. The push will emit the dst offset. */
for (i = 0; i < param_count; i++)
for (i = 0; i < available_param_count; i++)
PUSH_OFFSET_TYPE(wasm_type->types[i]);
if (is_if_block)
PUSH_OFFSET_TYPE(VALUE_TYPE_I32);

if (p_available_param_count) {
*p_available_param_count = available_param_count;
}

ret = true;

fail:
/* Free the emit data */
wasm_runtime_free(emit_data);
if (emit_data) {
wasm_runtime_free(emit_data);
}

return ret;
}
Expand Down Expand Up @@ -7070,7 +7095,7 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
uint8 *func_const_end, *func_const = NULL;
int16 operand_offset = 0;
uint8 last_op = 0;
bool disable_emit, preserve_local = false;
bool disable_emit, preserve_local = false, if_condition_available = true;
float32 f32_const;
float64 f64_const;

Expand Down Expand Up @@ -7140,11 +7165,24 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
break;

case WASM_OP_IF:
{
#if WASM_ENABLE_FAST_INTERP != 0
BranchBlock *parent_block = loader_ctx->frame_csp - 1;
int32 available_stack_cell =
(int32)(loader_ctx->stack_cell_num
- parent_block->stack_cell_num);

if (available_stack_cell <= 0
&& parent_block->is_stack_polymorphic)
if_condition_available = false;
else
if_condition_available = true;

PRESERVE_LOCAL_FOR_BLOCK();
#endif
POP_I32();
goto handle_op_block_and_loop;
}
case WASM_OP_BLOCK:
case WASM_OP_LOOP:
#if WASM_ENABLE_FAST_INTERP != 0
Expand All @@ -7154,6 +7192,9 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
{
uint8 value_type;
BlockType block_type;
#if WASM_ENABLE_FAST_INTERP != 0
uint32 available_params = 0;
#endif

p_org = p - 1;
CHECK_BUF(p, p_end, 1);
Expand Down Expand Up @@ -7195,35 +7236,63 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
/* Pop block parameters from stack */
if (BLOCK_HAS_PARAM(block_type)) {
WASMType *wasm_type = block_type.u.type;
for (i = 0; i < block_type.u.type->param_count; i++)

BranchBlock *cur_block = loader_ctx->frame_csp - 1;
#if WASM_ENABLE_FAST_INTERP != 0
available_params = block_type.u.type->param_count;
#endif
for (i = 0; i < block_type.u.type->param_count; i++) {

int32 available_stack_cell =
(int32)(loader_ctx->stack_cell_num
- cur_block->stack_cell_num);
if (available_stack_cell <= 0
&& cur_block->is_stack_polymorphic) {
#if WASM_ENABLE_FAST_INTERP != 0
available_params = i;
#endif
break;
}

POP_TYPE(
wasm_type->types[wasm_type->param_count - i - 1]);
}
}

PUSH_CSP(LABEL_TYPE_BLOCK + (opcode - WASM_OP_BLOCK),
block_type, p);

/* Pass parameters to block */
if (BLOCK_HAS_PARAM(block_type)) {
for (i = 0; i < block_type.u.type->param_count; i++)
for (i = 0; i < block_type.u.type->param_count; i++) {
PUSH_TYPE(block_type.u.type->types[i]);
#if WASM_ENABLE_FAST_INTERP != 0
if (i >= available_params) {
PUSH_OFFSET_TYPE(block_type.u.type->types[i]);
}
#endif
}
}

#if WASM_ENABLE_FAST_INTERP != 0
if (opcode == WASM_OP_BLOCK || opcode == WASM_OP_LOOP) {
skip_label();

if (BLOCK_HAS_PARAM(block_type)) {
/* Make sure params are in dynamic space */
if (!copy_params_to_dynamic_space(
loader_ctx, false, error_buf, error_buf_size))
if (!copy_params_to_dynamic_space(loader_ctx, false,
NULL, error_buf,
error_buf_size))
goto fail;
}

if (opcode == WASM_OP_LOOP) {
(loader_ctx->frame_csp - 1)->code_compiled =
loader_ctx->p_code_compiled;
}
}
else if (opcode == WASM_OP_IF) {
BranchBlock *block = loader_ctx->frame_csp - 1;
/* If block has parameters, we should make sure they are in
* dynamic space. Otherwise, when else branch is missing,
* the later opcode may consume incorrect operand offset.
Expand All @@ -7241,8 +7310,7 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
* recover them before entering else branch.
*
*/
if (BLOCK_HAS_PARAM(block_type)) {
BranchBlock *block = loader_ctx->frame_csp - 1;
if (if_condition_available && BLOCK_HAS_PARAM(block_type)) {
uint64 size;

/* skip the if condition operand offset */
Expand All @@ -7251,7 +7319,8 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
skip_label();
/* Emit a copy instruction */
if (!copy_params_to_dynamic_space(
loader_ctx, true, error_buf, error_buf_size))
loader_ctx, true, &block->available_param_num,
error_buf, error_buf_size))
goto fail;

/* Emit the if instruction */
Expand All @@ -7272,6 +7341,9 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
- size / sizeof(int16),
(uint32)size);
}
else {
block->available_param_num = 0;
}

emit_empty_label_addr_and_frame_ip(PATCH_ELSE);
emit_empty_label_addr_and_frame_ip(PATCH_END);
Expand All @@ -7282,7 +7354,8 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,

case WASM_OP_ELSE:
{
BlockType block_type = (loader_ctx->frame_csp - 1)->block_type;
BranchBlock *block = NULL;
BlockType block_type;

if (loader_ctx->csp_num < 2
|| (loader_ctx->frame_csp - 1)->label_type
Expand All @@ -7292,13 +7365,15 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
"opcode else found without matched opcode if");
goto fail;
}
block = loader_ctx->frame_csp - 1;

/* check whether if branch's stack matches its result type */
if (!check_block_stack(loader_ctx, loader_ctx->frame_csp - 1,
error_buf, error_buf_size))
if (!check_block_stack(loader_ctx, block, error_buf,
error_buf_size))
goto fail;

(loader_ctx->frame_csp - 1)->else_addr = p - 1;
block->else_addr = p - 1;
block_type = block->block_type;

#if WASM_ENABLE_FAST_INTERP != 0
/* if the result of if branch is in local or const area, add a
Expand All @@ -7319,10 +7394,9 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,

#if WASM_ENABLE_FAST_INTERP != 0
/* Recover top param_count values of frame_offset stack */
if (BLOCK_HAS_PARAM((block_type))) {
if (block->available_param_num) {
uint32 size;
BranchBlock *block = loader_ctx->frame_csp - 1;
size = sizeof(int16) * block_type.u.type->param_cell_num;
size = sizeof(int16) * block->available_param_num;
bh_memcpy_s(loader_ctx->frame_offset, size,
block->param_frame_offsets, size);
loader_ctx->frame_offset += (size / sizeof(int16));
Expand Down Expand Up @@ -8385,8 +8459,6 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
- module->import_global_count]
.type;

POP_TYPE(global_type);

#if WASM_ENABLE_FAST_INTERP == 0
if (global_type == VALUE_TYPE_I64
|| global_type == VALUE_TYPE_F64) {
Expand Down Expand Up @@ -8425,6 +8497,9 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
emit_uint32(loader_ctx, global_idx);
POP_OFFSET_TYPE(global_type);
#endif /* end of WASM_ENABLE_FAST_INTERP */

POP_TYPE(global_type);

break;
}

Expand Down
Loading

0 comments on commit f96257b

Please sign in to comment.