From f96257badef5a0289867710c4ee213617d805bb5 Mon Sep 17 00:00:00 2001 From: Xu Jun Date: Thu, 4 Jan 2024 10:00:36 +0800 Subject: [PATCH] Fix fast-interp polymorphic stack processing (#2974) Fix issue #2951, #2952 and #2953. --- core/iwasm/interpreter/wasm_loader.c | 151 +++++++++++++++------ core/iwasm/interpreter/wasm_mini_loader.c | 153 ++++++++++++++++------ 2 files changed, 225 insertions(+), 79 deletions(-) diff --git a/core/iwasm/interpreter/wasm_loader.c b/core/iwasm/interpreter/wasm_loader.c index 4098d1af..03530702 100644 --- a/core/iwasm/interpreter/wasm_loader.c +++ b/core/iwasm/interpreter/wasm_loader.c @@ -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. @@ -6857,15 +6860,18 @@ fail: * 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; @@ -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 = (uint64)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))) - return false; - - cells = emit_data; - src_offsets = (int16 *)(cells + param_count); + 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); + + 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 ((size > 0) + && !(emit_data = loader_malloc(size, error_buf, error_buf_size))) + return false; + + cells = emit_data; + src_offsets = (int16 *)(cells + param_count); + 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; @@ -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; } @@ -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; @@ -7140,11 +7165,24 @@ re_scan: 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 @@ -7154,6 +7192,9 @@ re_scan: { 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); @@ -7195,9 +7236,27 @@ re_scan: /* 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), @@ -7205,25 +7264,35 @@ re_scan: /* 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. @@ -7241,8 +7310,7 @@ re_scan: * 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 */ @@ -7251,7 +7319,8 @@ re_scan: 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 */ @@ -7272,6 +7341,9 @@ re_scan: - 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); @@ -7282,7 +7354,8 @@ re_scan: 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 @@ -7292,13 +7365,15 @@ re_scan: "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 @@ -7319,10 +7394,9 @@ re_scan: #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)); @@ -8385,8 +8459,6 @@ re_scan: - 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) { @@ -8425,6 +8497,9 @@ re_scan: emit_uint32(loader_ctx, global_idx); POP_OFFSET_TYPE(global_type); #endif /* end of WASM_ENABLE_FAST_INTERP */ + + POP_TYPE(global_type); + break; } diff --git a/core/iwasm/interpreter/wasm_mini_loader.c b/core/iwasm/interpreter/wasm_mini_loader.c index 8262de82..4bbec3d1 100644 --- a/core/iwasm/interpreter/wasm_mini_loader.c +++ b/core/iwasm/interpreter/wasm_mini_loader.c @@ -3594,6 +3594,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. @@ -5343,16 +5346,20 @@ fail: * 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; uint8 *cells = NULL, cell; int16 *src_offsets = NULL; @@ -5362,35 +5369,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 = (uint64)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))) - return false; - - cells = emit_data; - src_offsets = (int16 *)(cells + param_count); + 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); + + 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 ((size > 0) + && !(emit_data = loader_malloc(size, error_buf, error_buf_size))) + return false; + + cells = emit_data; + src_offsets = (int16 *)(cells + param_count); + 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; @@ -5400,37 +5419,43 @@ 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); - /* Free the emit data */ - wasm_runtime_free(emit_data); - return true; + if (p_available_param_count) { + *p_available_param_count = available_param_count; + } + + ret = true; fail: /* Free the emit data */ - wasm_runtime_free(emit_data); - return false; + if (emit_data) { + wasm_runtime_free(emit_data); + } + + return ret; } #endif @@ -5497,7 +5522,8 @@ 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; @@ -5567,11 +5593,23 @@ re_scan: 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 @@ -5581,6 +5619,9 @@ re_scan: { uint8 value_type; BlockType block_type; +#if WASM_ENABLE_FAST_INTERP != 0 + uint32 available_params = 0; +#endif p_org = p - 1; value_type = read_uint8(p); @@ -5611,9 +5652,27 @@ re_scan: /* 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), @@ -5621,8 +5680,14 @@ re_scan: /* 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 @@ -5630,8 +5695,9 @@ re_scan: 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) { @@ -5640,6 +5706,7 @@ re_scan: } } 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. @@ -5657,8 +5724,7 @@ re_scan: * 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 */ @@ -5667,7 +5733,8 @@ re_scan: 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 */ @@ -5688,6 +5755,9 @@ re_scan: - 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); @@ -5698,17 +5768,19 @@ re_scan: case WASM_OP_ELSE: { + BranchBlock *block = NULL; BlockType block_type = (loader_ctx->frame_csp - 1)->block_type; bh_assert(loader_ctx->csp_num >= 2 && (loader_ctx->frame_csp - 1)->label_type == LABEL_TYPE_IF); + 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; #if WASM_ENABLE_FAST_INTERP != 0 /* if the result of if branch is in local or const area, add a @@ -5729,10 +5801,9 @@ re_scan: #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)); @@ -6650,8 +6721,6 @@ re_scan: - module->import_global_count] .type; - POP_TYPE(global_type); - #if WASM_ENABLE_FAST_INTERP == 0 if (is_64bit_type(global_type)) { *p_org = WASM_OP_SET_GLOBAL_64; @@ -6677,6 +6746,8 @@ re_scan: POP_OFFSET_TYPE(global_type); #endif /* end of WASM_ENABLE_FAST_INTERP */ + POP_TYPE(global_type); + (void)is_mutable; break; }