Re: [GNU Bison 2.4.2] testsuite: Failing tests of bison 2.4.2 on Solaris 8+9 w/Sun Studio 11+12

[Joel E. Denny]

On Wed, 31 Mar 2010, tys lefering wrote:

> On Wed, March 31, 2010 18:55, Joel E. Denny wrote:
> > Thanks for the report.  Please try the patch below and let us know if it
> > fixes it.
> Applied the patch to the temp bison tar.gz and tested it:
> ## ------------- ##
> ## Test results. ##
> ## ------------- ##
> 
> All 243 tests were successful.

Thanks.  I pushed that to branch-2.4.3, branch-2.5, and master.

> also tested './configure --enable-gcc-warnings=yes' using gcc-4.5
> (and the -Werror commented out to keep compilation going)

Thanks for the report.  All but the last warning you reported is for 
gnulib, and we've been ignoring those.  That is, -Werror should already be 
off.

> gcc -std=gnu99  -I. -I../lib  -I../lib  -Wall -Wextra -Wno-sign-compare
> -Wcast-align -Wcast-qual -Wformat -Wpointer-arith -Wwrite-strings
> -Wbad-function-cast -Wmissing-declarations -Wmissing-prototypes -Wshadow
> -Wstrict-prototypes  -g -O2 -MT parse-gram.o -MD -MP -MF
> .deps/parse-gram.Tpo -c -o parse-gram.o parse-gram.c
> parse-gram.c: In function 'gram_parse':
> parse-gram.c:2790:3: warning: array subscript is below array bounds

It looks like there's an old bug here that needs to be fixed.  We declare 
this:

  YYLTYPE yyerror_range[2];

But then we do this:

  YYLLOC_DEFAULT (yyloc, (yyerror_range - 1), 2);

The problem is yyerror_range - 1, which points one element before the 
beginning of yyerror_range.  The YYLLOC_DEFAULT we provide adds at least 1 
before dereferencing that, and the Bison manual implies that a user's 
YYLLOC_DEFAULT should do this too.  However, according to the C standard, 
the behavior of the initial subtraction is undefined, so any arithmetic 
after that must be undefined as well.

At least that's my understanding.  I have no idea how important this is in 
practice, but the fix is simple and is already implemented in glr.c.

Please let us know if this patch, not yet pushed, works for you.  I've 
included the generated changes, which are important for testing the patch.

>From 48f4100a8299918a3abc9c5d29db85319cad3cee Mon Sep 17 00:00:00 2001
From: Joel E. Denny <address@hidden>
Date: Sat, 3 Apr 2010 14:58:22 -0400
Subject: [PATCH] portability: fix pointer arithmetic to conform to C standard.

Reported by Tys Lefering at
<http://lists.gnu.org/archive/html/bug-bison/2010-03/msg00035.html>.
This fix is already implemented in glr.c and does not apply to
lalr1.java.
* data/lalr1.cc (yy::parser::parse): Increase size of
yyerror_range and adjust subscripting so you don't have to
subtract one from the beginning of the array.
* data/yacc.c (b4_declare_parser_state_variables,
yyparse, yypush_parse): Likewise.
---
 ChangeLog        |   13 +++++++++++++
 data/lalr1.cc    |   12 ++++++------
 data/yacc.c      |   12 ++++++------
 src/parse-gram.c |   16 ++++++++--------
 src/parse-gram.h |    2 +-
 5 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 507576b..ec42c73 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2010-04-03  Joel E. Denny  <address@hidden>
+
+       portability: fix pointer arithmetic to conform to C standard.
+       Reported by Tys Lefering at
+       <http://lists.gnu.org/archive/html/bug-bison/2010-03/msg00035.html>.
+       This fix is already implemented in glr.c and does not apply to
+       lalr1.java.
+       * data/lalr1.cc (yy::parser::parse): Increase size of
+       yyerror_range and adjust subscripting so you don't have to
+       subtract one from the beginning of the array.
+       * data/yacc.c (b4_declare_parser_state_variables,
+       yyparse, yypush_parse): Likewise.
+
 2010-03-31  Joel E. Denny  <address@hidden>
 
        portability: fix test suite for GCC 4.5's new #error message.
diff --git a/data/lalr1.cc b/data/lalr1.cc
index 6c66dd1..183ddcd 100644
--- a/data/lalr1.cc
+++ b/data/lalr1.cc
@@ -538,7 +538,7 @@ do {                                        \
     /// Location of the lookahead.
     location_type yylloc;
     /// The locations where the error started and ended.
-    location_type yyerror_range[2];
+    location_type yyerror_range[3];
 
     /// $$.
     semantic_type yyval;
@@ -709,7 +709,7 @@ m4_ifdef([b4_lex_param], [, ]b4_lex_param))[;
 b4_error_verbose_if([, yytoken])[));
       }
 
-    yyerror_range[0] = yylloc;
+    yyerror_range[1] = yylloc;
     if (yyerrstatus_ == 3)
       {
        /* If just tried and failed to reuse lookahead token after an
@@ -744,7 +744,7 @@ b4_error_verbose_if([, yytoken])[));
     if (false)
       goto yyerrorlab;
 
-    yyerror_range[0] = yylocation_stack_[yylen - 1];
+    yyerror_range[1] = yylocation_stack_[yylen - 1];
     /* Do not reclaim the symbols of the rule which action triggered
        this YYERROR.  */
     yypop_ (yylen);
@@ -776,7 +776,7 @@ b4_error_verbose_if([, yytoken])[));
        if (yystate_stack_.height () == 1)
        YYABORT;
 
-       yyerror_range[0] = yylocation_stack_[0];
+       yyerror_range[1] = yylocation_stack_[0];
        yydestruct_ ("Error: popping",
                     yystos_[yystate],
                     &yysemantic_stack_[0], &yylocation_stack_[0]);
@@ -785,10 +785,10 @@ b4_error_verbose_if([, yytoken])[));
        YY_STACK_PRINT ();
       }
 
-    yyerror_range[1] = yylloc;
+    yyerror_range[2] = yylloc;
     // Using YYLLOC is tempting, but would change the location of
     // the lookahead.  YYLOC is available though.
-    YYLLOC_DEFAULT (yyloc, (yyerror_range - 1), 2);
+    YYLLOC_DEFAULT (yyloc, yyerror_range, 2);
     yysemantic_stack_.push (yylval);
     yylocation_stack_.push (yyloc);
 
diff --git a/data/yacc.c b/data/yacc.c
index 67e1840..ed44a4c 100644
--- a/data/yacc.c
+++ b/data/yacc.c
@@ -1072,7 +1072,7 @@ m4_define([b4_declare_parser_state_variables],
     YYLTYPE *yylsp;
 
     /* The locations where the error started and ended.  */
-    YYLTYPE yyerror_range[2];
+    YYLTYPE yyerror_range[3];
 ]])[
     YYSIZE_T yystacksize;
 ]])
@@ -1533,7 +1533,7 @@ yyerrlab:
 #endif
     }
 
-]b4_locations_if([[  yyerror_range[0] = yylloc;]])[
+]b4_locations_if([[  yyerror_range[1] = yylloc;]])[
 
   if (yyerrstatus == 3)
     {
@@ -1570,7 +1570,7 @@ yyerrorlab:
   if (/*CONSTCOND*/ 0)
      goto yyerrorlab;
 
-]b4_locations_if([[  yyerror_range[0] = yylsp[1-yylen];
+]b4_locations_if([[  yyerror_range[1] = yylsp[1-yylen];
 ]])[  /* Do not reclaim the symbols of the rule which action triggered
      this YYERROR.  */
   YYPOPSTACK (yylen);
@@ -1604,7 +1604,7 @@ yyerrlab1:
       if (yyssp == yyss)
        YYABORT;
 
-]b4_locations_if([[      yyerror_range[0] = *yylsp;]])[
+]b4_locations_if([[      yyerror_range[1] = *yylsp;]])[
       yydestruct ("Error: popping",
                  yystos[yystate], yyvsp]b4_locations_if([, 
yylsp])[]b4_user_args[);
       YYPOPSTACK (1);
@@ -1614,10 +1614,10 @@ yyerrlab1:
 
   *++yyvsp = yylval;
 ]b4_locations_if([[
-  yyerror_range[1] = yylloc;
+  yyerror_range[2] = yylloc;
   /* Using YYLLOC is tempting, but would change the location of
      the lookahead.  YYLOC is available though.  */
-  YYLLOC_DEFAULT (yyloc, (yyerror_range - 1), 2);
+  YYLLOC_DEFAULT (yyloc, yyerror_range, 2);
   *++yylsp = yyloc;]])[
 
   /* Shift the error token.  */
diff --git a/src/parse-gram.c b/src/parse-gram.c
index 04a934e..34c2802 100644
--- a/src/parse-gram.c
+++ b/src/parse-gram.c
@@ -1,4 +1,4 @@
-/* A Bison parser, made by GNU Bison 2.4.1.61-f1ce.  */
+/* A Bison parser, made by GNU Bison 2.4.2.5-1849.  */
 
 /* Skeleton implementation for Bison's Yacc-like parsers in C
    
@@ -45,7 +45,7 @@
 #define YYBISON 1
 
 /* Bison version.  */
-#define YYBISON_VERSION "2.4.1.61-f1ce"
+#define YYBISON_VERSION "2.4.2.5-1849"
 
 /* Skeleton name.  */
 #define YYSKELETON_NAME "yacc.c"
@@ -1640,7 +1640,7 @@ YYLTYPE yylloc;
     YYLTYPE *yylsp;
 
     /* The locations where the error started and ended.  */
-    YYLTYPE yyerror_range[2];
+    YYLTYPE yyerror_range[3];
 
     YYSIZE_T yystacksize;
 
@@ -2703,7 +2703,7 @@ yyerrlab:
 #endif
     }
 
-  yyerror_range[0] = yylloc;
+  yyerror_range[1] = yylloc;
 
   if (yyerrstatus == 3)
     {
@@ -2740,7 +2740,7 @@ yyerrorlab:
   if (/*CONSTCOND*/ 0)
      goto yyerrorlab;
 
-  yyerror_range[0] = yylsp[1-yylen];
+  yyerror_range[1] = yylsp[1-yylen];
   /* Do not reclaim the symbols of the rule which action triggered
      this YYERROR.  */
   YYPOPSTACK (yylen);
@@ -2774,7 +2774,7 @@ yyerrlab1:
       if (yyssp == yyss)
        YYABORT;
 
-      yyerror_range[0] = *yylsp;
+      yyerror_range[1] = *yylsp;
       yydestruct ("Error: popping",
                  yystos[yystate], yyvsp, yylsp);
       YYPOPSTACK (1);
@@ -2784,10 +2784,10 @@ yyerrlab1:
 
   *++yyvsp = yylval;
 
-  yyerror_range[1] = yylloc;
+  yyerror_range[2] = yylloc;
   /* Using YYLLOC is tempting, but would change the location of
      the lookahead.  YYLOC is available though.  */
-  YYLLOC_DEFAULT (yyloc, (yyerror_range - 1), 2);
+  YYLLOC_DEFAULT (yyloc, yyerror_range, 2);
   *++yylsp = yyloc;
 
   /* Shift the error token.  */
diff --git a/src/parse-gram.h b/src/parse-gram.h
index 133555d..7be84a9 100644
--- a/src/parse-gram.h
+++ b/src/parse-gram.h
@@ -1,4 +1,4 @@
-/* A Bison parser, made by GNU Bison 2.4.1.61-f1ce.  */
+/* A Bison parser, made by GNU Bison 2.4.2.5-1849.  */
 
 /* Skeleton interface for Bison's Yacc-like parsers in C
    
-- 
1.5.4.3