Adding Robbie and Vladimir...
On Thu, Mar 24, 2022 at 07:12:22PM -0500, Glenn Washburn wrote:
> Gnulib does not define abort(), but expects it to be defined, usually by a
> libc implementation. However, GRUB has no libc and does not have an abort()
> function. Until recently GRUB had patched out abort() in gnulib, effectively
> making it a noop. This changed with a recent update of the version of gnulib
> GRUB uses to be a compiler defined trap. While this is fine for user space
> code where the kernel can be expected to cleanup after a process when this
> happens, the firmware may not be good at doing this.
>
> GRUB does have a grub_abort(), with the same function signature that can be
> used instead. So instead define gnulib's abort() implementation to be
> grub_abort(). This provides consistency of behavior regardless of whether
> the abort happens in gnulib code or in GRUB code.
>
> Because we want to avoid patching gnulib again and gnulib expects that
> abort(), now grub_abort(), is defined externally, we must provide a
> prototype in the config.h. This is complicated by the fact that config.h
> is included in many GRUB compilation units as well as grub/misc.h which
> already declares grub_abort(). A macro, GNULIB, is provided only to
> compilation units using gnulib code, so that the grub_abort() prototype
> is only included in those compilation units where it is lacking.
>
> Also, export grub_abort() so that it can now be used within modules.
>
> Signed-off-by: Glenn Washburn <developm...@efficientek.com>
> ---
> conf/Makefile.common | 2 +-
> config.h.in | 17 ++++++++++-------
> grub-core/kern/misc.c | 2 +-
> include/grub/misc.h | 1 +
> 4 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/conf/Makefile.common b/conf/Makefile.common
> index b343a038ee..67996b710c 100644
> --- a/conf/Makefile.common
> +++ b/conf/Makefile.common
> @@ -65,7 +65,7 @@ grubconfdir = $(sysconfdir)/grub.d
> platformdir = $(pkglibdir)/$(target_cpu)-$(platform)
> starfielddir = $(pkgdatadir)/themes/starfield
>
> -CFLAGS_GNULIB = -Wno-undef -Wno-sign-compare -Wno-unused
> -Wno-unused-parameter -Wno-redundant-decls -Wno-unreachable-code
> -Wno-conversion
> +CFLAGS_GNULIB = -Wno-undef -Wno-sign-compare -Wno-unused
> -Wno-unused-parameter -Wno-redundant-decls -Wno-unreachable-code
> -Wno-conversion -DGNULIB=1
> CPPFLAGS_GNULIB = -I$(top_builddir)/grub-core/lib/gnulib
> -I$(top_srcdir)/grub-core/lib/gnulib
>
> CFLAGS_POSIX = -fno-builtin
> diff --git a/config.h.in b/config.h.in
> index 01dcbbfc82..5719b28ebe 100644
> --- a/config.h.in
> +++ b/config.h.in
> @@ -139,14 +139,17 @@ typedef __UINT_FAST32_TYPE__ uint_fast32_t;
> # define _GL_INLINE_HEADER_END _Pragma ("GCC diagnostic pop")
>
> /*
> - * We don't have an abort() for gnulib to call in regexp. Because gnulib is
> - * built as a separate object that is then linked with, it doesn't have any
> - * of our headers or functions around to use - so we unfortunately can't
> - * print anything. Builds of emu include the system's stdlib, which includes
> - * a prototype for abort(), so leave this as a macro that doesn't take
> - * arguments.
> + * We don't have an abort() for gnulib's functions to call (eg. in regexp),
> but
> + * one needs to be provided and grub_abort() is the equivalent. Gnulib does
> not
> + * provide a prototype for abort either, so one must be provided. However,
> this
> + * duplicates the prototype in grub/misc.h for GRUB compilation units, which
> + * causes a failure to compile. So only include the prototype if this is in a
> + * gnulib compilation unit.
> */
> -# define abort __builtin_trap
> +# ifdef GNULIB
> +# define abort grub_abort
> +void __attribute__ ((noreturn)) abort (void);
> +# endif
Please take a look at the commit db7337a3d (grub-core/lib/posix_wrap/stdlib.h
(abort): Removed). It looks we have more natural mechanism for this kind of
functions. Though I am not sure why it was removed. Vladimir?
> # endif /* !_GL_INLINE_HEADER_BEGIN */
>
> /* gnulib doesn't build cleanly with older compilers. */
> diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
> index 6c0221cc33..dfae4f9d78 100644
> --- a/grub-core/kern/misc.c
> +++ b/grub-core/kern/misc.c
> @@ -1249,7 +1249,7 @@ grub_printf_fmt_check (const char *fmt, const char
> *fmt_expected)
>
>
> /* Abort GRUB. This function does not return. */
> -static void __attribute__ ((noreturn))
> +void __attribute__ ((noreturn))
> grub_abort (void)
> {
> grub_printf ("\nAborted.");
> diff --git a/include/grub/misc.h b/include/grub/misc.h
> index 7d2b551969..776dbf8af0 100644
> --- a/include/grub/misc.h
> +++ b/include/grub/misc.h
> @@ -353,6 +353,7 @@ int EXPORT_FUNC(grub_vsnprintf) (char *str, grub_size_t
> n, const char *fmt,
> char *EXPORT_FUNC(grub_xasprintf) (const char *fmt, ...)
> __attribute__ ((format (GNU_PRINTF, 1, 2))) WARN_UNUSED_RESULT;
> char *EXPORT_FUNC(grub_xvasprintf) (const char *fmt, va_list args)
> WARN_UNUSED_RESULT;
> +void EXPORT_FUNC(grub_abort) (void) __attribute__ ((noreturn));
I am OK with these changes.
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
