Re: new compiler warning with bison 2.6.5 (PostgreSQL)

[Akim Demaille]

Hi Peter,

Sorry for the delays,

Le 17 nov. 2012 à 06:08, Peter Eisentraut <address@hidden> a écrit :

> When building the SQL grammar in the PostgreSQL source with bison 2.6.5,
> a new compiler warning appears:
> 
> bison -d -o gram.c gram.y
> gcc -O2 -Wlogical-op -Wall -Wmissing-prototypes -Wpointer-arith 
> -Wdeclaration-after-statement -Wendif-labels -Wlogical-op 
> -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv 
> -fexcess-precision=standard -g -Wno-error -I. -I. -I../../../src/include 
> -D_FORTIFY_SOURCE=2 -DLINUX_OOM_ADJ -D_GNU_SOURCE -I/usr/include/libxml2   -c 
> -o gram.o gram.c -MMD -MP -MF .deps/gram.Po
> In file included from gram.y:13561:0:
> ### this is the fault of flex
> scan.c: In function ‘yy_try_NUL_trans’:
> scan.c:16257:23: warning: unused variable ‘yyg’ [-Wunused-variable]
> ### this one is new
> gram.y: In function ‘base_yyparse’:
> gram.c:19961:12: warning: ‘base_yylloc’ is used uninitialized in this 
> function [-Wuninitialized]
> gram.c:19886:5: note: ‘base_yylloc’ was declared here
> 
> I have attached the source and the processed file.  It's pretty easy to
> visually verify the problem using the line numbers.
> 
> If you want to reproduce the whole thing in context, you can do:
> 
> git clone git://git.postgresql.org/git/postgresql.git
> cd postgresql
> ./configure BISON=some/where/bison
> make

Thanks for the detailed report.  We have been able to reproduce it
within the test suite.  I propose the following changes to address
the warning.  I have checked that it solves both the case in the test
suite, and the case of PostgreSQL.

Nevertheless, opinion would be welcome!  Otherwise I'll install it next
week, and proceed with a bug fix release 2.6.6.

commit fb4c8a7cb97844f6c66921a77e79311a19d12fc2
Author: Theophile Ranquet <address@hidden>
Date:   Mon Nov 19 10:43:56 2012 +0000

    yacc.c: always initialize yylloc
    
    The initial location might be used if the parser starts by an empty
    reduction, so really ensure proper initialization of the initial
    location.  The previous approach fails for PostgreSQL, which uses
    Reported by Peter Eisentraut.
    http://lists.gnu.org/archive/html/bug-bison/2012-11/msg00023.html
    With help from Théophile Ranquet.
    
    * data/yacc.c (b4_declare_scanner_communication_variables): Be sure
    to initialize yylloc, even when its structure is unknown.
    (yyparse): Simplify the call to b4_dollar_pushdef.
    * tests/actions.at (Initial location): Check of similar pattern
    as in the case of PostgreSQL.

diff --git a/NEWS b/NEWS
index 2952b63..393b892 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,10 @@ GNU Bison NEWS
 
 * Noteworthy changes in release ?.? (????-??-??) [?]
 
+** Bug fixes
+
+  Warnings about uninitialized yylloc in yyparse have been fixed.
+
 ** Documentation
 
   The sections about shift/reduce and reduce/reduce conflicts resolution
diff --git a/THANKS b/THANKS
index b12bb7e..e87568d 100644
--- a/THANKS
+++ b/THANKS
@@ -88,6 +88,7 @@ Pascal Bart               address@hidden
 Paul Eggert               address@hidden
 Paul Hilfinger            address@hidden
 Per Allansson             address@hidden
+Peter Eisentraut          address@hidden
 Peter Fales               address@hidden
 Peter Hamorsky            address@hidden
 Peter Simons              address@hidden
diff --git a/data/yacc.c b/data/yacc.c
index 2e0ecc2..bbc9d91 100644
--- a/data/yacc.c
+++ b/data/yacc.c
@@ -184,7 +184,8 @@ int yychar;
    or non-GCC compilers.  */
 static YYSTYPE yyval_default;
 # define YY_INITIAL_VALUE(Value) = Value
-#endif]])[
+#endif]b4_locations_if([[
+static YYLTYPE yyloc_default][]b4_yyloc_default[;]])])[
 #ifndef YY_IGNORE_MAYBE_UNINITIALIZED_BEGIN
 # define YY_IGNORE_MAYBE_UNINITIALIZED_BEGIN
 # define YY_IGNORE_MAYBE_UNINITIALIZED_END
@@ -197,7 +198,7 @@ static YYSTYPE yyval_default;
 YYSTYPE yylval YY_INITIAL_VALUE(yyval_default);]b4_locations_if([[
 
 /* Location data for the lookahead symbol.  */
-YYLTYPE yylloc][]b4_yyloc_default[;
+YYLTYPE yylloc]b4_pure_if([ = yyloc_default], [b4_yyloc_default])[;
 ]])b4_pure_if([], [[
 
 /* Number of syntax errors so far.  */
@@ -1410,7 +1411,8 @@ b4_c_function_def([[yyparse]], [[int]], b4_parse_param)[
   yypstate *yyps_local;]b4_pure_if([[
   int yychar;
   YYSTYPE yylval;]b4_locations_if([[
-  YYLTYPE yylloc][]b4_yyloc_default[;]])])[
+  static YYLTYPE yyloc_default][]b4_yyloc_default[;
+  YYLTYPE yylloc = yyloc_default;]])])[
   if (yyps)
     yyps_local = yyps;
   else
@@ -1559,8 +1561,7 @@ b4_c_function_def([[yyparse]], [[int]], b4_parse_param)[
   yychar = YYEMPTY; /* Cause a token to be read.  */
 ]m4_ifdef([b4_initial_action], [
 b4_dollar_pushdef([m4_define([b4_dollar_dollar_used])yylval], [],
-               [m4_define([b4_at_dollar_used])dnl
-b4_push_if([b4_pure_if([*])yypushed_loc], [yylloc])])dnl
+                  [b4_push_if([b4_pure_if([*])yypushed_loc], [yylloc])])dnl
 /* User initialization code.  */
 b4_user_initial_action
 b4_dollar_popdef[]dnl
diff --git a/tests/actions.at b/tests/actions.at
index ec88cb9..78977c8 100644
--- a/tests/actions.at
+++ b/tests/actions.at
@@ -73,8 +73,8 @@ AT_CLEANUP
 ## Initial location.  ##
 ## ------------------ ##
 
-# AT_TEST(SKELETON-NAME, DIRECTIVES)
-# ----------------------------------
+# AT_TEST(SKELETON-NAME, DIRECTIVES, [MORE-DIRECTIVES], [LOCATION = 1.1])
+# -----------------------------------------------------------------------
 # Check that the initial location is correct.
 m4_pushdef([AT_TEST],
 [AT_SETUP([Initial location: $1 $2])
@@ -85,7 +85,8 @@ AT_DATA_GRAMMAR([[input.y]],
 %locations
 %debug
 %skeleton "$1"
-$2
+]$2[
+]$3[
 %parse-param { int x } // Useless, but used to force yyerror purity.
 %code
 {
@@ -122,8 +123,8 @@ main (void)
 
 AT_FULL_COMPILE([input])
 AT_PARSER_CHECK([./input], 1, [],
-[[1.1
-1.1: syntax error
+[m4_default([$4], [1.1])
+m4_default([$4], [1.1])[: syntax error
 ]])
 AT_BISON_OPTION_POPDEFS
 AT_CLEANUP
@@ -138,6 +139,36 @@ AT_TEST([glr.c])
 AT_TEST([lalr1.cc])
 AT_TEST([glr.cc])
 
+## A very different test, based on PostgreSQL's implementation of the
+## locations.  See
+## http://lists.gnu.org/archive/html/bug-bison/2012-11/msg00023.html
+##
+## Weirdly enough, to trigger the warning with GCC 4.7, we must not
+## use fprintf, so run the test twice: once to check the warning
+## (absence thereof), and another time to check the value.
+AT_TEST([yacc.c], [%define api.pure],
+[[%{
+# define YYLTYPE int
+# define YY_LOCATION_PRINT(Stream, Loc)      \
+   (void) (Loc)
+# define YYLLOC_DEFAULT(Current, Rhs, N)    \
+  (Current) = ((Rhs)[N ? 1 : 0])
+%}
+]],
+[@&address@hidden)
+
+AT_TEST([yacc.c], [%define api.pure],
+[[%{
+# define YYLTYPE int
+# define YY_LOCATION_PRINT(Stream, Loc)      \
+    fprintf ((Stream), "%d", (Loc))
+# define YYLLOC_DEFAULT(Current, Rhs, N)    \
+  (Current) = ((Rhs)[N ? 1 : 0])
+%}
+]],
+[0])
+
+
 m4_popdef([AT_TEST])