[jira] [Assigned] (THRIFT-6212) Compiler loops forever on an unterminated comment at end of file

["Jens Geyer (Jira)" <ji...@apache.org>]

     [ 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)