From 592f6f8f4f914e2d2a77f4fbe3e0a4ad1f715543 Mon Sep 17 00:00:00 2001 From: Sami Vaarala Date: Tue, 10 Mar 2015 16:23:19 +0200 Subject: [PATCH 1/4] Bug testcase for GH-132 --- .../test-bug-enum-shuffle-gh132.js | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 ecmascript-testcases/test-bug-enum-shuffle-gh132.js diff --git a/ecmascript-testcases/test-bug-enum-shuffle-gh132.js b/ecmascript-testcases/test-bug-enum-shuffle-gh132.js new file mode 100644 index 00000000..40f3ffbf --- /dev/null +++ b/ecmascript-testcases/test-bug-enum-shuffle-gh132.js @@ -0,0 +1,51 @@ +/* + * https://github.com/svaarala/duktape/issues/132 + * + * Does not seem to capture all of the problematic behavior in babel.js + * referenced in the issue. + */ + +/*=== +done +===*/ + +function genfunc(n) { + var res = []; + var i; + res.push('(function test' + n + '() {'); + for (i = 0; i < n; i++) { + res.push(' var _v' + i + ' = ' + i + ';'); + } + res.push(' var k;'); + res.push(' var ret = 0;'); + res.push(' for (k in { "foo": "bar", "bar": "quux" }) {'); + res.push(' ret++;'); + res.push(' }'); + res.push(' return ret;'); + res.push('})'); + return res.join('\n'); +} + +function test() { + var i; + + for (i = 128; i < 384; i++) { + try { + var src = genfunc(i); + //print(src); + var fn = eval(src); + var ret = fn(); + if (ret != 2) { throw new Error('invalid return value: ' + ret); } + } catch (e) { + print(i, e); + } + } + + print('done'); +} + +try { + test(); +} catch (e) { + print(e.stack || e); +} From 8cf786d93e8e345f916e28abe56e21892de50b47 Mon Sep 17 00:00:00 2001 From: Sami Vaarala Date: Tue, 10 Mar 2015 04:13:07 +0200 Subject: [PATCH 2/4] Add shuffle flags to INIT/NEXTENUM (GH-132) --- src/duk_js_compiler.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/duk_js_compiler.c b/src/duk_js_compiler.c index 051e22a2..803d635d 100644 --- a/src/duk_js_compiler.c +++ b/src/duk_js_compiler.c @@ -5072,7 +5072,7 @@ DUK_LOCAL void duk__parse_for_stmt(duk_compiler_ctx *comp_ctx, duk_ivalue *res, pc_l2 = duk__get_current_pc(comp_ctx); reg_target = duk__exprtop_toreg(comp_ctx, res, DUK__BP_FOR_EXPR /*rbp_flags*/); /* Expression */ duk__emit_extraop_b_c(comp_ctx, - DUK_EXTRAOP_INITENUM, + DUK_EXTRAOP_INITENUM | DUK__EMIT_FLAG_B_IS_TARGET, (duk_regconst_t) (reg_temps + 1), (duk_regconst_t) reg_target); pc_jumpto_l4 = duk__emit_jump_empty(comp_ctx); @@ -5086,7 +5086,7 @@ DUK_LOCAL void duk__parse_for_stmt(duk_compiler_ctx *comp_ctx, duk_ivalue *res, pc_l4 = duk__get_current_pc(comp_ctx); duk__emit_extraop_b_c(comp_ctx, - DUK_EXTRAOP_NEXTENUM, + DUK_EXTRAOP_NEXTENUM | DUK__EMIT_FLAG_B_IS_TARGET, (duk_regconst_t) (reg_temps + 0), (duk_regconst_t) (reg_temps + 1)); pc_jumpto_l5 = duk__emit_jump_empty(comp_ctx); /* NEXTENUM jump slot: executed when enum finished */ From 869ac61514fdb2136605c45c568979d9a554ea53 Mon Sep 17 00:00:00 2001 From: Sami Vaarala Date: Tue, 10 Mar 2015 16:23:41 +0200 Subject: [PATCH 3/4] Fix NEXTENUM jump slot shuffle issue --- src/duk_js_compiler.c | 33 +++++++++++++++++++++++++-------- src/duk_js_compiler.h | 3 +++ 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/duk_js_compiler.c b/src/duk_js_compiler.c index 803d635d..e4486597 100644 --- a/src/duk_js_compiler.c +++ b/src/duk_js_compiler.c @@ -1001,12 +1001,13 @@ DUK_LOCAL void duk__convert_to_func_template(duk_compiler_ctx *comp_ctx, duk_boo /* Code emission flags, passed in the 'opcode' field. Opcode + flags * fit into 16 bits for now, so use duk_small_uint.t. */ -#define DUK__EMIT_FLAG_NO_SHUFFLE_A (1 << 8) -#define DUK__EMIT_FLAG_NO_SHUFFLE_B (1 << 9) -#define DUK__EMIT_FLAG_NO_SHUFFLE_C (1 << 10) -#define DUK__EMIT_FLAG_A_IS_SOURCE (1 << 11) /* slot A is a source (default: target) */ -#define DUK__EMIT_FLAG_B_IS_TARGET (1 << 12) /* slot B is a target (default: source) */ -#define DUK__EMIT_FLAG_C_IS_TARGET (1 << 13) /* slot C is a target (default: source) */ +#define DUK__EMIT_FLAG_NO_SHUFFLE_A (1 << 8) +#define DUK__EMIT_FLAG_NO_SHUFFLE_B (1 << 9) +#define DUK__EMIT_FLAG_NO_SHUFFLE_C (1 << 10) +#define DUK__EMIT_FLAG_A_IS_SOURCE (1 << 11) /* slot A is a source (default: target) */ +#define DUK__EMIT_FLAG_B_IS_TARGET (1 << 12) /* slot B is a target (default: source) */ +#define DUK__EMIT_FLAG_C_IS_TARGET (1 << 13) /* slot C is a target (default: source) */ +#define DUK__EMIT_FLAG_RESERVE_JUMPSLOT (1 << 14) /* reserve a jumpslot after instr before target spilling, used for NEXTENUM */ /* XXX: clarify on when and where DUK__CONST_MARKER is allowed */ /* XXX: opcode specific assertions on when consts are allowed */ @@ -1315,6 +1316,16 @@ DUK_LOCAL void duk__emit_a_b_c(duk_compiler_ctx *comp_ctx, duk_small_uint_t op_f ins |= DUK_ENC_OP_A_B_C(op_flags & 0xff, a, b, c); duk__emit(comp_ctx, ins); + /* NEXTENUM needs a jump slot right after the main instruction. + * When the JUMP is taken, output spilling is not needed so this + * workaround is possible. The jump slot PC is exceptionally + * plumbed through comp_ctx to minimize call sites. + */ + if (op_flags & DUK__EMIT_FLAG_RESERVE_JUMPSLOT) { + comp_ctx->emit_jumpslot_pc = duk__get_current_pc(comp_ctx); + duk__emit_abc(comp_ctx, DUK_OP_JUMP, 0); + } + /* Output shuffling: only one output register is realistically possible. * Zero is OK to check against: if the target register was zero, it is * never shuffled. @@ -5084,12 +5095,18 @@ DUK_LOCAL void duk__parse_for_stmt(duk_compiler_ctx *comp_ctx, duk_ivalue *res, duk__parse_stmt(comp_ctx, res, 0 /*allow_source_elem*/); /* temp reset is not necessary after duk__parse_stmt(), which already does it */ + /* NEXTENUM needs a jump slot right after the main opcode. + * We need the code emitter to reserve the slot: if there's + * target shuffling, the target shuffle opcodes must happen + * after the jump slot (for NEXTENUM the shuffle opcodes are + * not needed if the enum is finished). + */ pc_l4 = duk__get_current_pc(comp_ctx); duk__emit_extraop_b_c(comp_ctx, - DUK_EXTRAOP_NEXTENUM | DUK__EMIT_FLAG_B_IS_TARGET, + DUK_EXTRAOP_NEXTENUM | DUK__EMIT_FLAG_B_IS_TARGET | DUK__EMIT_FLAG_RESERVE_JUMPSLOT, (duk_regconst_t) (reg_temps + 0), (duk_regconst_t) (reg_temps + 1)); - pc_jumpto_l5 = duk__emit_jump_empty(comp_ctx); /* NEXTENUM jump slot: executed when enum finished */ + pc_jumpto_l5 = comp_ctx->emit_jumpslot_pc; /* NEXTENUM jump slot: executed when enum finished */ duk__emit_jump(comp_ctx, pc_l1); /* jump to next loop, using reg_v34_iter as iterated value */ pc_l5 = duk__get_current_pc(comp_ctx); diff --git a/src/duk_js_compiler.h b/src/duk_js_compiler.h index ae52e2c6..df2da049 100644 --- a/src/duk_js_compiler.h +++ b/src/duk_js_compiler.h @@ -209,6 +209,9 @@ struct duk_compiler_ctx { duk_int_t recursion_depth; duk_int_t recursion_limit; + /* code emission temporary */ + duk_int_t emit_jumpslot_pc; + /* current function being compiled (embedded instead of pointer for more compact access) */ duk_compiler_func curr_func; }; From d7b503c340f62dfba18d5071d5b183d3956bf370 Mon Sep 17 00:00:00 2001 From: Sami Vaarala Date: Wed, 11 Mar 2015 01:17:14 +0200 Subject: [PATCH 4/4] Releases: enum shuffle bug --- RELEASES.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/RELEASES.rst b/RELEASES.rst index a0928fdf..fe81e6dd 100644 --- a/RELEASES.rst +++ b/RELEASES.rst @@ -840,6 +840,9 @@ Planned bound variable, e.g. 'return 0, (function { return 1; })(), 2;' would return 1 instead of 2 (GH-131) +* Fix for-in statement shuffle bug which caused enumeration to fail + in large functions which enable register shuffling (GH-132) + * Add support for TI-Nspire (using Ndless, see GH-113) 2.0.0 (XXXX-XX-XX)