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.

Sign up for GitHub

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

Jump to bottom

Fix fast-interp "pre-compiled label offset out of range" issue #2659

Merged
merged 2 commits into from
Oct 24, 2023
Merged

Fix fast-interp "pre-compiled label offset out of range" issue #2659

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eeabe37
Fix fast-interp label offset out of range issue
wenyongh Oct 22, 2023
c48f17b
change offset to label_addr
wenyongh Oct 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 21 additions & 6 deletions core/iwasm/interpreter/wasm_interp_fast.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1128,12 +1128,27 @@ wasm_interp_dump_op_count()
goto *p_label_addr; \
} while (0)
#else
#define FETCH_OPCODE_AND_DISPATCH() \
do { \
const void *p_label_addr = label_base + *(int16 *)frame_ip; \
frame_ip += sizeof(int16); \
goto *p_label_addr; \
#if UINTPTR_MAX == UINT64_MAX
#define FETCH_OPCODE_AND_DISPATCH() \
do { \
const void *p_label_addr; \
bh_assert(((uintptr_t)frame_ip & 1) == 0); \
Copy link
Collaborator

Choose a reason for hiding this comment

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

should & 3?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the frame_ip must be 2-byte aligned, but not necessarily 4-byte aligned. If we force the frame_ip to be 4-byte aligned in wasm_loader, we need to add padding byte if needed, and here, we need to add code like frame_ip = align_uint(frame_ip, 4) or if (frame_ip & 2) frame_ip += 2, I had a try, it impacts performance a lot.

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!

I originally thought the CPU requires 4-bytes align, if 2-bytes align is OK, then no problem here.

/* int32 relative offset was emitted in 64-bit target */ \
p_label_addr = label_base + (int32)LOAD_U32_WITH_2U16S(frame_ip); \
frame_ip += sizeof(int32); \
goto *p_label_addr; \
} while (0)
#else
#define FETCH_OPCODE_AND_DISPATCH() \
do { \
const void *p_label_addr; \
bh_assert(((uintptr_t)frame_ip & 1) == 0); \
/* uint32 label address was emitted in 32-bit target */ \
p_label_addr = (void *)(uintptr_t)LOAD_U32_WITH_2U16S(frame_ip); \
frame_ip += sizeof(int32); \
goto *p_label_addr; \
} while (0)
#endif
#endif /* end of WASM_CPU_SUPPORTS_UNALIGNED_ADDR_ACCESS */
#define HANDLE_OP_END() FETCH_OPCODE_AND_DISPATCH()

Expand Down Expand Up @@ -1183,7 +1198,7 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module,
register uint8 *frame_ip = &opcode_IMPDEP; /* cache of frame->ip */
register uint32 *frame_lp = NULL; /* cache of frame->lp */
#if WASM_ENABLE_LABELS_AS_VALUES != 0
#if WASM_CPU_SUPPORTS_UNALIGNED_ADDR_ACCESS == 0
#if WASM_CPU_SUPPORTS_UNALIGNED_ADDR_ACCESS == 0 && UINTPTR_MAX == UINT64_MAX
/* cache of label base addr */
register uint8 *label_base = &&HANDLE_WASM_OP_UNREACHABLE;
#endif
Expand Down
68 changes: 36 additions & 32 deletions core/iwasm/interpreter/wasm_loader.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5389,21 +5389,27 @@ wasm_loader_pop_frame_csp(WASMLoaderContext *ctx, char *error_buf,
LOG_OP("\ndelete last op\n"); \
} while (0)
#else /* else of WASM_CPU_SUPPORTS_UNALIGNED_ADDR_ACCESS */
#if UINTPTR_MAX == UINT64_MAX
#define emit_label(opcode) \
do { \
int32 offset = \
(int32)((uint8 *)handle_table[opcode] - (uint8 *)handle_table[0]); \
if (!(offset >= INT16_MIN && offset < INT16_MAX)) { \
set_error_buf(error_buf, error_buf_size, \
"pre-compiled label offset out of range"); \
goto fail; \
} \
wasm_loader_emit_int16(loader_ctx, offset); \
/* emit int32 relative offset in 64-bit target */ \
wasm_loader_emit_uint32(loader_ctx, offset); \
LOG_OP("\nemit_op [%02x]\t", opcode); \
} while (0)
#else
#define emit_label(opcode) \
do { \
uint32 offset = (uint32)(uintptr_t)handle_table[opcode]; \
Copy link
Collaborator

Choose a reason for hiding this comment

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

had better rename offset to position or label_addr?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks.

/* emit uint32 label address in 32-bit target */ \
wasm_loader_emit_uint32(loader_ctx, offset); \
LOG_OP("\nemit_op [%02x]\t", opcode); \
} while (0)
#endif
#define skip_label() \
do { \
wasm_loader_emit_backspace(loader_ctx, sizeof(int16)); \
wasm_loader_emit_backspace(loader_ctx, sizeof(int32)); \
LOG_OP("\ndelete last op\n"); \
} while (0)
#endif /* end of WASM_CPU_SUPPORTS_UNALIGNED_ADDR_ACCESS */
Expand Down Expand Up @@ -5733,12 +5739,6 @@ preserve_referenced_local(WASMLoaderContext *loader_ctx, uint8 opcode,
(void)error_buf;
(void)error_buf_size;
return true;
#if WASM_ENABLE_LABELS_AS_VALUES != 0
#if WASM_CPU_SUPPORTS_UNALIGNED_ADDR_ACCESS == 0
fail:
return false;
#endif
#endif
}

static bool
Expand Down Expand Up @@ -7793,6 +7793,9 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
uint8 ref_type;
BranchBlock *cur_block = loader_ctx->frame_csp - 1;
int32 available_stack_cell;
#if WASM_ENABLE_FAST_INTERP != 0
uint8 *p_code_compiled_tmp = loader_ctx->p_code_compiled;
#endif

POP_I32();

Expand Down Expand Up @@ -7821,26 +7824,26 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
#if WASM_ENABLE_FAST_INTERP != 0
if (loader_ctx->p_code_compiled) {
uint8 opcode_tmp = WASM_OP_SELECT_64;
uint8 *p_code_compiled_tmp =
loader_ctx->p_code_compiled - 2;
#if WASM_ENABLE_LABELS_AS_VALUES != 0
#if WASM_CPU_SUPPORTS_UNALIGNED_ADDR_ACCESS != 0
*(void **)(p_code_compiled_tmp
- sizeof(void *)) =
handle_table[opcode_tmp];
#else
#if UINTPTR_MAX == UINT64_MAX
/* emit int32 relative offset in 64-bit target
*/
int32 offset =
(int32)((uint8 *)handle_table[opcode_tmp]
- (uint8 *)handle_table[0]);
if (!(offset >= INT16_MIN
&& offset < INT16_MAX)) {
set_error_buf(error_buf, error_buf_size,
"pre-compiled label offset "
"out of range");
goto fail;
}
*(int16 *)(p_code_compiled_tmp
- sizeof(int16)) = (int16)offset;
*(int32 *)(p_code_compiled_tmp
- sizeof(int32)) = offset;
#else
/* emit uint32 label address in 32-bit target */
*(uint32 *)(p_code_compiled_tmp
- sizeof(uint32)) =
(uint32)(uintptr_t)handle_table[opcode_tmp];
#endif
#endif /* end of WASM_CPU_SUPPORTS_UNALIGNED_ADDR_ACCESS */
#else /* else of WASM_ENABLE_LABELS_AS_VALUES */
#if WASM_CPU_SUPPORTS_UNALIGNED_ADDR_ACCESS != 0
Expand Down Expand Up @@ -7934,16 +7937,17 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
*(void **)(p_code_compiled_tmp - sizeof(void *)) =
handle_table[opcode_tmp];
#else
#if UINTPTR_MAX == UINT64_MAX
/* emit int32 relative offset in 64-bit target */
int32 offset = (int32)((uint8 *)handle_table[opcode_tmp]
- (uint8 *)handle_table[0]);
if (!(offset >= INT16_MIN && offset < INT16_MAX)) {
set_error_buf(
error_buf, error_buf_size,
"pre-compiled label offset out of range");
goto fail;
}
*(int16 *)(p_code_compiled_tmp - sizeof(int16)) =
(int16)offset;
*(int32 *)(p_code_compiled_tmp - sizeof(int32)) =
offset;
#else
/* emit uint32 label address in 32-bit target */
*(uint32 *)(p_code_compiled_tmp - sizeof(uint32)) =
(uint32)(uintptr_t)handle_table[opcode_tmp];
#endif
#endif /* end of WASM_CPU_SUPPORTS_UNALIGNED_ADDR_ACCESS */
#else /* else of WASM_ENABLE_LABELS_AS_VALUES */
#if WASM_CPU_SUPPORTS_UNALIGNED_ADDR_ACCESS != 0
Expand Down
67 changes: 35 additions & 32 deletions core/iwasm/interpreter/wasm_mini_loader.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4009,21 +4009,27 @@ wasm_loader_pop_frame_csp(WASMLoaderContext *ctx, char *error_buf,
LOG_OP("\ndelete last op\n"); \
} while (0)
#else /* else of WASM_CPU_SUPPORTS_UNALIGNED_ADDR_ACCESS */
#if UINTPTR_MAX == UINT64_MAX
#define emit_label(opcode) \
do { \
int32 offset = \
(int32)((uint8 *)handle_table[opcode] - (uint8 *)handle_table[0]); \
if (!(offset >= INT16_MIN && offset < INT16_MAX)) { \
set_error_buf(error_buf, error_buf_size, \
"pre-compiled label offset out of range"); \
goto fail; \
} \
wasm_loader_emit_int16(loader_ctx, offset); \
/* emit int32 relative offset in 64-bit target */ \
wasm_loader_emit_uint32(loader_ctx, offset); \
LOG_OP("\nemit_op [%02x]\t", opcode); \
} while (0)
#else
#define emit_label(opcode) \
do { \
uint32 offset = (uint32)(uintptr_t)handle_table[opcode]; \
/* emit uint32 label address in 32-bit target */ \
wasm_loader_emit_uint32(loader_ctx, offset); \
LOG_OP("\nemit_op [%02x]\t", opcode); \
} while (0)
#endif
#define skip_label() \
do { \
wasm_loader_emit_backspace(loader_ctx, sizeof(int16)); \
wasm_loader_emit_backspace(loader_ctx, sizeof(int32)); \
LOG_OP("\ndelete last op\n"); \
} while (0)
#endif /* end of WASM_CPU_SUPPORTS_UNALIGNED_ADDR_ACCESS */
Expand Down Expand Up @@ -4351,13 +4357,6 @@ preserve_referenced_local(WASMLoaderContext *loader_ctx, uint8 opcode,
}

return true;

#if WASM_ENABLE_LABELS_AS_VALUES != 0
#if WASM_CPU_SUPPORTS_UNALIGNED_ADDR_ACCESS == 0
fail:
return false;
#endif
#endif
}

static bool
Expand Down Expand Up @@ -6146,6 +6145,9 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
uint8 ref_type;
BranchBlock *cur_block = loader_ctx->frame_csp - 1;
int32 available_stack_cell;
#if WASM_ENABLE_FAST_INTERP != 0
uint8 *p_code_compiled_tmp = loader_ctx->p_code_compiled;
#endif

POP_I32();

Expand All @@ -6168,26 +6170,26 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
#if WASM_ENABLE_FAST_INTERP != 0
if (loader_ctx->p_code_compiled) {
uint8 opcode_tmp = WASM_OP_SELECT_64;
uint8 *p_code_compiled_tmp =
loader_ctx->p_code_compiled - 2;
#if WASM_ENABLE_LABELS_AS_VALUES != 0
#if WASM_CPU_SUPPORTS_UNALIGNED_ADDR_ACCESS != 0
*(void **)(p_code_compiled_tmp
- sizeof(void *)) =
handle_table[opcode_tmp];
#else
#if UINTPTR_MAX == UINT64_MAX
/* emit int32 relative offset in 64-bit target
*/
int32 offset =
(int32)((uint8 *)handle_table[opcode_tmp]
- (uint8 *)handle_table[0]);
if (!(offset >= INT16_MIN
&& offset < INT16_MAX)) {
set_error_buf(error_buf, error_buf_size,
"pre-compiled label offset "
"out of range");
goto fail;
}
*(int16 *)(p_code_compiled_tmp
- sizeof(int16)) = (int16)offset;
*(int32 *)(p_code_compiled_tmp
- sizeof(int32)) = offset;
#else
/* emit uint32 label address in 32-bit target */
*(uint32 *)(p_code_compiled_tmp
- sizeof(uint32)) =
(uint32)(uintptr_t)handle_table[opcode_tmp];
#endif
#endif /* end of WASM_CPU_SUPPORTS_UNALIGNED_ADDR_ACCESS */
#else /* else of WASM_ENABLE_LABELS_AS_VALUES */
#if WASM_CPU_SUPPORTS_UNALIGNED_ADDR_ACCESS != 0
Expand Down Expand Up @@ -6263,15 +6265,16 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
*(void **)(p_code_compiled_tmp - sizeof(void *)) =
handle_table[opcode_tmp];
#else
#if UINTPTR_MAX == UINT64_MAX
/* emit int32 relative offset in 64-bit target */
int32 offset = (int32)((uint8 *)handle_table[opcode_tmp]
- (uint8 *)handle_table[0]);
if (!(offset >= INT16_MIN && offset < INT16_MAX)) {
set_error_buf(error_buf, error_buf_size,
"pre-compiled label offset out of range");
goto fail;
}
*(int16 *)(p_code_compiled_tmp - sizeof(int16)) =
(int16)offset;
*(int32 *)(p_code_compiled_tmp - sizeof(int32)) = offset;
#else
/* emit uint32 label address in 32-bit target */
*(uint32 *)(p_code_compiled_tmp - sizeof(uint32)) =
(uint32)(uintptr_t)handle_table[opcode_tmp];
#endif
#endif /* end of WASM_CPU_SUPPORTS_UNALIGNED_ADDR_ACCESS */
#else /* else of WASM_ENABLE_LABELS_AS_VALUES */
#if WASM_CPU_SUPPORTS_UNALIGNED_ADDR_ACCESS != 0
Expand Down