Skip to content

Commit

Permalink
Fixed break instruction in a try-catch block.
Browse files Browse the repository at this point in the history
Previously, JUMP offset for a break instruction inside a try-catch
block was not set to a correct offset during code generation
when a return instruction was present in inner try-catch block.

The fix is to update the JUMP offset appropriately.

This closes #553 issue on Github.
  • Loading branch information
xeioex committed Jun 29, 2022
1 parent 6886970 commit 4045538
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 14 deletions.
41 changes: 39 additions & 2 deletions src/njs_generator.c
Original file line number Diff line number Diff line change
Expand Up @@ -4495,6 +4495,11 @@ njs_generate_try_catch(njs_vm_t *vm, njs_generator_t *generator,
NJS_GENERATOR_ALL,
&ctx->try_exit_label);

/*
* block can be NULL when &ctx->try_exit_label is "@return"
* for outermost try-catch block.
*/

if (block != NULL) {
patch = njs_generate_make_exit_patch(vm, block,
&ctx->try_exit_label,
Expand All @@ -4503,6 +4508,26 @@ njs_generate_try_catch(njs_vm_t *vm, njs_generator_t *generator,
if (njs_slow_path(patch == NULL)) {
return NJS_ERROR;
}

} else {

/*
* when block == NULL, we still want to patch the "finally"
* instruction break_offset.
*/

block = njs_generate_find_block(vm, generator->block,
NJS_GENERATOR_ALL,
&no_label);

if (block != NULL) {
patch = njs_generate_make_exit_patch(vm, block, &no_label,
njs_code_offset(generator, finally)
+ offsetof(njs_vmcode_finally_t, break_offset));
if (njs_slow_path(patch == NULL)) {
return NJS_ERROR;
}
}
}
}
}
Expand Down Expand Up @@ -4669,15 +4694,27 @@ njs_generate_try_end(njs_vm_t *vm, njs_generator_t *generator,
* outermost try-catch block.
*/
block = njs_generate_find_block(vm, generator->block,
NJS_GENERATOR_ALL
| NJS_GENERATOR_TRY, dest_label);
NJS_GENERATOR_ALL, dest_label);
if (block != NULL) {
patch = njs_generate_make_exit_patch(vm, block, dest_label,
njs_code_offset(generator, finally)
+ offsetof(njs_vmcode_finally_t, break_offset));
if (njs_slow_path(patch == NULL)) {
return NJS_ERROR;
}

} else {

block = njs_generate_find_block(vm, generator->block,
NJS_GENERATOR_ALL, &no_label);
if (block != NULL) {
patch = njs_generate_make_exit_patch(vm, block, &no_label,
njs_code_offset(generator, finally)
+ offsetof(njs_vmcode_finally_t, break_offset));
if (njs_slow_path(patch == NULL)) {
return NJS_ERROR;
}
}
}
}

Expand Down
56 changes: 44 additions & 12 deletions src/test/njs_unit_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -3411,20 +3411,52 @@ static njs_unit_test_t njs_test[] =
njs_str("a,2") },

{ njs_str("function f(n) { "
" var r1 = 0, r2 = 0, r3 = 0;"
" a:{ try { try { "
" if (n == 0) { break a; } "
" if (n == 1) { throw 'a'; } "
" } "
" catch (e) { break a; } finally { r1++; } } "
" catch (e) {} "
" finally { r2++; } "
" r3++; "
" }; "
"return [r1, r2, r3]"
"}; njs.dump([f(0), f(1), f(3)])"),
" var r1 = 0, r2 = 0, r3 = 0;"
" a:{ try { try { "
" if (n == 0) { break a; } "
" if (n == 1) { throw 'a'; } "
" } "
" catch (e) { break a; } finally { r1++; } } "
" catch (e) {} "
" finally { r2++; } "
" r3++; "
" }; "
"return [r1, r2, r3]"
"}; njs.dump([f(0), f(1), f(3)])"),
njs_str("[[1,1,0],[1,1,0],[1,1,1]]") },


{ njs_str("function f(n) {"
" while (1)"
" try {"
" if (n == 0) { break; }"
" if (n == 1) { throw 'a'; }"
""
" try { return 42; }"
" catch (a) {}"
""
" } catch (b) { return b; }"
"};"
"njs.dump([f(0), f(1), f(2)])"),
njs_str("[undefined,'a',42]") },

{ njs_str("function f(n, r) {"
" while (1)"
" try {"
" if (n == 0) { break; }"
" if (n == 1) { throw 'a'; }"
""
" try { return 42; }"
" catch (a) {}"
" finally { r.push('in');}"
""
" } catch (b) { return b; }"
" finally { r.push('out'); }"
"};"
"function g(n) { var r = []; return [f(n, r), r]}"
"njs.dump([g(0), g(1), g(2)])"),
njs_str("[[undefined,['out']],['a',['out']],[42,['in','out']]]") },

/**/

{ njs_str("function f() { Object.prototype.toString = 1; };"
Expand Down

0 comments on commit 4045538

Please sign in to comment.