Re: EOF while in parse_matched_pair closes interactive shell

[Eduardo A . Bustamante López]

On Tue, Feb 21, 2023 at 11:03:59AM -0500, Chet Ramey wrote:
(...)
> The shell should exit on EOF. Previous versions relied on undocumented
> bison behavior, which resulted in a token that wasn't handled by the
> grammar.

Thank you for reviewing this and for the explanation. I initially thought this
changed as a result of this change:

| parse.y
|     - yylex: return YYUNDEF as current_token if read_token returns < 0.
|       Fixes parser reset issue reported by Todd Stein <toddbstein@gmail.com>
|       in https://savannah.gnu.org/support/index.php?110745

I changed `yylex' to return -1 the way it was, and it doesn't make a difference
in this case.

By luck, I found that removing this line reverts to the original behavior:

$ git diff -- parse.y
diff --git a/parse.y b/parse.y
index 9178e9a7..87d45ac1 100644
--- a/parse.y
+++ b/parse.y
@@ -3714,7 +3714,7 @@ parse_matched_pair (int qc, int open, int close, size_t 
*lenp, int flags)
          free (ret);
          parser_error (start_lineno, _("unexpected EOF while looking for 
matching `%c'"), close);
          EOF_Reached = 1;      /* XXX */
-         parser_state |= PST_NOERROR;  /* avoid redundant error message */
+         /*parser_state |= PST_NOERROR;*/      /* avoid redundant error 
message */
          return (&matched_pair_error);
        }

My understanding of how this works is as follows. The unexpected EOF causes
`parse_matched_pair' to return an error, which eventually leads to a call to
`yyerror'. In `yyerror', we call `report_syntax_error' _only if_ the NOERROR
parser flag is off.  Despite its name, `report_syntax_error' does more than
that. It'll actually clear `EOF_Reached' when the shell is interactive:

  bash: syntax error: unexpected end of file
  6458          if (interactive && EOF_Reached)
  6459        EOF_Reached = 0;
  6462      last_command_exit_value = (executing_builtin && 
parse_and_execute_level) ? EX_BADSYNTAX : EX_BADUSAGE;

The parser eventually makes it way back to `reader_loop'. In 5.2, EOF_Reached is
1 at that point. In 5.1, it's 0 as it was cleared by `report_syntax_error'.

Knowing this now, I was able to find the commit that changed this, and the note
explaining the change:

| commit b48c234286ae692c758306dac26a5bb72f782fef
| Date:   Mon Oct 31 10:08:36 2022 -0400
| (...)
| parse.y
|        - parse_matched_pair: set PST_NOERROR if we read to EOF without finding
|          a closing match and call parser_error; avoids redundant error
|          message

As an aside, this change also restores the original behavior:

$ git diff -- parse.y
diff --git a/parse.y b/parse.y
index 9178e9a7..d2d20874 100644
--- a/parse.y
+++ b/parse.y
@@ -3713,7 +3713,7 @@ parse_matched_pair (int qc, int open, int close, size_t 
*lenp, int flags)
        {
          free (ret);
          parser_error (start_lineno, _("unexpected EOF while looking for 
matching `%c'"), close);
-         EOF_Reached = 1;      /* XXX */
+         /*EOF_Reached = 1;*/  /* XXX */
          parser_state |= PST_NOERROR;  /* avoid redundant error message */
          return (&matched_pair_error);
        }

In any case, I do understand why you've made this change. I've checked how bash
deals with an EOF while parsing other partial commands (e.g. `if', `for x in',
`echo $(') and these all behave in the same way. So I don't disagree. I just
thought I should share my troubleshooting notes.

> If there's a bug here, it's that interactive shells need to handle
> IGNOREEOF in this case.

Thanks for reminding me about IGNOREEOF. It does seem like `parse_matched_pair'
doesn't work with `ignoreeof' in this case. I suppose that's because the parser
doesn't see `yacc_EOF', so it never calls `handle_eof_input_unit'.