Re: gcc 9.0.0 build issues

[Ondrej Mosnacek <omosnace@redhat.com>]

On Fri, Feb 8, 2019 at 8:40 PM Roberts, William C
<william.c.roberts@intel.com> 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...
> >
>
> So I looked at filing a bug with GCC, and one thing that helps it get looked at is sample code
> to trigger the problem. I'm not even seeing a GCC 9 release, so I am assuming it's in a dev
> mode?
>
> Since you have it running could you see if you can re-produce the error in a snippet and file the bug?

FWIW, I extracted the relevant part of the code and I managed to
reproduce the warning with this self-contained snippet in Compiler
Explorer:

https://gcc.godbolt.org/z/LlVva9

>
> I would also diff the object file with -frwapv to see if it is producing different code for that loop.
>
> Bill
--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.