Re: FYI: master: c++: fix GCC8 warnings about uninitialized values

[Akim Demaille]

Hi Paul,

This is a place where your expertise would be most welcome!

> Le 15 août 2018 à 20:24, Akim Demaille <address@hidden> a écrit :
> 
> I still have warnings in some other tests, but I’m unsure how to fix them.

All these failures with GCC8 -O3 are due to -Werror=maybe-uninitialized.

163: parse.error=verbose and consistent errors:      FAILED (conflicts.at:625)
165: parse.error=verbose and consistent errors: lr.default-reduction=consistent 
FAILED (conflicts.at:635)
166: parse.error=verbose and consistent errors: lr.default-reduction=accepting 
FAILED (conflicts.at:641)
167: parse.error=verbose and consistent errors: lr.type=canonical-lr FAILED 
(conflicts.at:645)
168: parse.error=verbose and consistent errors: parse.lac=full FAILED 
(conflicts.at:650)
169: parse.error=verbose and consistent errors: parse.lac=full 
lr.default-reduction=accepting FAILED (conflicts.at:655)


Here is a ‘reduced’ test case.

input.y
Description: Binary data

It is quite complex, and I don’t understand what frightens GCC, but I get:

$ ./_build/8s/tests/bison /tmp/input.y
$ gcc-mp-8 -O3 -Wmaybe-uninitialized input.tab.c -c
input.tab.c: In function 'yyparse':
input.tab.c:976:9: warning: 'yylval' may be used uninitialized in this function 
[-Wmaybe-uninitialized]
 YYSTYPE yylval YY_INITIAL_VALUE (= yyval_default);
         ^~~~~~

The test case is quite touchy: if I remove more code, the warning goes away.  
With -O2 it is still there, but at -O1 it goes away.

I believe the warning is wrong.  When reading the report (-rall), I see nothing 
weird that could explain what GCC sees (like an error even before we read a 
token).  I have compiled the executable with address sanitizer (both GCC and 
Clang), and it runs without any error.  Valgrind does not see any problem 
either.  If I add %debug, the warning goes away, but when running the test, 
indeed we do read a token (and set its yylval) before doing anything fancy.

So I guess there is no bug to fix, but only a warning to silence.  My favorite 
approach would have been to do what Paul already did years ago: spread even 
more Pragmas to disable the warning locally, using

#if defined __GNUC__ && 407 <= __GNUC__ * 100 + __GNUC_MINOR__
/* Suppress an incorrect diagnostic about yylval being uninitialized.  */
# define YY_IGNORE_MAYBE_UNINITIALIZED_BEGIN \
    _Pragma ("GCC diagnostic push") \
    _Pragma ("GCC diagnostic ignored \"-Wuninitialized\"")\
    _Pragma ("GCC diagnostic ignored \"-Wmaybe-uninitialized\"")
# define YY_IGNORE_MAYBE_UNINITIALIZED_END \
    _Pragma ("GCC diagnostic pop")
#else
# define YY_IGNORE_MAYBE_UNINITIALIZED_BEGIN
# define YY_IGNORE_MAYBE_UNINITIALIZED_END
#endif

The problem is that I could not find any place where this pragma is effective 
except the definition of yylval itself:

diff --git a/data/yacc.c b/data/yacc.c
index e72b098b..3f262908 100644
--- a/data/yacc.c
+++ b/data/yacc.c
@@ -179,7 +179,9 @@ int yychar;
 /* Default value used for initialization, for pacifying older GCCs
    or non-GCC compilers.  */
 YY_INITIAL_VALUE (static YYSTYPE yyval_default;)
-YYSTYPE yylval YY_INITIAL_VALUE (= yyval_default);]b4_locations_if([[
+YY_IGNORE_MAYBE_UNINITIALIZED_BEGIN
+YYSTYPE yylval YY_INITIAL_VALUE (= yyval_default);
+YY_IGNORE_MAYBE_UNINITIALIZED_END]b4_locations_if([[
 
 /* Location data for the lookahead symbol.  */
 static YYLTYPE yyloc_default]b4_yyloc_default[;


I’m a bit worried about this.  However, on the good side, if I remove the other 
pragmas, I get warnings again, so I guess we did not globally disable the 
warning for yylval.

Alternatively, we could disable the warning just for these tests.  But I guess 
some user could face these warnings in her project, so it would be nice to 
avoid it.

Paul, what’s your opinion on this?