RE: gcc 9.0.0 build issues

From: "Roberts, William C" <william.c.roberts@intel.com>
To: Petr Lautrbach <plautrba@redhat.com>,
	"selinux@vger.kernel.org" <selinux@vger.kernel.org>,
	"sds@tycho.nsa.gov" <sds@tycho.nsa.gov>
Cc: Ondrej Mosnacek <omosnace@redhat.com>
Subject: RE: gcc 9.0.0 build issues
Date: Thu, 7 Feb 2019 17:52:32 +0000	[thread overview]
Message-ID: <476DC76E7D1DF2438D32BFADF679FC5649CDF45F@ORSMSX101.amr.corp.intel.com> (raw)
In-Reply-To: <pjdlg2rydoj.fsf@redhat.com>

> -----Original Message-----
> From: Petr Lautrbach [mailto:plautrba@redhat.com]
> Sent: Thursday, February 7, 2019 4:40 AM
> To: selinux@vger.kernel.org
> Cc: Petr Lautrbach <plautrba@redhat.com>; Roberts, William C
> <william.c.roberts@intel.com>; Ondrej Mosnacek <omosnace@redhat.com>
> Subject: Re: gcc 9.0.0 build issues
> 
> 
> Ondrej Mosnacek <omosnace@redhat.com> writes:
> 
> > On Fri, Feb 1, 2019 at 8:36 PM Petr Lautrbach <plautrba@redhat.com>
> > wrote:
> >> gcc-9.0.0-0.3.fc30.x86_64 from Fedora Rawhide:
> >>
> >> gcc version 9.0.0 20190119 (Red Hat 9.0.0-0.3) (GCC)
> >>
> ...
> >> When libselinux is built separately, other CFLAGS is used:
> >>
> >> $ cd libselinux
> >>
> >> $ make DESTDIR=~/obj install install-pywrap ...
> >>
> >> make[1]: Entering directory
> >> '/home/build/SELinuxProject-selinux/libselinux/src'
> >>
> >> cc -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self
> >> -Wmissing-include-dirs -Wunused -Wunknown-pragmas -Wstrict-aliasing
> >> -Wshadow -Wpointer-arith -Wbad-function-cast -Wcast-align
> >> -Wwrite-strings -Waggregate-return -Wstrict-prototypes
> >> -Wold-style-definition -Wmissing-prototypes -Wmissing-declarations
> >> -Wmissing-noreturn -Wmissing-format-attribute -Wredundant-decls
> >> -Wnested-externs -Winline -Winvalid-pch -Wvolatile-register-var
> >> -Wdisabled-optimization -Wbuiltin-macro-redefined -Wattributes
> >> -Wmultichar -Wdeprecated-declarations -Wdiv-by-zero
> >> -Wdouble-promotion -Wendif-labels -Wextra -Wformat-extra-args
> >> -Wformat-zero-length -Wformat=2 -Wmultichar -Woverflow
> >> -Wpointer-to-int-cast -Wpragmas -Wno-missing-field-initializers
> >> -Wno-sign-compare -Wno-format-nonliteral
> >> -Wframe-larger-than=32768
> >> -fstack-protector-all --param=ssp-buffer-size=4 -fexceptions
> >> -fasynchronous-unwind-tables -fdiagnostics-show-option
> >> -funit-at-a-time -Werror -Wno-aggregate-return -Wno-redundant-decls
> >> -fipa-pure-const -Wlogical-op -Wpacked-bitfield-compat -Wsync-nand
> >> -Wcoverage-mismatch -Wcpp -Wformat-contains-nul -Wnormalized=nfc
> >> -Wsuggest-attribute=const -Wsuggest-attribute=noreturn
> >> -Wsuggest-attribute=pure -Wtrampolines -Wjump-misses-init
> >> -Wno-suggest-attribute=pure -Wno-suggest-attribute=const
> >> -U_FORTIFY_SOURCE
> >> -D_FORTIFY_SOURCE=2
> >> -Wstrict-overflow=5 -I../include -D_GNU_SOURCE
> >> -DNO_ANDROID_BACKEND   -c -o booleans.o booleans.c
> >> booleans.c: In function ‘security_get_boolean_names’:
> >> booleans.c:39:5: error: assuming signed overflow does not occur when
> >> changing X +- C1 cmp C2 to X cmp C2 -+ C1 [-Werror=strict-overflow]
> >>    39 | int security_get_boolean_names(char ***names, int *len)
> >>       |     ^~~~~~~~~~~~~~~~~~~~~~~~~~
> >> cc1: all warnings being treated as errors
> >
> > This one is really weird... Perhaps a bug in GCC? At the very least
> > the warning message and source code location are super confusing,
> > which is a bug on its own...
> 
> It's detected only with -Wstrict-overflow=3 and higher. Makefile in libselinux uses
> level 5 which was added by commit
> 9fe430345 ("Makefile: add -Wstrict-overflow=5 to CFLAGS)
> 
> The problem code is on lines 84 and 85 in
> libselinux/src/booleans.c:
> 
> 84:	for (--i; i >= 0; --i)
> 85:    free(n[i]);
> 
> 
> It could be suppressed by something like this:
> 
> --- a/libselinux/src/booleans.c
> +++ b/libselinux/src/booleans.c
> @@ -39,7 +39,7 @@ static int filename_select(const struct dirent
> *d)
>  int security_get_boolean_names(char ***names, int *len)  {
>         char path[PATH_MAX];
> -       int i, rc;
> +       int i, j, rc;
>         struct dirent **namelist;
>         char **n;
> 
> @@ -81,8 +81,8 @@ int security_get_boolean_names(char ***names, int *len)
>         free(namelist);
>         return rc;
>        bad_freen:
> -       for (--i; i >= 0; --i)
> -               free(n[i]);
> +       for (j = 0; j < i; j++)
> +               free(n[j]);
>         free(n);
>        bad:
>         goto out;
> 
> 
> William, what would you consider to be the right fix in this case?

The previous code looks correct IMO, I can't see an actual problem. Looks like
GCC complaining incorrectly or were missing something. In the case of gcc
Incorrectly complaining I usually take a course of action to work around it, but
Im not sure how other maintainers feel about that @sds anything?