Re: RFC: Freeing discarded objects

[Akim Demaille]

| Akim,

Hi Cris,

| I scanned your patch (esp. to bison.simple), and it's looking useful, but I 
| think you missed a case (I missed the same spot at first):
| 
| Just before here:
| >  /* YY_DECL_VARIABLES -- depending whether we use a pure parser,
| >     variables are global, or local to YYPARSE.  */
| > @@ -1123,6 +1126,7 @@
| >     YYABORT;
| >        YYDPRINTF ((stderr, "Discarding token %d (%s).\n",
| >               yychar, yytname[yychar1]));
| > +      yydestructor (yychar1, yylval);
| >        yychar =3D YYEMPTY;
| >      }
| 
| (where the discard is performed) is the case for exiting at EOF. If you're 
| half way through a production when eof comes, and your not otherwise caught 
| by an error  handler (e.g. "statement:  error ';' " doesn't match at eof!), 
| then bison will return at YYABORT here with leaked data on the stack.

Aaah!  Yes, of course!  Thanks.

| I've attached a small patch which should fix this case - I'm not so informed 
| about all the internals of the run-time engine, so I hope I picked the right 
| parameters for yydestructor here...
| 
| Cris
| 
| 
| --- bison.simple      Tue Jun 18 09:46:23 2002
| +++ bison.simple.eof  Tue Jun 18 09:53:19 2002
| @@ -1123,7 +1123,16 @@
|  
|        /* Return failure if at end of input.  */
|        if (yychar == YYEOF)
| -     YYABORT;
| +        {
| +          YYPOPSTACK;
| +       while (yyssp > yyss)
| +         {
| +           yydestructor (*yyssp, *yyvsp);
| +           YYPOPSTACK;
| +         }
| +       YYABORT;
| +        }
| +
|        YYDPRINTF ((stderr, "Discarding token %d (%s).\n",
|                 yychar, yytname[yychar1]));
|        yydestructor (yychar1, yylval);

It is not correct: yyssp is a stack of states, not a symbol numbers:
you need to use `stos[state] -> corresponding symbol number'.

I have completed your patch with ChangeLog, YYDEBUG report and test
case.

Thanks!

Index: ChangeLog
from  Cris Bailiff  <address@hidden>
        and Akim Demaille  <address@hidden>

        * data/bison.simple.new (yyerrlab1): Be sure to pop and destroy
        what's left on the stack when the error recovery hits EOF.
        * tests/actions.at (Destructors): Complete to exercise this case.

Index: THANKS
===================================================================
RCS file: /cvsroot/bison/bison/THANKS,v
retrieving revision 1.38
diff -u -u -r1.38 THANKS
--- THANKS 1 May 2002 10:28:04 -0000 1.38
+++ THANKS 18 Jun 2002 09:06:56 -0000
@@ -9,6 +9,7 @@
 Arnold Robbins         address@hidden
 Benoit Perrot           address@hidden
 Bruce Lilly             address@hidden
+Cris Bailiff            address@hidden
 Cris van Pelt           address@hidden
 Daniel Hagerty          address@hidden
 David J. MacKenzie      address@hidden
Index: data/bison.simple
===================================================================
RCS file: /cvsroot/bison/bison/data/bison.simple,v
retrieving revision 1.33
diff -u -u -r1.33 bison.simple
--- data/bison.simple 17 Jun 2002 08:43:11 -0000 1.33
+++ data/bison.simple 18 Jun 2002 09:06:57 -0000
@@ -1123,7 +1123,38 @@
 
       /* Return failure if at end of input.  */
       if (yychar == YYEOF)
-       YYABORT;
+        {
+         /* Pop the error token.  */
+          YYPOPSTACK;
+         /* Pop the rest of the stack.  */
+         while (yyssp > yyss)
+           {
+#if YYDEBUG
+             if (yydebug)
+               {
+                 if (yystos[*yyssp] < YYNTOKENS)
+                   {
+                     YYFPRINTF (stderr, "Error: popping token %d (%s",
+                                yytoknum[yystos[*yyssp]],
+                                yytname[yystos[*yyssp]]);
+# ifdef YYPRINT
+                     YYPRINT (stderr, yytoknum[yystos[*yyssp]], *yyvsp);
+# endif
+                     YYFPRINTF (stderr, ")\n");
+                   }
+                 else
+                   {
+                     YYFPRINTF (stderr, "Error: popping nonterminal (%s)\n",
+                                yytname[yystos[*yyssp]]);
+                   }
+               }
+#endif
+             yydestructor (yystos[*yyssp], *yyvsp);
+             YYPOPSTACK;
+           }
+         YYABORT;
+        }
+
       YYDPRINTF ((stderr, "Discarding token %d (%s).\n",
                  yychar, yytname[yychar1]));
       yydestructor (yychar1, yylval);
@@ -1263,8 +1294,8 @@
     {
 m4_map([b4_symbol_destructor], m4_defn([b4_symbol_destructors]))dnl
       default:
-        YYDPRINTF ((stderr, "yydestructor: unknown symbol type: %s\n",
-                   yytname[[symbol_type]]));
+        YYDPRINTF ((stderr, "yydestructor: unknown symbol type: %d (%s)\n",
+                   symbol_type, yytname[[symbol_type]]));
         break;
     }
 }
Index: tests/actions.at
===================================================================
RCS file: /cvsroot/bison/bison/tests/actions.at,v
retrieving revision 1.6
diff -u -u -r1.6 actions.at
--- tests/actions.at 17 Jun 2002 08:43:12 -0000 1.6
+++ tests/actions.at 18 Jun 2002 09:06:57 -0000
@@ -169,61 +169,100 @@
 
 #define YYERROR_VERBOSE 1
 #define YYDEBUG 1
-/* #define YYPRINT yyprint */
-
-static int yylex (void);
-static void yyerror (const char *msg);
-static void yyprint (FILE *out, int toknum, int tokval);
+#define YYPRINT yyprint
 %}
-
+%verbose
 %union
 {
   int ival;
 }
-%type <ival> thing 'x'
+%type <ival> 'x' thing line input
+%destructor { printf ("Freeing input %d\n", $$); } input
+%destructor { printf ("Freeing line %d\n", $$); } line
 %destructor { printf ("Freeing thing %d\n", $$); } thing
 %destructor { printf ("Freeing 'x' %d\n", $$); } 'x'
 
+%{
+static int yylex (void);
+static void yyerror (const char *msg);
+static void yyprint (FILE *out, int num, YYSTYPE val);
+%}
+
+
 %%
 input:
   /* Nothing. */
-| input line
+    {
+      $$ = 0;
+      printf ("input(%d): /* Nothing */';'\n", $$);
+    }
+| line input /* Right recursive to load the stack so that popping at
+               EOF can be exercised.  */
+    {
+      $$ = 2;
+      printf ("input(%d): line(%d) input(%d)';'\n", $$, $1, $2);
+    }
 ;
 
 line:
   thing thing thing ';'
-    { printf ("input: thing(%d) thing(%d) thing(%d) ';'\n", $1, $2, $3); }
+    {
+      $$ = $1;
+      printf ("line(%d): thing(%d) thing(%d) thing(%d) ';'\n", $$, $1, $2, $3);
+    }
 | thing thing ';'
-    { printf ("input: thing(%d) thing(%d) ';'\n", $1, $2); }
+    {
+      $$ = $1;
+      printf ("line(%d): thing(%d) thing(%d) ';'\n", $$, $1, $2);
+    }
 | thing ';'
-    { printf ("input: thing(%d) ';'\n", $1); }
+    {
+      $$ = $1;
+      printf ("line(%d): thing(%d) ';'\n", $$, $1);
+    }
 | error ';'
-    { printf ("input: error ';'\n"); }
+    {
+      $$ = -1;
+      printf ("line(%d): error ';'\n", $$);
+    }
 ;
 
 thing:
-  'x'  { printf ("thing: 'x' (%d)\n", $1); $$ = $1; }
+  'x'
+    {
+      $$ = $1;
+      printf ("thing(%d): 'x'(%d)\n", $$, $1);
+    }
 ;
 %%
 static int
 yylex (void)
 {
-  static const int input[] =
+  static const unsigned int input[] =
     {
+      /* Exericise the discarding of stack top and input until `error'
+         can be reduced.  */
       'x', 'x', 'x', 'x', 'x', 'x', ';',
+
+      /* Load the stack and provoke an error that cannot be caught be
+         the grammar, and check that the stack is cleared. */
       'x', 'x', ';',
       'x', ';',
-      'x', 'y', ';'
+      'y'
     };
   static int counter = 0;
 
   if (counter < (sizeof(input) / sizeof (input[0])))
     {
        yylval.ival = counter;
+       printf ("sending: '%c'(%d)\n", input[counter], counter);
        return input[counter++];
     }
   else
-    return EOF;
+    {
+      printf ("sending: EOF\n");
+      return EOF;
+    }
 }
 
 static void
@@ -233,10 +272,9 @@
 }
 
 static void
-yyprint (FILE *out, int toknum, int tokval)
+yyprint (FILE *out, int num, YYSTYPE val)
 {
-  if (0 < toknum && toknum < 256)
-    fprintf (out, " = %d", tokval);
+  fprintf (out, " = %d", val.ival);
 }
 
 int
@@ -255,28 +293,42 @@
 
 AT_CHECK([bison input.y -d -v -o input.c])
 AT_CHECK([$CC $CFLAGS $CPPFLAGS input.c -o input], 0, [], [ignore])
-AT_CHECK([./input], 0,
-[[thing: 'x' (0)
-thing: 'x' (1)
-thing: 'x' (2)
+AT_CHECK([./input], 1,
+[[sending: 'x'(0)
+thing(0): 'x'(0)
+sending: 'x'(1)
+thing(1): 'x'(1)
+sending: 'x'(2)
+thing(2): 'x'(2)
+sending: 'x'(3)
 parse error, unexpected 'x', expecting ';'
 Freeing thing 2
 Freeing thing 1
 Freeing thing 0
 Freeing 'x' 3
+sending: 'x'(4)
 Freeing 'x' 4
+sending: 'x'(5)
 Freeing 'x' 5
-input: error ';'
-thing: 'x' (7)
-thing: 'x' (8)
-input: thing(7) thing(8) ';'
-thing: 'x' (10)
-input: thing(10) ';'
-thing: 'x' (12)
-parse error, unexpected $undefined., expecting 'x' or ';'
-Freeing thing 12
-input: error ';'
-Successful parse.
+sending: ';'(6)
+line(-1): error ';'
+sending: 'x'(7)
+thing(7): 'x'(7)
+sending: 'x'(8)
+thing(8): 'x'(8)
+sending: ';'(9)
+line(7): thing(7) thing(8) ';'
+sending: 'x'(10)
+thing(10): 'x'(10)
+sending: ';'(11)
+line(10): thing(10) ';'
+sending: 'y'(12)
+parse error, unexpected $undefined., expecting $ or error or 'x'
+sending: EOF
+Freeing line 10
+Freeing line 7
+Freeing line -1
+Parsing FAILED.
 ]])
 
 AT_CLEANUP