Skip to content

Commit

Permalink
#25: yet another kernel fix: yyErrOk() a.k.a. yyerrok SHOULD NOT …
Browse files Browse the repository at this point in the history 
…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.)
  • Loading branch information
@GerHobbelt
GerHobbelt committed Oct 30, 2017
1 parent 2109eed commit f946d36
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 38 deletions.
48 changes: 29 additions & 19 deletions lib/jison-parser-kernel.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
// }
};
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;`
Expand Down Expand Up @@ -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);
}
Expand All @@ -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 });
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand All @@ -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];
Expand Down
48 changes: 29 additions & 19 deletions lib/jison.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
// }
};
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;\`
Expand Down Expand Up @@ -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);
}
Expand All @@ -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 });
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand All @@ -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];
Expand Down

0 comments on commit f946d36

Please sign in to comment.