RE: gcc 9.0.0 build issues

["Roberts, William C" <william.c.roberts@intel.com>]

> -----Original Message-----
> From: Stephen Smalley [mailto:sds@tycho.nsa.gov]
> Sent: Thursday, February 7, 2019 10:21 AM
> To: Roberts, William C <william.c.roberts@intel.com>; Petr Lautrbach
> <plautrba@redhat.com>; selinux@vger.kernel.org
> Cc: Ondrej Mosnacek <omosnace@redhat.com>
> Subject: Re: gcc 9.0.0 build issues
> 
> On 2/7/19 1:18 PM, Roberts, William C wrote:
> >
> >
> >> -----Original Message-----
> >> From: Stephen Smalley [mailto:sds@tycho.nsa.gov]
> >> Sent: Thursday, February 7, 2019 10:17 AM
> >> To: Roberts, William C <william.c.roberts@intel.com>; Petr Lautrbach
> >> <plautrba@redhat.com>; selinux@vger.kernel.org
> >> Cc: Ondrej Mosnacek <omosnace@redhat.com>
> >> Subject: Re: gcc 9.0.0 build issues
> >>
> >> On 2/7/19 12:52 PM, Roberts, William C wrote:
> >>>
> >>>
> >>>> -----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?
> >>
> >> AFAICS, the code is correct as is.  Not a fan of rewriting code to
> >> appease overly zealous compilers...
> >>
> > I guess whomever is building can override CFLAGS and drop the value
> > down. We should Probably file a bug with gcc?
> 
> Yes, that would be helpful if someone could do that.
> 
On it