Fix bugs occurring in Instrument Test #1488
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wenyongh
reviewed
Sep 16, 2022
core/iwasm/fast-jit/jit_ir.c
Outdated
Comment on lines
383
to
393
| if (!(entry_block = jit_cc_new_basic_block(cc, 0)) | ||
| || !(exit_block = jit_cc_new_basic_block(cc, 0))) | ||
| goto fail; | ||
| goto fail2; | ||
|
|
||
| if (!(cc->exce_basic_blocks = | ||
| jit_calloc(sizeof(JitBasicBlock *) * JIT_EXCE_NUM))) | ||
| goto fail; | ||
| goto fail2; | ||
|
|
||
| if (!(cc->incoming_insns_for_exec_bbs = | ||
| jit_calloc(sizeof(JitIncomingInsnList) * JIT_EXCE_NUM))) | ||
| goto fail; | ||
| goto fail2; |
Collaborator
There was a problem hiding this comment.
The code looks very strange, as it goto upper failed label firstly, normally we should goto fail1 firstly but not goto fail2. How about changing to:
if (!(entry_block = jit_cc_new_basic_block(cc, 0)))
goto fail;
if (!(exit_block = jit_cc_new_basic_block(cc, 0))) {
jit_basic_block_delete(entry_block);
goto fail;
}
/* Record the entry and exit labels, whose indexes must be 0 and 1
respectively. */
cc->entry_label = jit_basic_block_label(entry_block);
cc->exit_label = jit_basic_block_label(exit_block);
bh_assert(jit_reg_no(cc->entry_label) == 0
&& jit_reg_no(cc->exit_label) == 1);
if (!(cc->exce_basic_blocks =
jit_calloc(sizeof(JitBasicBlock *) * JIT_EXCE_NUM)))
goto fail;
if (!(cc->incoming_insns_for_exec_bbs =
jit_calloc(sizeof(JitIncomingInsnList) * JIT_EXCE_NUM)))
goto fail;
Contributor
Author
There was a problem hiding this comment.
Ok, I have changed it.
| arg_regs[5] = argv; | ||
|
|
||
| if (!jit_emit_callnative(cc, jit_call_indirect, native_ret, arg_regs, 6)) { | ||
| if (jit_get_last_error(cc) |
Collaborator
There was a problem hiding this comment.
Not sure why we should add jit_get_last_error(cc) here, if not, which error will it cause?
Contributor
Author
There was a problem hiding this comment.
- Before calling
jit_emit_callnative, we callpack_argv(cc). - It may set error in function
pack_argv. - If we continue to call
jit_emit_callnativeafter callingpack_argv(cc)failed, it will causebh_assertfailed
Collaborator
There was a problem hiding this comment.
So had better move check into pack_argv:
static JitReg
pack_argv(JitCompContext *cc)
{
...
if (jit_get_last_error(cc))
return 0;
return argv;
}And add check when calling pack_argv:
if (!(argv = pack_argv(cc)))
goto fail;
Contributor
Author
There was a problem hiding this comment.
Thank you for you sugestion. I have changed it.
| GEN_INSN(CMP, cc->cmp_reg, native_ret, NEW_CONST(I32, 0)); | ||
| if (!jit_emit_exception(cc, JIT_EXCE_ALREADY_THROWN, JIT_OP_BEQ, | ||
| cc->cmp_reg, NULL)) { | ||
| if (jit_get_last_error(cc) |
Contributor
Author
There was a problem hiding this comment.
Thank you. This code has been deleted and found unnecessary.
and fix bh_assert failed in jit_emit_function.c
vickiegpt
pushed a commit
to vickiegpt/wamr-aot-gc-checkpoint-restore
that referenced
this pull request
May 27, 2024
- Add checks for `pack_argv` - Fix the checks in creating cc entry/exit basic blocks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[1] In function
jit_compile_op_call_indirect: ignoring to detect error after generate instruction or registerhttps://github.com/wasm-micro-runtime/wamr-internal-dev/issues/423
[2] In function
jit_cc_init: ingore to free entry_block and exit_blockhttps://github.com/wasm-micro-runtime/wamr-internal-dev/issues/422