Jens Geyer created THRIFT-6212:
----------------------------------
Summary: 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
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)
