Fix infinite loop in ParsePre from re-arming InlineDup on reentry - #1186
Open
afonsojanu wants to merge 1 commit into
Open
Fix infinite loop in ParsePre from re-arming InlineDup on reentry#1186afonsojanu wants to merge 1 commit into
afonsojanu wants to merge 1 commit into
Conversation
TY_(ParsePre) called TY_(InlineDup)(doc, NULL) unconditionally on every invocation, including reentrant continuations that resume the same <pre>/<xmp> element after dispatching a child node back through the parser trampoline. ParseBlock and ParseInline only make this call on the initial, non-reentrant entry, and that distinction matters: InlineDup re-arms lexer->insert to point at whatever is still open on the inline stack, and GetToken() synthesizes exactly one implicit clone per arming before InsertedToken clears it back to NULL. Since ParsePre re-armed it again on every reentry, the lexer kept manufacturing a fresh clone forever, each one dispatched straight back into another reentry of the same call, without ever consuming more of the input stream. A minimal repro is '<b><pre><br>', which previously hung indefinitely with RSS climbing without bound; confirmed with a debugger that the input stream position stops advancing entirely once the loop starts. Moves the InlineDup call into the non-reentrant branch to match the existing pattern in ParseBlock/ParseInline, and adds a regression test case for issue htacg#1185. Ran the full existing regression_testing/cases corpus (671 files) before and after the change with no differences other than the new test case. Fixes htacg#1185
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What this fixes
TY_(ParsePre)insrc/parser.ccallsTY_(InlineDup)(doc, NULL)unconditionally on every invocation, including reentrant continuations that resume the same<pre>/<xmp>element after dispatching one of its child nodes back through the parser trampoline inParseHTMLWithNode.TY_(ParseBlock)andTY_(ParseInline)only make this call inside the non-reentrant branch (theelsewhenelement != NULL), and that turns out to matter:TY_(InlineDup)re-armslexer->insertto point at the still-open element on the inline stack, andGetToken()synthesizes exactly one implicit clone of it per arming beforeTY_(InsertedToken)clearslexer->insertback to NULL. SinceParsePrere-arms it again on every reentry, the lexer keeps manufacturing a brand-new implicit clone forever, each of which gets dispatched straight back into another reentry of the sameParsePrecall, without ever touching the input stream again.Repro (hangs indefinitely, RSS grows without bound, confirmed with gdb/lldb that the input stream position genuinely stops advancing once this starts):
I opened #1185 with the write-up, a debugger trace showing the input position frozen while new
Nodes keep getting allocated, and the same<pre>bad contentvalue across the recurring stack frame each time.The fix moves the
TY_(InlineDup)call into the initial-entry branch, matching the pattern already used in the two sibling parsers.Testing
regression_testing/cases/github-cases/case-1185@1.htmland matchinggithub-expects/case-1185.{html,txt}, following the convention inREADME/TESTING.md. Verified the old binary hangs on this input (I gave it several seconds and it never returned) and the patched one returns quickly with the expected output/exit code.regression_testing/casescorpus (all six case sets, 671 files) through both the pre-patch and post-patch builds with the same invocationtest.rbuses (-lang en_us -f <out> -config <conf> --tidy-mark no -o <result>), diffing stdout/stderr text and generated HTML byte for byte. Zero behavioral differences anywhere in the corpus other than the new test case itself.