Re: Bash build issues in `devel' branch due to -Werror compiler flag

[Chet Ramey]

On 12/16/18 2:35 AM, Eduardo A. Bustamante López wrote:
> Commit 9d80be9ab5cc17011c634e0348c64c15fcba95bf adds the following compiler 
> flag:
> 
>   dualbus@debian:~/src/gnu/bash$ cat -n configure.ac | grep Werror -C3
>     1159              CFLAGS="$CFLAGS -Wno-parentheses -Wno-format-security"
>     1160              if test -n "$DEBUG"
>     1161              then
>   ~>1162                      CFLAGS="$CFLAGS -Werror"
>     1163              fi
>     1164      fi
>     1165      

Good. This is the kind of feedback I want from enabling this option during
the pre-release period. Thanks for taking the time.

> During build (with gcc):
> 
>   dualbus@debian:~/src/gnu/bash$ make -j$(nproc) -s
>   make[1]: warning: -j4 forced in submake: resetting jobserver mode.
>   ./parse.y: warning: 1 shift/reduce conflict [-Wconflicts-sr]
>   make[1]: warning: -j4 forced in submake: resetting jobserver mode.
>   expr.c:217:17: error: conflicting types for built-in function ‘exp2’ 
> [-Werror=builtin-declaration-mismatch]
>    static intmax_t exp2 __P((void));
>                    ^~~~
>   cc1: all warnings being treated as errors
> 
> 
> I know the `exp2' function here has nothing to do with the built-in 
> exponential
> function, and that it has had that name for a long time, but the build breaks 
> due to that.

Interesting. There seems to be a pretty wide variance between the different
versions of gcc and clang. clang, its other faults aside, doesn't complain
about this (e.g., "Apple LLVM version 10.0.0 (clang-1000.11.45.5)"), while
gcc (e.g., gcc-8.2.1) does.

>   dualbus@debian:~/src/gnu/bash$ make -j$(nproc) -s
>   make[1]: warning: -j4 forced in submake: resetting jobserver mode.
>   
>         ***********************************************************
>         *                                                         *
>         * GNU bash, version 5.0.0(1)-rc1 (x86_64-pc-linux-gnu)
>         *                                                         *
>         ***********************************************************
>   
>   making lib/glob/libglob.a in ./lib/glob
>   make[1]: warning: -j4 forced in submake: resetting jobserver mode.
>   smatch.c: In function ‘_fnmatch_fallback_wc’:
>   smatch.c:318:11: error: implicit declaration of function ‘fnmatch’; did you 
> mean ‘gmatch’? [-Werror=implicit-function-declaration]
>      return (fnmatch ((const char *)w2, (const char *)w1, 0));
>              ^~~~~~~
>              gmatch
> 
> 
> Which I believe is an actual issue and I assume the fix is along the lines of:

It needs a different fix, but there should be a declaration in scope there.


> I also tried clang version 7.0.1-+rc3-2, which gives an additional error:
> 
>   making lib/sh/libsh.a in ./lib/sh
>   (...)
>   getenv.c:55:7: error: comparison of nonnull parameter 'name' equal to a 
> null pointer is 'false' on first encounter 
> [-Werror,-Wtautological-pointer-compare]
>     if (name == 0 || *name == '\0')
>         ^~~~    ~
>   /usr/include/stdlib.h:631:50: note: declared 'nonnull' here
>   extern char *getenv (const char *__name) __THROW __nonnull ((1)) __wur;
>                                                    ^
>   /usr/include/x86_64-linux-gnu/sys/cdefs.h:293:44: note: expanded from macro 
> '__nonnull'
>   # define __nonnull(params) __attribute__ ((__nonnull__ params))
> 
> 
> This is due to the stdlib.h header being pulled by bashansi.h in
> lib/sh/getenv.c, thus, providing a function signature that doesn't match 
> Bash's
> re-definition of getenv():

I think this should be allowed: if you allow getenv() to be redefined
(configure checks for this) it should always be ok to redefine it with
a standard POSIX function signature.

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
                 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRU    chet@case.edu    http://tiswww.cwru.edu/~chet/