[
https://issues.apache.org/jira/browse/THRIFT-6212?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer reassigned THRIFT-6212:
----------------------------------
Assignee: Jens Geyer
> Compiler loops forever on an unterminated comment at end of file
> ----------------------------------------------------------------
>
> Key: THRIFT-6212
> URL: https://issues.apache.org/jira/browse/THRIFT-6212
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (General)
> Reporter: Jens Geyer
> Assignee: Jens Geyer
> Priority: Minor
>
> h2. Symptom
> A {{.thrift}} file whose last comment is never closed makes the compiler loop
> forever instead of reporting an error:
> {code}
> $ printf '/* unterminated comment' > c.thrift
> $ thrift --gen cpp c.thrift # never returns
> $ printf '/** unterminated doc' > d.thrift
> $ thrift --gen cpp d.thrift # never returns
> {code}
> Reproduced against master @ {{5c5e93e1e}} (the lexer is byte-identical to
> {{59a949c80}}, where it was measured). Both invocations were still running
> when a 10 second timeout killed them.
> h2. Cause
> Both comment scanners test {{yyinput()}} for {{EOF}}:
> {code}
> int ch = yyinput();
> parsed.push_back(ch);
> switch (ch) {
> case EOF:
> yyerror("Unexpected end of file in multiline comment at %d\n",
> yylineno);
> exit(1);
> {code}
> flex's {{yyinput()}} returns {{0}} at end of input, never {{-1}}, and keeps
> returning {{0}} on every subsequent call. Measured with a standalone flex
> 2.6.4 scanner using this file's own options, with and without {{noyywrap}}.
> So {{case EOF:}} never matches, the byte falls through to {{default:}},
> {{parsed.push_back(ch)}} appends a NUL, and the loop asks for one more
> character forever, growing {{parsed}} without bound.
> Two sites in {{compiler/cpp/src/thrift/thriftl.ll}}:
> * line 136, the {{doctext_begin}} rule
> * line 179, the {{multicm_begin}} rule
> When the input comes from a memory buffer rather than a file there is no
> {{yyin}}, and the loop reaches {{fread()}} on a null {{FILE*}} instead of
> spinning:
> {code}
> #0 __GI__IO_fread (buf=..., size=1, count=3, fp=0x0) at ./libio/iofread.c:37
> #1 yy_get_next_buffer () at thriftl.cc:1708
> #2 yyinput () at thriftl.cc:1841
> {code}
> That path matters because the compiler test suite drives the lexer through
> {{yy_scan_bytes()}}, so a test case for an unterminated comment takes the
> whole suite down until this is fixed.
> h2. Relationship to THRIFT-4244
> The identical defect in the string-literal scanner is being fixed under
> THRIFT-4244, which introduces a {{yywrap()}} callback and a flag that tells
> real end of input from a literal NUL byte. The comment scanners were
> deliberately left out of that pull request's scope.
> The fix here reuses that flag, so it depends on THRIFT-4244 landing first.
> h2. Proposed fix
> Replace {{case EOF:}} with {{case 0:}} guarded by the end-of-input flag, at
> both sites:
> {code}
> while(state < 2)
> {
> + yyinput_reached_eof = false;
> int ch = yyinput();
> parsed.push_back(ch);
> switch (ch) {
> - case EOF:
> - yyerror("Unexpected end of file in multiline comment at %d\n",
> yylineno);
> - exit(1);
> + case 0:
> + if (yyinput_reached_eof) {
> + yyerror("Unexpected end of file in multiline comment at %d\n",
> yylineno);
> + exit(1);
> + }
> + state = 0;
> + break;
> {code}
> Guarding on the flag rather than treating every {{0}} as end of input keeps
> the current behaviour for a literal NUL byte inside a closed comment, which
> today lands in {{parsed}} and is either discarded ({{multicm_begin}}) or
> truncated by {{strdup()}} ({{doctext_begin}}).
> Accepted input is unaffected either way: the change only reaches input that
> never terminates today.
> h2. Scope
> Compiler only. The lexer is shared by every language binding, so no generator
> changes are needed.
> This description was AI-assisted (Claude Opus 5); every claim above was
> verified by execution against the named commits.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
