Message ID | 20210926223619.552849-1-maskray@google.com |
---|---|
State | New |
Headers | show |
Series | linux: Fix a non-constant expression in _Static_assert | expand |
On 9/26/21 3:36 PM, Fangrui Song via Libc-alpha wrote: > diff --git a/sysdeps/unix/sysv/linux/opendir.c b/sysdeps/unix/sysv/linux/opendir.c ... > - const size_t allocation_size = 32768; > + enum { allocation_size = 32768 }; Thanks, this is obviously a good fix. It's the usual way I fix this sort of thing, for values that must fit in 'int' range which this one does.
On 2021-09-26, Fangrui Song wrote: >According to C11 6.6p6, `const int` as an operand does not make up a >constant expression. Clang reports an error and GCC reports an error >if fed into the preprocessed output: > >../sysdeps/unix/sysv/linux/opendir.c:107:19: error: static_assert expression is not an integral constant expression > _Static_assert (allocation_size >= sizeof (struct dirent64), > >GCC accepting the unpreprocessed source is a bug. > >Fixes: 4b962c9e859de23b461d61f860dbd3f21311e83a ("linux: Simplify opendir buffer allocation") >--- > sysdeps/unix/sysv/linux/opendir.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/sysdeps/unix/sysv/linux/opendir.c b/sysdeps/unix/sysv/linux/opendir.c >index 48f254d169..88640f44ee 100644 >--- a/sysdeps/unix/sysv/linux/opendir.c >+++ b/sysdeps/unix/sysv/linux/opendir.c >@@ -103,7 +103,7 @@ __alloc_dir (int fd, bool close_fd, int flags, > file system provides a bogus value. */ > enum { max_buffer_size = 1048576 }; > >- const size_t allocation_size = 32768; >+ enum { allocation_size = 32768 }; > _Static_assert (allocation_size >= sizeof (struct dirent64), > "allocation_size < sizeof (struct dirent64)"); > >-- >2.33.0.685.g46640cef36-goog The description needs some adjustment https://www.iso-9899.info/n1570.html#6.6p10 10 An implementation may accept other forms of constant expressions. gcc accepting `const int` is fine as an extension point, but having different behaviors under -O0 and -O2 is not great. #include <stddef.h> int main() { const size_t size = 32768; struct A {int a;}; _Static_assert (size >= sizeof (struct A), ""); } % gcc -c a.c -O2 # no error % gcc -c a.c -O0 a.c: In function ‘main’: a.c:5:24: error: expression in static assertion is not constant 5 | _Static_assert (size >= sizeof (struct A), ""); |
* Paul Eggert: > On 9/26/21 3:36 PM, Fangrui Song via Libc-alpha wrote: >> diff --git a/sysdeps/unix/sysv/linux/opendir.c b/sysdeps/unix/sysv/linux/opendir.c > ... >> - const size_t allocation_size = 32768; >> + enum { allocation_size = 32768 }; > > Thanks, this is obviously a good fix. It's the usual way I fix this > sort of thing, for values that must fit in 'int' range which this one > does. I think GCC supports this for all integer types as an extension. Thanks, Florian
* Fangrui Song via Libc-alpha: > The description needs some adjustment > https://www.iso-9899.info/n1570.html#6.6p10 > > 10 An implementation may accept other forms of constant expressions. Yeah, but the change itself is clearly correct. Thanks. Florian
On Mon, Sep 27, 2021 at 6:09 AM Florian Weimer <fweimer@redhat.com> wrote: > > * Fangrui Song via Libc-alpha: > > > The description needs some adjustment > > https://www.iso-9899.info/n1570.html#6.6p10 > > > > 10 An implementation may accept other forms of constant expressions. > > Yeah, but the change itself is clearly correct. Thanks. > > Florian > Ugh, you may have missed my point that GCC -O0 and -O2 are not consistent on whether a `const int` operand can be used in a constant expression. Updated description: --- linux: Fix a non-constant expression in _Static_assert According to C11 6.6p6, `const int` as an operand may not make up a constant expression. GCC's -O0 and -O2 optimization levels are not consistent on whether this operand can be used in a constant expression: ../sysdeps/unix/sysv/linux/opendir.c:107:19: error: static_assert expression is not an integral constant expression _Static_assert (allocation_size >= sizeof (struct dirent64), Use enum which is guaranteed to be a constant expression. This fixes an error when compiling with GCC -O0 and Clang. Fixes: 4b962c9e859de23b461d61f860dbd3f21311e83a ("linux: Simplify opendir buffer allocation")
On Mon, Sep 27, 2021 at 10:18 AM Fāng-ruì Sòng via Libc-alpha <libc-alpha@sourceware.org> wrote: > > On Mon, Sep 27, 2021 at 6:09 AM Florian Weimer <fweimer@redhat.com> wrote: > > > > * Fangrui Song via Libc-alpha: > > > > > The description needs some adjustment > > > https://www.iso-9899.info/n1570.html#6.6p10 > > > > > > 10 An implementation may accept other forms of constant expressions. > > > > Yeah, but the change itself is clearly correct. Thanks. > > > > Florian > > > > Ugh, you may have missed my point that GCC -O0 and -O2 are not > consistent on whether a `const int` operand can be used in a constant > expression. Yes and that was fixed for GCC 8 by r8-4755. Since you were not specific on which version of GCC, I am assuming it was GCC7.x. Thanks, Andrew Pinski > > Updated description: > > --- > > linux: Fix a non-constant expression in _Static_assert > > According to C11 6.6p6, `const int` as an operand may not make up a > constant expression. GCC's -O0 and -O2 optimization levels are not > consistent on whether this operand can be used in a constant expression: > > ../sysdeps/unix/sysv/linux/opendir.c:107:19: error: static_assert > expression is not an integral constant expression > _Static_assert (allocation_size >= sizeof (struct dirent64), > > Use enum which is guaranteed to be a constant expression. > This fixes an error when compiling with GCC -O0 and Clang. > > Fixes: 4b962c9e859de23b461d61f860dbd3f21311e83a ("linux: Simplify > opendir buffer allocation")
On 2021-09-27, Andrew Pinski wrote: >On Mon, Sep 27, 2021 at 10:18 AM Fāng-ruì Sòng via Libc-alpha ><libc-alpha@sourceware.org> wrote: >> >> On Mon, Sep 27, 2021 at 6:09 AM Florian Weimer <fweimer@redhat.com> wrote: >> > >> > * Fangrui Song via Libc-alpha: >> > >> > > The description needs some adjustment >> > > https://www.iso-9899.info/n1570.html#6.6p10 >> > > >> > > 10 An implementation may accept other forms of constant expressions. >> > >> > Yeah, but the change itself is clearly correct. Thanks. >> > >> > Florian >> > >> >> Ugh, you may have missed my point that GCC -O0 and -O2 are not >> consistent on whether a `const int` operand can be used in a constant >> expression. > >Yes and that was fixed for GCC 8 by r8-4755. >Since you were not specific on which version of GCC, I am assuming it >was GCC7.x. > >Thanks, >Andrew Pinski Sorry for the imprecise description. I think the issue is different from your mentioned GCC 8 bugfix. I commented out these lines in include/libc-symbols.h -#if !defined __ASSEMBLER__ && !defined __OPTIMIZE__ -# error "glibc cannot be compiled without optimization" -#endif +//#if !defined __ASSEMBLER__ && !defined __OPTIMIZE__ +//# error "glibc cannot be compiled without optimization" +//#endif Next, a=(../sysdeps/unix/sysv/linux/dl-opendir.c -std=gnu11 -fgnu89-inline -g -Wall -Wwrite-strings -Wundef -fmerge-all-constants -frounding-math -fno-stack-protector -fno-common -Wstrict-prototypes -Wold-style-definition -fmath-errno -fPIC -fno-stack-protector -DSTACK_PROTECTOR_LEVEL=0 -mno-mmx -ftls-model=initial-exec -I../include -I$HOME/Dev/glibc/out/clang/elf -I$HOME/Dev/glibc/out/clang -I../sysdeps/unix/sysv/linux/x86_64/64 -I../sysdeps/unix/sysv/linux/x86_64 -I../sysdeps/unix/sysv/linux/x86/include -I../sysdeps/unix/sysv/linux/x86 -I../sysdeps/x86/nptl -I../sysdeps/unix/sysv/linux/wordsize-64 -I../sysdeps/x86_64/nptl -I../sysdeps/unix/sysv/linux/include -I../sysdeps/unix/sysv/linux -I../sysdeps/nptl -I../sysdeps/pthread -I../sysdeps/gnu -I../sysdeps/unix/inet -I../sysdeps/unix/sysv -I../sysdeps/unix/x86_64 -I../sysdeps/unix -I../sysdeps/posix -I../sysdeps/x86_64/64 -I../sysdeps/x86_64/fpu/multiarch -I../sysdeps/x86_64/fpu -I../sysdeps/x86/fpu -I../sysdeps/x86_64/multiarch -I../sysdeps/x86_64 -I../sysdeps/x86/include -I../sysdeps/x86 -I../sysdeps/ieee754/float128 -I../sysdeps/ieee754/ldbl-96/include -I../sysdeps/ieee754/ldbl-96 -I../sysdeps/ieee754/dbl-64 -I../sysdeps/ieee754/flt-32 -I../sysdeps/wordsize-64 -I../sysdeps/ieee754 -I../sysdeps/generic -I.. -I../libio -I. -D_LIBC_REENTRANT -include $HOME/Dev/glibc/out/clang/libc-modules.h -include ../include/libc-symbols.h -DPIC -DSHARED -DTOP_NAMESPACE=glibc -fsyntax-only) % gcc-11 $=a -O2 # good % gcc-11 $=a -O0 # -O1 is similar In file included from ../include/features.h:488, from ../posix/sys/types.h:25, from ../include/sys/types.h:1, from ../sysdeps/unix/sysv/linux/dirstream.h:21, from ../include/dirent.h:3, from ../sysdeps/unix/sysv/linux/opendir.c:18, from ../sysdeps/unix/sysv/linux/dl-opendir.c:1: ../sysdeps/unix/sysv/linux/opendir.c: In function ‘__alloc_dir’: ../sysdeps/unix/sysv/linux/opendir.c:107:35: error: expression in static assertion is not constant 107 | _Static_assert (allocation_size >= sizeof (struct dirent64), | ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~ ../include/sys/cdefs.h:7:59: note: in definition of macro ‘_Static_assert’ 7 | # define _Static_assert(expr, diagnostic) _Static_assert (expr, diagnostic) | ^~~~ Then I tested gcc-8, gcc-9, gcc-10 on Debian testing, they have the same behavior as gcc-11. >> >> Updated description: >> >> --- >> >> linux: Fix a non-constant expression in _Static_assert >> >> According to C11 6.6p6, `const int` as an operand may not make up a >> constant expression. GCC's -O0 and -O2 optimization levels are not >> consistent on whether this operand can be used in a constant expression: >> >> ../sysdeps/unix/sysv/linux/opendir.c:107:19: error: static_assert >> expression is not an integral constant expression >> _Static_assert (allocation_size >= sizeof (struct dirent64), >> >> Use enum which is guaranteed to be a constant expression. >> This fixes an error when compiling with GCC -O0 and Clang. >> >> Fixes: 4b962c9e859de23b461d61f860dbd3f21311e83a ("linux: Simplify >> opendir buffer allocation")
On Mon, Sep 27, 2021 at 11:20 AM Fāng-ruì Sòng <maskray@google.com> wrote: > > > On 2021-09-27, Andrew Pinski wrote: > >On Mon, Sep 27, 2021 at 10:18 AM Fāng-ruì Sòng via Libc-alpha > ><libc-alpha@sourceware.org> wrote: > >> > >> On Mon, Sep 27, 2021 at 6:09 AM Florian Weimer <fweimer@redhat.com> wrote: > >> > > >> > * Fangrui Song via Libc-alpha: > >> > > >> > > The description needs some adjustment > >> > > https://www.iso-9899.info/n1570.html#6.6p10 > >> > > > >> > > 10 An implementation may accept other forms of constant expressions. > >> > > >> > Yeah, but the change itself is clearly correct. Thanks. > >> > > >> > Florian > >> > > >> > >> Ugh, you may have missed my point that GCC -O0 and -O2 are not > >> consistent on whether a `const int` operand can be used in a constant > >> expression. > > > >Yes and that was fixed for GCC 8 by r8-4755. > >Since you were not specific on which version of GCC, I am assuming it > >was GCC7.x. > > > >Thanks, > >Andrew Pinski > > Sorry for the imprecise description. > I think the issue is different from your mentioned GCC 8 bugfix. > > I commented out these lines in include/libc-symbols.h > > -#if !defined __ASSEMBLER__ && !defined __OPTIMIZE__ > -# error "glibc cannot be compiled without optimization" > -#endif > +//#if !defined __ASSEMBLER__ && !defined __OPTIMIZE__ > +//# error "glibc cannot be compiled without optimization" > +//#endif > > Next, > > a=(../sysdeps/unix/sysv/linux/dl-opendir.c -std=gnu11 -fgnu89-inline -g -Wall -Wwrite-strings -Wundef -fmerge-all-constants -frounding-math -fno-stack-protector -fno-common -Wstrict-prototypes -Wold-style-definition -fmath-errno -fPIC -fno-stack-protector -DSTACK_PROTECTOR_LEVEL=0 -mno-mmx -ftls-model=initial-exec -I../include -I$HOME/Dev/glibc/out/clang/elf -I$HOME/Dev/glibc/out/clang -I../sysdeps/unix/sysv/linux/x86_64/64 -I../sysdeps/unix/sysv/linux/x86_64 -I../sysdeps/unix/sysv/linux/x86/include -I../sysdeps/unix/sysv/linux/x86 -I../sysdeps/x86/nptl -I../sysdeps/unix/sysv/linux/wordsize-64 -I../sysdeps/x86_64/nptl -I../sysdeps/unix/sysv/linux/include -I../sysdeps/unix/sysv/linux -I../sysdeps/nptl -I../sysdeps/pthread -I../sysdeps/gnu -I../sysdeps/unix/inet -I../sysdeps/unix/sysv -I../sysdeps/unix/x86_64 -I../sysdeps/unix -I../sysdeps/posix -I../sysdeps/x86_64/64 -I../sysdeps/x86_64/fpu/multiarch -I../sysdeps/x86_64/fpu -I../sysdeps/x86/fpu -I../sysdeps/x86_64/multiarch -I../sysdeps/x86_64 -I../sysdeps/x86/include -I../sysdeps/x86 -I../sysdeps/ieee754/float128 -I../sysdeps/ieee754/ldbl-96/include -I../sysdeps/ieee754/ldbl-96 -I../sysdeps/ieee754/dbl-64 -I../sysdeps/ieee754/flt-32 -I../sysdeps/wordsize-64 -I../sysdeps/ieee754 -I../sysdeps/generic -I.. -I../libio -I. -D_LIBC_REENTRANT -include $HOME/Dev/glibc/out/clang/libc-modules.h -include ../include/libc-symbols.h -DPIC -DSHARED -DTOP_NAMESPACE=glibc -fsyntax-only) > > % gcc-11 $=a -O2 # good > % gcc-11 $=a -O0 # -O1 is similar > In file included from ../include/features.h:488, > from ../posix/sys/types.h:25, > from ../include/sys/types.h:1, > from ../sysdeps/unix/sysv/linux/dirstream.h:21, > from ../include/dirent.h:3, > from ../sysdeps/unix/sysv/linux/opendir.c:18, > from ../sysdeps/unix/sysv/linux/dl-opendir.c:1: > ../sysdeps/unix/sysv/linux/opendir.c: In function ‘__alloc_dir’: > ../sysdeps/unix/sysv/linux/opendir.c:107:35: error: expression in static assertion is not constant > 107 | _Static_assert (allocation_size >= sizeof (struct dirent64), > | ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~ > ../include/sys/cdefs.h:7:59: note: in definition of macro ‘_Static_assert’ > 7 | # define _Static_assert(expr, diagnostic) _Static_assert (expr, diagnostic) > | ^~~~ > > Then I tested gcc-8, gcc-9, gcc-10 on Debian testing, they have the same behavior as gcc-11. Please file a bug. > > >> > >> Updated description: > >> > >> --- > >> > >> linux: Fix a non-constant expression in _Static_assert > >> > >> According to C11 6.6p6, `const int` as an operand may not make up a > >> constant expression. GCC's -O0 and -O2 optimization levels are not > >> consistent on whether this operand can be used in a constant expression: > >> > >> ../sysdeps/unix/sysv/linux/opendir.c:107:19: error: static_assert > >> expression is not an integral constant expression > >> _Static_assert (allocation_size >= sizeof (struct dirent64), > >> > >> Use enum which is guaranteed to be a constant expression. > >> This fixes an error when compiling with GCC -O0 and Clang. > >> > >> Fixes: 4b962c9e859de23b461d61f860dbd3f21311e83a ("linux: Simplify > >> opendir buffer allocation")
On Mon, Sep 27, 2021 at 11:21 AM Andrew Pinski <pinskia@gmail.com> wrote: > > On Mon, Sep 27, 2021 at 11:20 AM Fāng-ruì Sòng <maskray@google.com> wrote: > > > > > > On 2021-09-27, Andrew Pinski wrote: > > >On Mon, Sep 27, 2021 at 10:18 AM Fāng-ruì Sòng via Libc-alpha > > ><libc-alpha@sourceware.org> wrote: > > >> > > >> On Mon, Sep 27, 2021 at 6:09 AM Florian Weimer <fweimer@redhat.com> wrote: > > >> > > > >> > * Fangrui Song via Libc-alpha: > > >> > > > >> > > The description needs some adjustment > > >> > > https://www.iso-9899.info/n1570.html#6.6p10 > > >> > > > > >> > > 10 An implementation may accept other forms of constant expressions. > > >> > > > >> > Yeah, but the change itself is clearly correct. Thanks. > > >> > > > >> > Florian > > >> > > > >> > > >> Ugh, you may have missed my point that GCC -O0 and -O2 are not > > >> consistent on whether a `const int` operand can be used in a constant > > >> expression. > > > > > >Yes and that was fixed for GCC 8 by r8-4755. > > >Since you were not specific on which version of GCC, I am assuming it > > >was GCC7.x. > > > > > >Thanks, > > >Andrew Pinski > > > > Sorry for the imprecise description. > > I think the issue is different from your mentioned GCC 8 bugfix. > > > > I commented out these lines in include/libc-symbols.h > > > > -#if !defined __ASSEMBLER__ && !defined __OPTIMIZE__ > > -# error "glibc cannot be compiled without optimization" > > -#endif > > +//#if !defined __ASSEMBLER__ && !defined __OPTIMIZE__ > > +//# error "glibc cannot be compiled without optimization" > > +//#endif > > > > Next, > > > > a=(../sysdeps/unix/sysv/linux/dl-opendir.c -std=gnu11 -fgnu89-inline -g -Wall -Wwrite-strings -Wundef -fmerge-all-constants -frounding-math -fno-stack-protector -fno-common -Wstrict-prototypes -Wold-style-definition -fmath-errno -fPIC -fno-stack-protector -DSTACK_PROTECTOR_LEVEL=0 -mno-mmx -ftls-model=initial-exec -I../include -I$HOME/Dev/glibc/out/clang/elf -I$HOME/Dev/glibc/out/clang -I../sysdeps/unix/sysv/linux/x86_64/64 -I../sysdeps/unix/sysv/linux/x86_64 -I../sysdeps/unix/sysv/linux/x86/include -I../sysdeps/unix/sysv/linux/x86 -I../sysdeps/x86/nptl -I../sysdeps/unix/sysv/linux/wordsize-64 -I../sysdeps/x86_64/nptl -I../sysdeps/unix/sysv/linux/include -I../sysdeps/unix/sysv/linux -I../sysdeps/nptl -I../sysdeps/pthread -I../sysdeps/gnu -I../sysdeps/unix/inet -I../sysdeps/unix/sysv -I../sysdeps/unix/x86_64 -I../sysdeps/unix -I../sysdeps/posix -I../sysdeps/x86_64/64 -I../sysdeps/x86_64/fpu/multiarch -I../sysdeps/x86_64/fpu -I../sysdeps/x86/fpu -I../sysdeps/x86_64/multiarch -I../sysdeps/x86_64 -I../sysdeps/x86/include -I../sysdeps/x86 -I../sysdeps/ieee754/float128 -I../sysdeps/ieee754/ldbl-96/include -I../sysdeps/ieee754/ldbl-96 -I../sysdeps/ieee754/dbl-64 -I../sysdeps/ieee754/flt-32 -I../sysdeps/wordsize-64 -I../sysdeps/ieee754 -I../sysdeps/generic -I.. -I../libio -I. -D_LIBC_REENTRANT -include $HOME/Dev/glibc/out/clang/libc-modules.h -include ../include/libc-symbols.h -DPIC -DSHARED -DTOP_NAMESPACE=glibc -fsyntax-only) > > > > % gcc-11 $=a -O2 # good > > % gcc-11 $=a -O0 # -O1 is similar > > In file included from ../include/features.h:488, > > from ../posix/sys/types.h:25, > > from ../include/sys/types.h:1, > > from ../sysdeps/unix/sysv/linux/dirstream.h:21, > > from ../include/dirent.h:3, > > from ../sysdeps/unix/sysv/linux/opendir.c:18, > > from ../sysdeps/unix/sysv/linux/dl-opendir.c:1: > > ../sysdeps/unix/sysv/linux/opendir.c: In function ‘__alloc_dir’: > > ../sysdeps/unix/sysv/linux/opendir.c:107:35: error: expression in static assertion is not constant > > 107 | _Static_assert (allocation_size >= sizeof (struct dirent64), > > | ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~ > > ../include/sys/cdefs.h:7:59: note: in definition of macro ‘_Static_assert’ > > 7 | # define _Static_assert(expr, diagnostic) _Static_assert (expr, diagnostic) > > | ^~~~ > > > > Then I tested gcc-8, gcc-9, gcc-10 on Debian testing, they have the same behavior as gcc-11. > > Please file a bug. Filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102502
diff --git a/sysdeps/unix/sysv/linux/opendir.c b/sysdeps/unix/sysv/linux/opendir.c index 48f254d169..88640f44ee 100644 --- a/sysdeps/unix/sysv/linux/opendir.c +++ b/sysdeps/unix/sysv/linux/opendir.c @@ -103,7 +103,7 @@ __alloc_dir (int fd, bool close_fd, int flags, file system provides a bogus value. */ enum { max_buffer_size = 1048576 }; - const size_t allocation_size = 32768; + enum { allocation_size = 32768 }; _Static_assert (allocation_size >= sizeof (struct dirent64), "allocation_size < sizeof (struct dirent64)");