Re: [bison crash] Segmentation fault at quotearg_buffer_restyled

[Akim Demaille]

Hi!

> Le 6 mars 2020 à 06:49, Ahcheong Lee <address@hidden> a écrit :
> 
> Hello, this is Ahcheong Lee
> 
> I've found there was similar crash report on bison3.3 (link
> <https://lists.gnu.org/archive/html/bug-bison/2019-03/msg00008.html>),
> the crash was fixed, but it seems it appeared again.
> 
> There was a segmentation fault on quotearg_buffer_restyled,
> lib/quotearg.c:400
> You can reproduce it with the following command:
> ./bison <attached file>

Hi!

The error triggered by your amazingly broken input file (as expected from 
fuzzing) boils down to:

%token error error %%

on which we crash.  You are right that it's similar to a previous report, but 
it is still different.

Thanks a lot for catching this!  I'm installing the following in maint (to 
become 3.5.3).

commit 8bec6c7208ebf7b8e6873cf5f948b25d32d74056
Author: Akim Demaille <address@hidden>
Date:   Sun Mar 8 07:27:57 2020 +0100

    diagnostics: don't crash because of repeated definitions of error
    
    According to https://www.unix.com/man-page/POSIX/1posix/yacc/, the
    user is allowed to specify her user number for the error token:
    
        The token error shall be reserved for error handling. The name
        error can be used in grammar rules. It indicates places where the
        parser can recover from a syntax error. The default value of error
        shall be 256. Its value can be changed using a %token
        declaration. The lexical analyzer should not return the value of
        error.
    
    I think this feature is useless, the user should not have to deal with
    that.  The intend is probably to give the user a means to use 256 if
    she wants to, but provided "error" cleared the path first by being
    assigned another number.  In the case of Bison, 256 is assigned to
    "error" at the end if the user did not use it for a token of hers.  So
    this feature is useless.
    
    Yet it is valid, and if the user assigns twice a token number to
    "error", then the second time we want to complain about it and want to
    show the original definition.  At this point, we try to display the
    built-in definition of "error", whose location is NULL, and we crash.
    
    Rather, the location of the first user definition of "error" should
    become its defining location.
    
    Reported byg Ahcheong Lee.
    https://lists.gnu.org/r/bug-bison/2020-03/msg00007.html
    
    * src/symtab.c (symbol_class_set): If this is a declaration and the
    symbol was not declared yet, keep this as defining location.
    * tests/input.at (Redefining the error token): New.

diff --git a/src/location.c b/src/location.c
index 9f929c00..4b7273d2 100644
--- a/src/location.c
+++ b/src/location.c
@@ -175,6 +175,8 @@ location_print (location loc, FILE *out)
     }
   else
     {
+      aver (loc.start.file);
+      aver (loc.end.file);
       int end_col = 0 != loc.end.column ? loc.end.column - 1 : 0;
       res += fprintf (out, "%s",
                       quotearg_n_style (3, escape_quoting_style, 
loc.start.file));
diff --git a/src/symtab.c b/src/symtab.c
index b137bbf0..b4106ea0 100644
--- a/src/symtab.c
+++ b/src/symtab.c
@@ -555,7 +555,10 @@ symbol_class_set (symbol *sym, symbol_class class, 
location loc, bool declaring)
                                _("previous declaration"));
             }
           else
-            s->status = declared;
+            {
+              sym->location = loc;
+              s->status = declared;
+            }
         }
     }
 }
diff --git a/tests/input.at b/tests/input.at
index 4c1f5b25..dd5af739 100644
--- a/tests/input.at
+++ b/tests/input.at
@@ -298,6 +298,37 @@ input.y:8.14: error: syntax error, unexpected integer
 AT_CLEANUP
 
 
+## ---------------------------- ##
+## Redefining the error token.  ##
+## ---------------------------- ##
+
+AT_SETUP([Redefining the error token])
+
+# We used to crash when trying to display the original definition of
+# "error", which is a builtin without any location.
+
+AT_DATA([input.y],
+[[%token error 123
+%token error 124
+%%
+exp:
+]])
+
+AT_BISON_CHECK([-fcaret input.y], [1], [],
+[[input.y:2.8-12: warning: symbol error redeclared [-Wother]
+    2 | %token error 124
+      |        ^~~~~
+input.y:1.8-12:     previous declaration
+    1 | %token error 123
+      |        ^~~~~
+input.y:2.14-16: error: redefining user token number of error
+    2 | %token error 124
+      |              ^~~
+]])
+
+AT_CLEANUP
+
+
 ## ------------------ ##
 ## Dangling aliases.  ##
 ## ------------------ ##