From f946d36c3923000beea96ab5cce4091e9e2476d3 Mon Sep 17 00:00:00 2001 From: Ger Hobbelt Date: Mon, 30 Oct 2017 01:21:14 +0100 Subject: [PATCH] https://github.com/GerHobbelt/jison/issues/25: yet another kernel fix: `yyErrOk()` a.k.a. `yyerrok` SHOULD NOT reset/cleanup the `recoveringErrorInfo` object as one may invoke `yyerrok` while still inside the error recovery phase of the parser, thus *potentially* causing trouble down the lane for subsequent parse states. (This is another edge case that's hard to produce: better-safe-than-sorry coding style applies.) --- lib/jison-parser-kernel.js | 48 +++++++++++++++++++++++--------------- lib/jison.js | 48 +++++++++++++++++++++++--------------- 2 files changed, 58 insertions(+), 38 deletions(-) diff --git a/lib/jison-parser-kernel.js b/lib/jison-parser-kernel.js index 1a79b1fdd..caf4e6545 100644 --- a/lib/jison-parser-kernel.js +++ b/lib/jison-parser-kernel.js @@ -251,10 +251,20 @@ function parse(input, parseParams) { if (yydebug) yydebug('yyerrok: ', { symbol: symbol, state: state, newState: newState, recovering: recovering, action: action }); recovering = 0; - if (recoveringErrorInfo && typeof recoveringErrorInfo.destroy === 'function') { - recoveringErrorInfo.destroy(); - recoveringErrorInfo = undefined; - } + // DO NOT reset/cleanup `recoveringErrorInfo` yet: userland code + // MAY invoke this API before the error is actually fully + // recovered, in which case the parser recovery code won't be able + // to append the skipped tokens to this info object. + // + // The rest of the kernel code is safe enough that it won't inadvertedly + // re-use an old `recoveringErrorInfo` chunk so we'ld better wait + // with destruction/cleanup until the end of the parse or until another + // fresh parse error rears its ugly head... + // + // if (recoveringErrorInfo && typeof recoveringErrorInfo.destroy === 'function') { + // recoveringErrorInfo.destroy(); + // recoveringErrorInfo = undefined; + // } }; } @@ -894,7 +904,7 @@ function parse(input, parseParams) { // try to recover from error if (error_rule_depth < 0) { - ASSERT(recovering > 0); + ASSERT(recovering > 0, "line 897"); recoveringErrorInfo.info_stack_pointer = esp; // barf a fatal hairball when we're out of look-ahead symbols and none hit a match @@ -1046,9 +1056,9 @@ function parse(input, parseParams) { // only a single parse loop, which handles everything. Our goal is // to eke out every drop of performance in the main parse loop... - ASSERT(recoveringErrorInfo); - ASSERT(symbol === TERROR); - ASSERT(!action); + ASSERT(recoveringErrorInfo, "line 1049"); + ASSERT(symbol === TERROR, "line 1050"); + ASSERT(!action, "line 1051"); var errorSymbolFromParser = true; for (;;) { // retrieve state number from top of stack @@ -1084,7 +1094,7 @@ function parse(input, parseParams) { if (!action) { if (yydebug) yydebug('**NESTED ERROR DETECTED** while still recovering from previous error'); - ASSERT(recoveringErrorInfo); + ASSERT(recoveringErrorInfo, "line 1087"); // Prep state variables so that upon breaking out of // this "slow parse loop" and hitting the `continue;` @@ -1128,12 +1138,12 @@ function parse(input, parseParams) { // Push a special value onto the stack when we're // shifting the `error` symbol that is related to the // error we're recovering from. - ASSERT(recoveringErrorInfo); + ASSERT(recoveringErrorInfo, "line 1131"); vstack[sp] = recoveringErrorInfo; lstack[sp] = this.yyMergeLocationInfo(null, null, recoveringErrorInfo.loc, lexer.yylloc, true); } else { - ASSERT(symbol !== 0); - ASSERT(preErrorSymbol === 0); + ASSERT(symbol !== 0, "line 1135"); + ASSERT(preErrorSymbol === 0, "line 1136"); vstack[sp] = lexer.yytext; lstack[sp] = copy_yylloc(lexer.yylloc); } @@ -1160,7 +1170,7 @@ function parse(input, parseParams) { } } else { // error just occurred, resume old lookahead f/ before error, *unless* that drops us straight back into error mode: - ASSERT(recovering > 0); + ASSERT(recovering > 0, "line 1163"); symbol = preErrorSymbol; preErrorSymbol = 0; if (yydebug) yydebug('... SHIFT:error recovery: ', { recovering: recovering, symbol: symbol }); @@ -1191,7 +1201,7 @@ function parse(input, parseParams) { // while the error symbol hasn't been shifted onto // the stack yet. Hence we only exit this "slow parse loop" // when *both* conditions are met! - ASSERT(preErrorSymbol === 0); + ASSERT(preErrorSymbol === 0, "line 1194"); if (recovering === 0) { break; } @@ -1269,7 +1279,7 @@ function parse(input, parseParams) { // i.e. did the parser already produce a parse result in here?! // *or* did we hit an unsupported parse state, to be handled // in the `switch/default` code further below? - ASSERT(action !== 2); + ASSERT(action !== 2, "line 1272"); if (action === 0 || action === 1) { continue; } @@ -1349,8 +1359,8 @@ function parse(input, parseParams) { ++sp; symbol = 0; - ASSERT(preErrorSymbol === 0); // normal execution / no error - ASSERT(recovering === 0); // normal execution / no error + ASSERT(preErrorSymbol === 0, "line 1352"); // normal execution / no error + ASSERT(recovering === 0, "line 1353"); // normal execution / no error // Pick up the lexer details for the current symbol as that one is not 'look-ahead' any more: yyleng = lexer.yyleng; @@ -1361,8 +1371,8 @@ function parse(input, parseParams) { // reduce: case 2: - ASSERT(preErrorSymbol === 0); // normal execution / no error - ASSERT(recovering === 0); // normal execution / no error + ASSERT(preErrorSymbol === 0, "line 1364"); // normal execution / no error + ASSERT(recovering === 0, "line 1365"); // normal execution / no error this_production = this.productions_[newState - 1]; // `this.productions_[]` is zero-based indexed while states start from 1 upwards... yyrulelen = this_production[1]; diff --git a/lib/jison.js b/lib/jison.js index 3859c1b34..ad2c1bdef 100755 --- a/lib/jison.js +++ b/lib/jison.js @@ -6810,10 +6810,20 @@ parser.parse = `function parse(input, parseParams) { if (yydebug) yydebug('yyerrok: ', { symbol: symbol, state: state, newState: newState, recovering: recovering, action: action }); recovering = 0; - if (recoveringErrorInfo && typeof recoveringErrorInfo.destroy === 'function') { - recoveringErrorInfo.destroy(); - recoveringErrorInfo = undefined; - } + // DO NOT reset/cleanup \`recoveringErrorInfo\` yet: userland code + // MAY invoke this API before the error is actually fully + // recovered, in which case the parser recovery code won't be able + // to append the skipped tokens to this info object. + // + // The rest of the kernel code is safe enough that it won't inadvertedly + // re-use an old \`recoveringErrorInfo\` chunk so we'ld better wait + // with destruction/cleanup until the end of the parse or until another + // fresh parse error rears its ugly head... + // + // if (recoveringErrorInfo && typeof recoveringErrorInfo.destroy === 'function') { + // recoveringErrorInfo.destroy(); + // recoveringErrorInfo = undefined; + // } }; } @@ -7453,7 +7463,7 @@ parser.parse = `function parse(input, parseParams) { // try to recover from error if (error_rule_depth < 0) { - ASSERT(recovering > 0); + ASSERT(recovering > 0, "line 897"); recoveringErrorInfo.info_stack_pointer = esp; // barf a fatal hairball when we're out of look-ahead symbols and none hit a match @@ -7605,9 +7615,9 @@ parser.parse = `function parse(input, parseParams) { // only a single parse loop, which handles everything. Our goal is // to eke out every drop of performance in the main parse loop... - ASSERT(recoveringErrorInfo); - ASSERT(symbol === TERROR); - ASSERT(!action); + ASSERT(recoveringErrorInfo, "line 1049"); + ASSERT(symbol === TERROR, "line 1050"); + ASSERT(!action, "line 1051"); var errorSymbolFromParser = true; for (;;) { // retrieve state number from top of stack @@ -7643,7 +7653,7 @@ parser.parse = `function parse(input, parseParams) { if (!action) { if (yydebug) yydebug('**NESTED ERROR DETECTED** while still recovering from previous error'); - ASSERT(recoveringErrorInfo); + ASSERT(recoveringErrorInfo, "line 1087"); // Prep state variables so that upon breaking out of // this "slow parse loop" and hitting the \`continue;\` @@ -7687,12 +7697,12 @@ parser.parse = `function parse(input, parseParams) { // Push a special value onto the stack when we're // shifting the \`error\` symbol that is related to the // error we're recovering from. - ASSERT(recoveringErrorInfo); + ASSERT(recoveringErrorInfo, "line 1131"); vstack[sp] = recoveringErrorInfo; lstack[sp] = this.yyMergeLocationInfo(null, null, recoveringErrorInfo.loc, lexer.yylloc, true); } else { - ASSERT(symbol !== 0); - ASSERT(preErrorSymbol === 0); + ASSERT(symbol !== 0, "line 1135"); + ASSERT(preErrorSymbol === 0, "line 1136"); vstack[sp] = lexer.yytext; lstack[sp] = copy_yylloc(lexer.yylloc); } @@ -7719,7 +7729,7 @@ parser.parse = `function parse(input, parseParams) { } } else { // error just occurred, resume old lookahead f/ before error, *unless* that drops us straight back into error mode: - ASSERT(recovering > 0); + ASSERT(recovering > 0, "line 1163"); symbol = preErrorSymbol; preErrorSymbol = 0; if (yydebug) yydebug('... SHIFT:error recovery: ', { recovering: recovering, symbol: symbol }); @@ -7750,7 +7760,7 @@ parser.parse = `function parse(input, parseParams) { // while the error symbol hasn't been shifted onto // the stack yet. Hence we only exit this "slow parse loop" // when *both* conditions are met! - ASSERT(preErrorSymbol === 0); + ASSERT(preErrorSymbol === 0, "line 1194"); if (recovering === 0) { break; } @@ -7828,7 +7838,7 @@ parser.parse = `function parse(input, parseParams) { // i.e. did the parser already produce a parse result in here?! // *or* did we hit an unsupported parse state, to be handled // in the \`switch/default\` code further below? - ASSERT(action !== 2); + ASSERT(action !== 2, "line 1272"); if (action === 0 || action === 1) { continue; } @@ -7908,8 +7918,8 @@ parser.parse = `function parse(input, parseParams) { ++sp; symbol = 0; - ASSERT(preErrorSymbol === 0); // normal execution / no error - ASSERT(recovering === 0); // normal execution / no error + ASSERT(preErrorSymbol === 0, "line 1352"); // normal execution / no error + ASSERT(recovering === 0, "line 1353"); // normal execution / no error // Pick up the lexer details for the current symbol as that one is not 'look-ahead' any more: yyleng = lexer.yyleng; @@ -7920,8 +7930,8 @@ parser.parse = `function parse(input, parseParams) { // reduce: case 2: - ASSERT(preErrorSymbol === 0); // normal execution / no error - ASSERT(recovering === 0); // normal execution / no error + ASSERT(preErrorSymbol === 0, "line 1364"); // normal execution / no error + ASSERT(recovering === 0, "line 1365"); // normal execution / no error this_production = this.productions_[newState - 1]; // \`this.productions_[]\` is zero-based indexed while states start from 1 upwards... yyrulelen = this_production[1];