Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parse block type with leb i32 encoding and correct dst dynamic offset for opcode br #3482

Merged
merged 2 commits into from
May 31, 2024

Conversation

wenyongh
Copy link
Contributor

Fix several issues in wasm loader:

  • Parse a block's type index with leb int32 instead leb uint32
  • Correct dst dynamic offset of loop block arguments for opcode br
    when copying the stack operands to the arguments of loop block
  • Free each frame_csp's param_frame_offsets when destroy loader ctx
  • Fix compilation error in wasm_mini_loader.c
  • Add test cases of failed issues

This PR fixes issue #3467 and #3468.

@@ -8116,6 +8134,8 @@ wasm_loader_ctx_destroy(WASMLoaderContext *ctx)
if (ctx->frame_csp_bottom) {
#if WASM_ENABLE_FAST_INTERP != 0
free_all_label_patch_lists(ctx->frame_csp_bottom, ctx->csp_num);
free_all_label_param_frame_offsets(ctx->frame_csp_bottom,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wasm_loader_pop_frame_csp will also free the param_frame_offsets, should we set it to NULL in wasm_loader_pop_frame_csp?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wasm_loader_push_frame_csp will memset the frame csp with 0, seems no need to set param_frame_offsets to NULL again:

static bool
wasm_loader_push_frame_csp(WASMLoaderContext *ctx, uint8 label_type,
BlockType block_type, uint8 *start_addr,
char *error_buf, uint32 error_buf_size)
{
CHECK_CSP_PUSH();
memset(ctx->frame_csp, 0, sizeof(BranchBlock));

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks!

Copy link
Collaborator

@xujuntwt95329 xujuntwt95329 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wenyongh wenyongh merged commit 23e1d51 into bytecodealliance:main May 31, 2024
377 checks passed
@wenyongh wenyongh deleted the fix_loader_issues branch May 31, 2024 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants