SELinux Archive on lore.kernel.org
 help / Atom feed
* gcc 9.0.0 build issues
@ 2019-02-01 19:34 Petr Lautrbach
  2019-02-01 20:24 ` Ondrej Mosnacek
  0 siblings, 1 reply; 12+ messages in thread
From: Petr Lautrbach @ 2019-02-01 19:34 UTC (permalink / raw)
  To: selinux\


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)

$ make DESTDIR=~/obj install install-pywrap
...

make[2]: Entering directory 
'/home/build/SELinuxProject-selinux/libsepol/src'
cc -O2 -Werror -Wall -Wextra -Wmissing-format-attribute 
-Wmissing-noreturn -Wpointer-arith -Wshadow -Wstrict-prototypes 
-Wundef -Wunused -Wwrite-strings -I. -I../include -D_GNU_SOURCE 
-I../cil/include -fPIC -c -o genbools.o genbools.c
In function ‘strtrim’,
    inlined from ‘process_boolean.constprop’ at genbools.c:52:2:
genbools.c:24:2: error: ‘strncpy’ output may be truncated copying 
8191 bytes from a string of length 8191 
[-Werror=stringop-truncation]
   24 |  strncpy(dest, ptr, size);
      |  ^~~~~~~~~~~~~~~~~~~~~~~~

...

make[2]: Entering directory 
'/home/build/SELinuxProject-selinux/libsepol/src'
cc -O2 -Werror -Wall -Wextra -Wmissing-format-attribute 
-Wmissing-noreturn -Wpointer-arith -Wshadow -Wstrict-prototypes 
-Wundef -Wunused -Wwrite-strings -I. -I../include -D_GNU_SOURCE 
-I../cil/include -fPIC -DSHARED -c -o genbools.lo genbools.c
In function ‘strtrim’,
    inlined from ‘process_boolean.constprop’ at genbools.c:52:2:
genbools.c:24:2: error: ‘strncpy’ output may be truncated copying 
8191 bytes from a string of length 8191 
[-Werror=stringop-truncation]
   24 |  strncpy(dest, ptr, size);
      |  ^~~~~~~~~~~~~~~~~~~~~~~~

...

make[2]: Entering directory 
'/home/build/SELinuxProject-selinux/libselinux/src'
cc -O2 -Werror -Wall -Wextra -Wmissing-format-attribute 
-Wmissing-noreturn -Wpointer-arith -Wshadow -Wstrict-prototypes 
-Wundef -Wunused -Wwrite-strings -I../include -D_GNU_SOURCE 
-DNO_ANDROID_BACKEND   -c -o booleans.o booleans.c
In function ‘strtrim’,
    inlined from ‘process_boolean’ at booleans.c:362:2:
booleans.c:335:2: error: ‘strncpy’ output may be truncated copying 
between 8188 and 8191 bytes from a string of length 8191 
[-Werror=stringop-truncation]
  335 |  strncpy(dest, ptr, size);
      |  ^~~~~~~~~~~~~~~~~~~~~~~~

...



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



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: gcc 9.0.0 build issues
  2019-02-01 19:34 gcc 9.0.0 build issues Petr Lautrbach
@ 2019-02-01 20:24 ` Ondrej Mosnacek
  2019-02-07 12:40   ` Petr Lautrbach
  2019-02-07 15:32   ` Ondrej Mosnacek
  0 siblings, 2 replies; 12+ messages in thread
From: Ondrej Mosnacek @ 2019-02-01 20:24 UTC (permalink / raw)
  To: Petr Lautrbach; +Cc: selinux

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)
>
> $ make DESTDIR=~/obj install install-pywrap
> ...
>
> make[2]: Entering directory
> '/home/build/SELinuxProject-selinux/libsepol/src'
> cc -O2 -Werror -Wall -Wextra -Wmissing-format-attribute
> -Wmissing-noreturn -Wpointer-arith -Wshadow -Wstrict-prototypes
> -Wundef -Wunused -Wwrite-strings -I. -I../include -D_GNU_SOURCE
> -I../cil/include -fPIC -c -o genbools.o genbools.c
> In function ‘strtrim’,
>     inlined from ‘process_boolean.constprop’ at genbools.c:52:2:
> genbools.c:24:2: error: ‘strncpy’ output may be truncated copying
> 8191 bytes from a string of length 8191
> [-Werror=stringop-truncation]
>    24 |  strncpy(dest, ptr, size);
>       |  ^~~~~~~~~~~~~~~~~~~~~~~~
>

This is a typical strncpy() abuse. See 'man strncpy' -> /NOTES for how
to handle the corner case correctly (same for similar issues below).

> ...
>
> make[2]: Entering directory
> '/home/build/SELinuxProject-selinux/libsepol/src'
> cc -O2 -Werror -Wall -Wextra -Wmissing-format-attribute
> -Wmissing-noreturn -Wpointer-arith -Wshadow -Wstrict-prototypes
> -Wundef -Wunused -Wwrite-strings -I. -I../include -D_GNU_SOURCE
> -I../cil/include -fPIC -DSHARED -c -o genbools.lo genbools.c
> In function ‘strtrim’,
>     inlined from ‘process_boolean.constprop’ at genbools.c:52:2:
> genbools.c:24:2: error: ‘strncpy’ output may be truncated copying
> 8191 bytes from a string of length 8191
> [-Werror=stringop-truncation]
>    24 |  strncpy(dest, ptr, size);
>       |  ^~~~~~~~~~~~~~~~~~~~~~~~
>
> ...
>
> make[2]: Entering directory
> '/home/build/SELinuxProject-selinux/libselinux/src'
> cc -O2 -Werror -Wall -Wextra -Wmissing-format-attribute
> -Wmissing-noreturn -Wpointer-arith -Wshadow -Wstrict-prototypes
> -Wundef -Wunused -Wwrite-strings -I../include -D_GNU_SOURCE
> -DNO_ANDROID_BACKEND   -c -o booleans.o booleans.c
> In function ‘strtrim’,
>     inlined from ‘process_boolean’ at booleans.c:362:2:
> booleans.c:335:2: error: ‘strncpy’ output may be truncated copying
> between 8188 and 8191 bytes from a string of length 8191
> [-Werror=stringop-truncation]
>   335 |  strncpy(dest, ptr, size);
>       |  ^~~~~~~~~~~~~~~~~~~~~~~~
>
> ...
>
>
>
> 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...

--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: gcc 9.0.0 build issues
  2019-02-01 20:24 ` Ondrej Mosnacek
@ 2019-02-07 12:40   ` Petr Lautrbach
  2019-02-07 17:52     ` Roberts, William C
  2019-02-07 15:32   ` Ondrej Mosnacek
  1 sibling, 1 reply; 12+ messages in thread
From: Petr Lautrbach @ 2019-02-07 12:40 UTC (permalink / raw)
  To: selinux\; +Cc: Petr Lautrbach, William Roberts, Ondrej Mosnacek


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?



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: gcc 9.0.0 build issues
  2019-02-01 20:24 ` Ondrej Mosnacek
  2019-02-07 12:40   ` Petr Lautrbach
@ 2019-02-07 15:32   ` Ondrej Mosnacek
  1 sibling, 0 replies; 12+ messages in thread
From: Ondrej Mosnacek @ 2019-02-07 15:32 UTC (permalink / raw)
  To: Petr Lautrbach; +Cc: selinux

On Fri, Feb 1, 2019 at 9:24 PM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> 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)
> >
> > $ make DESTDIR=~/obj install install-pywrap
> > ...
> >
> > make[2]: Entering directory
> > '/home/build/SELinuxProject-selinux/libsepol/src'
> > cc -O2 -Werror -Wall -Wextra -Wmissing-format-attribute
> > -Wmissing-noreturn -Wpointer-arith -Wshadow -Wstrict-prototypes
> > -Wundef -Wunused -Wwrite-strings -I. -I../include -D_GNU_SOURCE
> > -I../cil/include -fPIC -c -o genbools.o genbools.c
> > In function ‘strtrim’,
> >     inlined from ‘process_boolean.constprop’ at genbools.c:52:2:
> > genbools.c:24:2: error: ‘strncpy’ output may be truncated copying
> > 8191 bytes from a string of length 8191
> > [-Werror=stringop-truncation]
> >    24 |  strncpy(dest, ptr, size);
> >       |  ^~~~~~~~~~~~~~~~~~~~~~~~
> >
>
> This is a typical strncpy() abuse. See 'man strncpy' -> /NOTES for how
> to handle the corner case correctly (same for similar issues below).

Actually, upon closer look it looks like there is no abuse and the
warning simply warns that only a part of the string might be copied
into the destination. I suppose we don't care about that possibility
here, so the best way to resolve the warning might be to just disable
the warning for this part of the code.

>
> > ...
> >
> > make[2]: Entering directory
> > '/home/build/SELinuxProject-selinux/libsepol/src'
> > cc -O2 -Werror -Wall -Wextra -Wmissing-format-attribute
> > -Wmissing-noreturn -Wpointer-arith -Wshadow -Wstrict-prototypes
> > -Wundef -Wunused -Wwrite-strings -I. -I../include -D_GNU_SOURCE
> > -I../cil/include -fPIC -DSHARED -c -o genbools.lo genbools.c
> > In function ‘strtrim’,
> >     inlined from ‘process_boolean.constprop’ at genbools.c:52:2:
> > genbools.c:24:2: error: ‘strncpy’ output may be truncated copying
> > 8191 bytes from a string of length 8191
> > [-Werror=stringop-truncation]
> >    24 |  strncpy(dest, ptr, size);
> >       |  ^~~~~~~~~~~~~~~~~~~~~~~~
> >
> > ...
> >
> > make[2]: Entering directory
> > '/home/build/SELinuxProject-selinux/libselinux/src'
> > cc -O2 -Werror -Wall -Wextra -Wmissing-format-attribute
> > -Wmissing-noreturn -Wpointer-arith -Wshadow -Wstrict-prototypes
> > -Wundef -Wunused -Wwrite-strings -I../include -D_GNU_SOURCE
> > -DNO_ANDROID_BACKEND   -c -o booleans.o booleans.c
> > In function ‘strtrim’,
> >     inlined from ‘process_boolean’ at booleans.c:362:2:
> > booleans.c:335:2: error: ‘strncpy’ output may be truncated copying
> > between 8188 and 8191 bytes from a string of length 8191
> > [-Werror=stringop-truncation]
> >   335 |  strncpy(dest, ptr, size);
> >       |  ^~~~~~~~~~~~~~~~~~~~~~~~
> >
> > ...
> >
> >
> >
> > 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...
>
> --
> Ondrej Mosnacek <omosnace at redhat dot com>
> Associate Software Engineer, Security Technologies
> Red Hat, Inc.

-- 
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: gcc 9.0.0 build issues
  2019-02-07 12:40   ` Petr Lautrbach
@ 2019-02-07 17:52     ` Roberts, William C
  2019-02-07 18:17       ` Stephen Smalley
  0 siblings, 1 reply; 12+ messages in thread
From: Roberts, William C @ 2019-02-07 17:52 UTC (permalink / raw)
  To: Petr Lautrbach, selinux, sds; +Cc: Ondrej Mosnacek



> -----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?



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: gcc 9.0.0 build issues
  2019-02-07 17:52     ` Roberts, William C
@ 2019-02-07 18:17       ` Stephen Smalley
  2019-02-07 18:18         ` Roberts, William C
  2019-02-08 19:40         ` Roberts, William C
  0 siblings, 2 replies; 12+ messages in thread
From: Stephen Smalley @ 2019-02-07 18:17 UTC (permalink / raw)
  To: Roberts, William C, Petr Lautrbach, selinux; +Cc: Ondrej Mosnacek

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...



^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: gcc 9.0.0 build issues
  2019-02-07 18:17       ` Stephen Smalley
@ 2019-02-07 18:18         ` Roberts, William C
  2019-02-07 18:20           ` Stephen Smalley
  2019-02-08 19:40         ` Roberts, William C
  1 sibling, 1 reply; 12+ messages in thread
From: Roberts, William C @ 2019-02-07 18:18 UTC (permalink / raw)
  To: Stephen Smalley, Petr Lautrbach, selinux; +Cc: Ondrej Mosnacek



> -----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?


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: gcc 9.0.0 build issues
  2019-02-07 18:18         ` Roberts, William C
@ 2019-02-07 18:20           ` Stephen Smalley
  2019-02-07 18:22             ` Roberts, William C
  0 siblings, 1 reply; 12+ messages in thread
From: Stephen Smalley @ 2019-02-07 18:20 UTC (permalink / raw)
  To: Roberts, William C, Petr Lautrbach, selinux; +Cc: Ondrej Mosnacek

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.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: gcc 9.0.0 build issues
  2019-02-07 18:20           ` Stephen Smalley
@ 2019-02-07 18:22             ` Roberts, William C
  0 siblings, 0 replies; 12+ messages in thread
From: Roberts, William C @ 2019-02-07 18:22 UTC (permalink / raw)
  To: Stephen Smalley, Petr Lautrbach, selinux; +Cc: Ondrej Mosnacek



> -----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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: gcc 9.0.0 build issues
  2019-02-07 18:17       ` Stephen Smalley
  2019-02-07 18:18         ` Roberts, William C
@ 2019-02-08 19:40         ` Roberts, William C
  2019-02-08 20:10           ` Ondrej Mosnacek
  2019-02-12 16:37           ` Petr Lautrbach
  1 sibling, 2 replies; 12+ messages in thread
From: Roberts, William C @ 2019-02-08 19:40 UTC (permalink / raw)
  To: Stephen Smalley, Petr Lautrbach, selinux; +Cc: Ondrej Mosnacek



> -----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?

I would also diff the object file with -frwapv to see if it is producing different code for that loop.

Bill

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: gcc 9.0.0 build issues
  2019-02-08 19:40         ` Roberts, William C
@ 2019-02-08 20:10           ` Ondrej Mosnacek
  2019-02-12 16:37           ` Petr Lautrbach
  1 sibling, 0 replies; 12+ messages in thread
From: Ondrej Mosnacek @ 2019-02-08 20:10 UTC (permalink / raw)
  To: Roberts, William C; +Cc: Stephen Smalley, Petr Lautrbach, selinux

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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: gcc 9.0.0 build issues
  2019-02-08 19:40         ` Roberts, William C
  2019-02-08 20:10           ` Ondrej Mosnacek
@ 2019-02-12 16:37           ` Petr Lautrbach
  1 sibling, 0 replies; 12+ messages in thread
From: Petr Lautrbach @ 2019-02-12 16:37 UTC (permalink / raw)
  To: Roberts\, William C; +Cc: Stephen Smalley, selinux\, Ondrej Mosnacek

"Roberts, William C" <william.c.roberts@intel.com> writes:

>> -----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?
>
> I would also diff the object file with -frwapv to see if it is producing different code for that loop.
>
> Bill

Fedora gcc is built with
--with-bugurl=http://bugzilla.redhat.com/bugzilla therefore I filed bug
there: https://bugzilla.redhat.com/show_bug.cgi?id=1676607

Petr

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, back to index

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-01 19:34 gcc 9.0.0 build issues Petr Lautrbach
2019-02-01 20:24 ` Ondrej Mosnacek
2019-02-07 12:40   ` Petr Lautrbach
2019-02-07 17:52     ` Roberts, William C
2019-02-07 18:17       ` Stephen Smalley
2019-02-07 18:18         ` Roberts, William C
2019-02-07 18:20           ` Stephen Smalley
2019-02-07 18:22             ` Roberts, William C
2019-02-08 19:40         ` Roberts, William C
2019-02-08 20:10           ` Ondrej Mosnacek
2019-02-12 16:37           ` Petr Lautrbach
2019-02-07 15:32   ` Ondrej Mosnacek

SELinux Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/selinux/0 selinux/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 selinux selinux/ https://lore.kernel.org/selinux \
		selinux@vger.kernel.org selinux@archiver.kernel.org
	public-inbox-index selinux


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.selinux


AGPL code for this site: git clone https://public-inbox.org/ public-inbox