fast-interp: Fix block with parameter in polymorphic stack issue (#3112)

The issue was reported in https://github.com/bytecodealliance/wasm-micro-runtime/issues/3061.

core/iwasm/interpreter/wasm_loader.c

@@ -5280,9 +5280,8 @@ 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;
+    /* This is used to recover dynamic offset for else branch */
+    uint16 start_dynamic_offset;
 #endif
 
     /* Indicate the operand stack is in polymorphic state.
@@ -7219,18 +7218,15 @@ fail:
  * 1) POP original parameter out;
  * 2) Push and copy original values to dynamic space.
  * The copy instruction format:
- *   Part a: available param count
+ *   Part a: 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,
-                             uint32 *p_available_param_count, char *error_buf,
-                             uint32 error_buf_size)
+                             char *error_buf, uint32 error_buf_size)
 {
     bool ret = false;
     int16 *frame_offset = NULL;
@@ -7242,91 +7238,72 @@ 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);
-
-        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));
+    uint64 size = (uint64)param_count * (sizeof(*cells) + sizeof(*src_offsets));
+    bh_assert(size > 0);
 
     /* 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)))
+    if (!(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 < available_param_count; i++) {
+    for (i = 0; i < param_count; i++) {
         cell = (uint8)wasm_value_type_cell_num(wasm_type->types[i]);
         cells[i] = cell;
         src_offsets[i] = *frame_offset;
         frame_offset += cell;
     }
-
     /* emit copy instruction */
     emit_label(EXT_OP_COPY_STACK_VALUES);
     /* Part a) */
-    emit_uint32(loader_ctx, is_if_block ? available_param_count + 1
-                                        : available_param_count);
+    emit_uint32(loader_ctx, is_if_block ? param_count + 1 : 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 < available_param_count; i++)
+    for (i = 0; i < param_count; i++)
         emit_byte(loader_ctx, cells[i]);
     if (is_if_block)
         emit_byte(loader_ctx, 1);
 
     /* Part d) */
-    for (i = 0; i < available_param_count; i++)
+    for (i = 0; i < 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 < available_param_count; i++)
+    for (i = 0; i < 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 */
-    if (emit_data) {
-        wasm_runtime_free(emit_data);
-    }
+    wasm_runtime_free(emit_data);
 
     return ret;
 }
@@ -7616,6 +7593,7 @@ re_scan:
 
                     BranchBlock *cur_block = loader_ctx->frame_csp - 1;
 #if WASM_ENABLE_FAST_INTERP != 0
+                    uint32 cell_num;
                     available_params = block_type.u.type->param_count;
 #endif
                     for (i = 0; i < block_type.u.type->param_count; i++) {
@@ -7633,6 +7611,13 @@ re_scan:
 
                         POP_TYPE(
                             wasm_type->types[wasm_type->param_count - i - 1]);
+#if WASM_ENABLE_FAST_INTERP != 0
+                        /* decrease the frame_offset pointer accordingly to keep
+                         * consistent with frame_ref stack */
+                        cell_num = wasm_value_type_cell_num(
+                            wasm_type->types[wasm_type->param_count - i - 1]);
+                        loader_ctx->frame_offset -= cell_num;
+#endif
                     }
                 }
                 PUSH_CSP(LABEL_TYPE_BLOCK + (opcode - WASM_OP_BLOCK),
@@ -7641,12 +7626,26 @@ re_scan:
                 /* Pass parameters to block */
                 if (BLOCK_HAS_PARAM(block_type)) {
                     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
+                        uint32 cell_num = wasm_value_type_cell_num(
+                            block_type.u.type->types[i]);
                         if (i >= available_params) {
-                            PUSH_OFFSET_TYPE(block_type.u.type->types[i]);
+                            /* If there isn't enough data on stack, push a dummy
+                             * offset to keep the stack consistent with
+                             * frame_ref.
+                             * Since the stack is already in polymorphic state,
+                             * the opcode will not be executed, so the dummy
+                             * offset won't cause any error */
+                            *loader_ctx->frame_offset++ = 0;
+                            if (cell_num > 1) {
+                                *loader_ctx->frame_offset++ = 0;
+                            }
+                        }
+                        else {
+                            loader_ctx->frame_offset += cell_num;
                         }
 #endif
+                        PUSH_TYPE(block_type.u.type->types[i]);
                     }
                 }
 
@@ -7656,9 +7655,8 @@ re_scan:
 
                     if (BLOCK_HAS_PARAM(block_type)) {
                         /* Make sure params are in dynamic space */
-                        if (!copy_params_to_dynamic_space(loader_ctx, false,
-                                                          NULL, error_buf,
-                                                          error_buf_size))
+                        if (!copy_params_to_dynamic_space(
+                                loader_ctx, false, error_buf, error_buf_size))
                             goto fail;
                     }
 
@@ -7691,17 +7689,21 @@ re_scan:
                      * recover them before entering else branch.
                      *
                      */
-                    if (if_condition_available && BLOCK_HAS_PARAM(block_type)) {
+                    if (BLOCK_HAS_PARAM(block_type)) {
                         uint64 size;
 
-                        /* skip the if condition operand offset */
-                        wasm_loader_emit_backspace(loader_ctx, sizeof(int16));
+                        /* In polymorphic state, there may be no if condition on
+                         * the stack, so the offset may not emitted */
+                        if (if_condition_available) {
+                            /* skip the if condition operand offset */
+                            wasm_loader_emit_backspace(loader_ctx,
+                                                       sizeof(int16));
+                        }
                         /* skip the if label */
                         skip_label();
                         /* Emit a copy instruction */
                         if (!copy_params_to_dynamic_space(
-                                loader_ctx, true, &block->available_param_num,
-                                error_buf, error_buf_size))
+                                loader_ctx, true, error_buf, error_buf_size))
                             goto fail;
 
                         /* Emit the if instruction */
@@ -7722,9 +7724,8 @@ re_scan:
                                         - size / sizeof(int16),
                                     (uint32)size);
                     }
-                    else {
-                        block->available_param_num = 0;
-                    }
+
+                    block->start_dynamic_offset = loader_ctx->dynamic_offset;
 
                     emit_empty_label_addr_and_frame_ip(PATCH_ELSE);
                     emit_empty_label_addr_and_frame_ip(PATCH_END);
@@ -7980,30 +7981,13 @@ re_scan:
 
 #if WASM_ENABLE_FAST_INTERP != 0
                 /* Recover top param_count values of frame_offset stack */
-                if (block->available_param_num) {
-                    uint32 available_param_cell_num = 0;
-
-                    /* total cell num of available parameters */
-                    for (i = 0; i < block->available_param_num; i++) {
-                        uint32 cell_num = wasm_value_type_cell_num(
-                            block->block_type.u.type->types[i]);
-
-                        /* recover dynamic offset */
-                        if (block->param_frame_offsets[available_param_cell_num]
-                            >= loader_ctx->dynamic_offset) {
-                            loader_ctx->dynamic_offset =
-                                block->param_frame_offsets
-                                    [available_param_cell_num]
-                                + cell_num;
-                        }
-
-                        available_param_cell_num += cell_num;
-                    }
-
-                    bh_memcpy_s(
-                        loader_ctx->frame_offset, available_param_cell_num,
-                        block->param_frame_offsets, available_param_cell_num);
-                    loader_ctx->frame_offset += available_param_cell_num;
+                if (BLOCK_HAS_PARAM((block_type))) {
+                    uint32 size;
+                    size = sizeof(int16) * block_type.u.type->param_cell_num;
+                    bh_memcpy_s(loader_ctx->frame_offset, size,
+                                block->param_frame_offsets, size);
+                    loader_ctx->frame_offset += (size / sizeof(int16));
+                    loader_ctx->dynamic_offset = block->start_dynamic_offset;
                 }
 #endif
 

core/iwasm/interpreter/wasm_mini_loader.c

@@ -3614,6 +3614,8 @@ typedef struct BranchBlock {
     /* 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;
+    /* This is used to recover dynamic offset for else branch */
+    uint16 start_dynamic_offset;
 #endif
 
     /* Indicate the operand stack is in polymorphic state.
@@ -5363,18 +5365,15 @@ fail:
  * 1) POP original parameter out;
  * 2) Push and copy original values to dynamic space.
  * The copy instruction format:
- *   Part a: available param count
+ *   Part a: 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,
-                             uint32 *p_available_param_count, char *error_buf,
-                             uint32 error_buf_size)
+                             char *error_buf, uint32 error_buf_size)
 {
     bool ret = false;
     int16 *frame_offset = NULL;
@@ -5386,91 +5385,72 @@ 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);
-
-        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));
+    uint64 size = (uint64)param_count * (sizeof(*cells) + sizeof(*src_offsets));
+    bh_assert(size > 0);
 
     /* 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)))
+    if (!(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 < available_param_count; i++) {
+    for (i = 0; i < param_count; i++) {
         cell = (uint8)wasm_value_type_cell_num(wasm_type->types[i]);
         cells[i] = cell;
         src_offsets[i] = *frame_offset;
         frame_offset += cell;
     }
-
     /* emit copy instruction */
     emit_label(EXT_OP_COPY_STACK_VALUES);
     /* Part a) */
-    emit_uint32(loader_ctx, is_if_block ? available_param_count + 1
-                                        : available_param_count);
+    emit_uint32(loader_ctx, is_if_block ? param_count + 1 : 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 < available_param_count; i++)
+    for (i = 0; i < param_count; i++)
         emit_byte(loader_ctx, cells[i]);
     if (is_if_block)
         emit_byte(loader_ctx, 1);
 
     /* Part d) */
-    for (i = 0; i < available_param_count; i++)
+    for (i = 0; i < 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 < available_param_count; i++)
+    for (i = 0; i < 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 */
-    if (emit_data) {
-        wasm_runtime_free(emit_data);
-    }
+    wasm_runtime_free(emit_data);
 
     return ret;
 }
@@ -5672,6 +5652,7 @@ re_scan:
 
                     BranchBlock *cur_block = loader_ctx->frame_csp - 1;
 #if WASM_ENABLE_FAST_INTERP != 0
+                    uint32 cell_num;
                     available_params = block_type.u.type->param_count;
 #endif
                     for (i = 0; i < block_type.u.type->param_count; i++) {
@@ -5689,6 +5670,13 @@ re_scan:
 
                         POP_TYPE(
                             wasm_type->types[wasm_type->param_count - i - 1]);
+#if WASM_ENABLE_FAST_INTERP != 0
+                        /* decrease the frame_offset pointer accordingly to keep
+                         * consistent with frame_ref stack */
+                        cell_num = wasm_value_type_cell_num(
+                            wasm_type->types[wasm_type->param_count - i - 1]);
+                        loader_ctx->frame_offset -= cell_num;
+#endif
                     }
                 }
 
@@ -5698,12 +5686,26 @@ re_scan:
                 /* Pass parameters to block */
                 if (BLOCK_HAS_PARAM(block_type)) {
                     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
+                        uint32 cell_num = wasm_value_type_cell_num(
+                            block_type.u.type->types[i]);
                         if (i >= available_params) {
-                            PUSH_OFFSET_TYPE(block_type.u.type->types[i]);
+                            /* If there isn't enough data on stack, push a dummy
+                             * offset to keep the stack consistent with
+                             * frame_ref.
+                             * Since the stack is already in polymorphic state,
+                             * the opcode will not be executed, so the dummy
+                             * offset won't cause any error */
+                            *loader_ctx->frame_offset++ = 0;
+                            if (cell_num > 1) {
+                                *loader_ctx->frame_offset++ = 0;
+                            }
+                        }
+                        else {
+                            loader_ctx->frame_offset += cell_num;
                         }
 #endif
+                        PUSH_TYPE(block_type.u.type->types[i]);
                     }
                 }
 
@@ -5712,9 +5714,8 @@ 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,
-                                                          NULL, error_buf,
-                                                          error_buf_size))
+                        if (!copy_params_to_dynamic_space(
+                                loader_ctx, false, error_buf, error_buf_size))
                             goto fail;
                     }
                     if (opcode == WASM_OP_LOOP) {
@@ -5741,17 +5742,21 @@ re_scan:
                      * recover them before entering else branch.
                      *
                      */
-                    if (if_condition_available && BLOCK_HAS_PARAM(block_type)) {
+                    if (BLOCK_HAS_PARAM(block_type)) {
                         uint64 size;
 
-                        /* skip the if condition operand offset */
-                        wasm_loader_emit_backspace(loader_ctx, sizeof(int16));
+                        /* In polymorphic state, there may be no if condition on
+                         * the stack, so the offset may not emitted */
+                        if (if_condition_available) {
+                            /* skip the if condition operand offset */
+                            wasm_loader_emit_backspace(loader_ctx,
+                                                       sizeof(int16));
+                        }
                         /* skip the if label */
                         skip_label();
                         /* Emit a copy instruction */
                         if (!copy_params_to_dynamic_space(
-                                loader_ctx, true, &block->available_param_num,
-                                error_buf, error_buf_size))
+                                loader_ctx, true, error_buf, error_buf_size))
                             goto fail;
 
                         /* Emit the if instruction */
@@ -5772,9 +5777,8 @@ re_scan:
                                         - size / sizeof(int16),
                                     (uint32)size);
                     }
-                    else {
-                        block->available_param_num = 0;
-                    }
+
+                    block->start_dynamic_offset = loader_ctx->dynamic_offset;
 
                     emit_empty_label_addr_and_frame_ip(PATCH_ELSE);
                     emit_empty_label_addr_and_frame_ip(PATCH_END);
@@ -5818,30 +5822,13 @@ re_scan:
 
 #if WASM_ENABLE_FAST_INTERP != 0
                 /* Recover top param_count values of frame_offset stack */
-                if (block->available_param_num) {
-                    uint32 available_param_cell_num = 0;
-
-                    /* total cell num of available parameters */
-                    for (i = 0; i < block->available_param_num; i++) {
-                        uint32 cell_num = wasm_value_type_cell_num(
-                            block->block_type.u.type->types[i]);
-
-                        /* recover dynamic offset */
-                        if (block->param_frame_offsets[available_param_cell_num]
-                            >= loader_ctx->dynamic_offset) {
-                            loader_ctx->dynamic_offset =
-                                block->param_frame_offsets
-                                    [available_param_cell_num]
-                                + cell_num;
-                        }
-
-                        available_param_cell_num += cell_num;
-                    }
-
-                    bh_memcpy_s(
-                        loader_ctx->frame_offset, available_param_cell_num,
-                        block->param_frame_offsets, available_param_cell_num);
-                    loader_ctx->frame_offset += available_param_cell_num;
+                if (BLOCK_HAS_PARAM((block_type))) {
+                    uint32 size;
+                    size = sizeof(int16) * block_type.u.type->param_cell_num;
+                    bh_memcpy_s(loader_ctx->frame_offset, size,
+                                block->param_frame_offsets, size);
+                    loader_ctx->frame_offset += (size / sizeof(int16));
+                    loader_ctx->dynamic_offset = block->start_dynamic_offset;
                 }
 #endif