[RFC,v1,2/2] Makeconfig: Compile glibc with -fno-delete-null-pointer-checks
Checks
Context |
Check |
Description |
redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
redhat-pt-bot/TryBot-32bit |
fail
|
Patch series failed to build
|
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
fail
|
Testing failed
|
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
fail
|
Testing failed
|
linaro-tcwg-bot/tcwg_glibc_build--master-arm |
fail
|
Testing failed
|
linaro-tcwg-bot/tcwg_glibc_check--master-arm |
fail
|
Testing failed
|
Commit Message
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(-)
Comments
* 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
@@ -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) \
@@ -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)