Message ID | 20230711131105.19203-3-alx@kernel.org |
---|---|
State | New |
Headers | show |
Series | Use -fno-delete-null-pointer-checks to build glibc | expand |
* Alejandro Colomar via Libc-alpha: > Don't undefine __nonnull to prevent compiler optimizations. Instead, > tell the compiler to not optimize based on the attribute, via the > appropriate flag. The manual for GCC 13 says: For function definitions: • If the compiler determines that a function parameter that is marked with nonnull is compared with null, and ‘-Wnonnull-compare’ option is enabled, a warning is issued. *Note Warning Options::. • The compiler may also perform optimizations based on the knowledge that ‘nonnull’ parameters cannot be null. This can currently not be disabled other than by removing the nonnull attribute. So this isn't actually equivalent today, and I think it impacts our ability to deal with null pointer errors in the implementation (in function definitions). Thanks, Florian
On 2023-07-11 09:41, Florian Weimer via Libc-alpha wrote: > * Alejandro Colomar via Libc-alpha: > >> Don't undefine __nonnull to prevent compiler optimizations. Instead, >> tell the compiler to not optimize based on the attribute, via the >> appropriate flag. > > The manual for GCC 13 says: > > For function definitions: > • If the compiler determines that a function parameter that is > marked with nonnull is compared with null, and > ‘-Wnonnull-compare’ option is enabled, a warning is issued. > *Note Warning Options::. > • The compiler may also perform optimizations based on the > knowledge that ‘nonnull’ parameters cannot be null. This can > currently not be disabled other than by removing the nonnull > attribute. > > So this isn't actually equivalent today, and I think it impacts our > ability to deal with null pointer errors in the implementation (in > function definitions). +1, this situation is equivalent to why we remove the access attribute during fortification too; it hinders the ability of the implementation to do those checks at runtime. Thanks, Sid
On Tue, 11 Jul 2023, Alejandro Colomar wrote: > Don't undefine __nonnull to prevent compiler optimizations. Instead, > tell the compiler to not optimize based on the attribute, via the > appropriate flag. I'll note that using -fno-delete-null-pointer-checks doesn't work (anymore?) to preserve the nullptr check in int __attribute__((nonnull)) foo (int *p) { if (p) return *p; return 0; } so the documentation of 'nonnull' which says @item The compiler may also perform optimizations based on the knowledge that certain function arguments cannot be null. These optimizations can be disabled by the @option{-fno-delete-null-pointer-checks} option. @xref{Optimize Options}. isn't accurate. I checked trunk, GCC 12 and GCC 7 here. For trunk the offending function is nonnull_arg_p where we probably want to preserve its behavior but some callers fail to check flag_delete_null_pointer_checks when they are not only issueing diagnostics. Richard. > Link: <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110617> > Reported-by: Florian Weimer <fweimer@redhat.com> > Cc: Xi Ruoyao <xry111@xry111.site> > Cc: Sam James <sam@gentoo.org> > Cc: Richard Biener <rguenth@gcc.gnu.org> > Cc: Andrew Pinski <apinski@marvell.com> > Signed-off-by: Alejandro Colomar <alx@kernel.org> > --- > Makeconfig | 1 + > include/sys/cdefs.h | 6 ------ > 2 files changed, 1 insertion(+), 6 deletions(-) > > diff --git a/Makeconfig b/Makeconfig > index 369a596e79..ecbe30e53f 100644 > --- a/Makeconfig > +++ b/Makeconfig > @@ -1045,6 +1045,7 @@ CPPFLAGS = $(config-extra-cppflags) $(CPPFLAGS-config) \ > > override CFLAGS = -std=gnu11 \ > -fgnu89-inline \ > + -fno-delete-null-pointer-checks \ > $(config-extra-cflags) \ > $(filter-out %frame-pointer,$(+cflags)) \ > $(+gccwarn-c) \ > diff --git a/include/sys/cdefs.h b/include/sys/cdefs.h > index b84ad34a70..e4a8dd7844 100644 > --- a/include/sys/cdefs.h > +++ b/include/sys/cdefs.h > @@ -10,12 +10,6 @@ > #include <misc/sys/cdefs.h> > > #ifndef _ISOMAC > -/* The compiler will optimize based on the knowledge the parameter is > - not NULL. This will omit tests. A robust implementation cannot allow > - this so when compiling glibc itself we ignore this attribute. */ > -# undef __nonnull > -# define __nonnull(params) > - > extern void __chk_fail (void) __attribute__ ((__noreturn__)); > libc_hidden_proto (__chk_fail) > rtld_hidden_proto (__chk_fail) >
On Wed, Jul 12, 2023 at 6:56 AM Richard Biener via Libc-alpha < libc-alpha@sourceware.org> wrote: > > > I checked trunk, GCC 12 and GCC 7 here. For trunk the offending > function is nonnull_arg_p where we probably want to preserve > its behavior but some callers fail to check > flag_delete_null_pointer_checks when they are not only issueing > diagnostics. > > int __attribute__((nonnull)) foo (int p) does not error out either at least in gcc-14.. if there is no pointer argument then the attribute is wrong ins't it ?
* Richard Biener: > On Tue, 11 Jul 2023, Alejandro Colomar wrote: > >> Don't undefine __nonnull to prevent compiler optimizations. Instead, >> tell the compiler to not optimize based on the attribute, via the >> appropriate flag. > > I'll note that using -fno-delete-null-pointer-checks doesn't work > (anymore?) to preserve the nullptr check in > > int __attribute__((nonnull)) foo (int *p) > { > if (p) > return *p; > return 0; > } > > so the documentation of 'nonnull' which says > > @item The compiler may also perform optimizations based on the > knowledge that certain function arguments cannot be null. These > optimizations can be disabled by the > @option{-fno-delete-null-pointer-checks} option. @xref{Optimize Options}. > > isn't accurate. That's the paragraph about function calls, not function definitions. The above is a function definition. So it's not a counterexample. The second paragraph says that the optimization behavior you see is currently not controllable by an option. Thanks, Florian
diff --git a/Makeconfig b/Makeconfig index 369a596e79..ecbe30e53f 100644 --- a/Makeconfig +++ b/Makeconfig @@ -1045,6 +1045,7 @@ CPPFLAGS = $(config-extra-cppflags) $(CPPFLAGS-config) \ override CFLAGS = -std=gnu11 \ -fgnu89-inline \ + -fno-delete-null-pointer-checks \ $(config-extra-cflags) \ $(filter-out %frame-pointer,$(+cflags)) \ $(+gccwarn-c) \ diff --git a/include/sys/cdefs.h b/include/sys/cdefs.h index b84ad34a70..e4a8dd7844 100644 --- a/include/sys/cdefs.h +++ b/include/sys/cdefs.h @@ -10,12 +10,6 @@ #include <misc/sys/cdefs.h> #ifndef _ISOMAC -/* The compiler will optimize based on the knowledge the parameter is - not NULL. This will omit tests. A robust implementation cannot allow - this so when compiling glibc itself we ignore this attribute. */ -# undef __nonnull -# define __nonnull(params) - extern void __chk_fail (void) __attribute__ ((__noreturn__)); libc_hidden_proto (__chk_fail) rtld_hidden_proto (__chk_fail)
Don't undefine __nonnull to prevent compiler optimizations. Instead, tell the compiler to not optimize based on the attribute, via the appropriate flag. Link: <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110617> Reported-by: Florian Weimer <fweimer@redhat.com> Cc: Xi Ruoyao <xry111@xry111.site> Cc: Sam James <sam@gentoo.org> Cc: Richard Biener <rguenth@gcc.gnu.org> Cc: Andrew Pinski <apinski@marvell.com> Signed-off-by: Alejandro Colomar <alx@kernel.org> --- Makeconfig | 1 + include/sys/cdefs.h | 6 ------ 2 files changed, 1 insertion(+), 6 deletions(-)