[RFC PATCH] configure: Enable -fno-omit-frame-pointer for backtrace module

[RFC PATCH] configure: Enable -fno-omit-frame-pointer for backtrace module

[Glenn Washburn]

The backtrace module is written assuming that the frame pointer is in %ebp.
By default, -Os optimization level is used, which enables the gcc option
-fomit-frame-pointer. This break the backtrace functionality. Enabling
this may cause an unnoticeable performance cost and virtually no size
increase.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
The backtrace command on x86_64 and probably i386 is broken due to the
above rationale. I've not verified, but presumably the backtrace that used
to be printed for an unhandled CPU exception is also broken. Do any distros
handle this?

Considering that (to my knowledge) no one has complained about this in the
over 13 years that -Os has been used, has this code actually been useful?
Is it worth disabling -fomit-frame-pointer? Though, I don't see much downside
right now in disabling it. Alternatively, we could disable/remove the
backtrace code. I think it would be nice to keep it and have it working.

Nowadays, presumably QEMU makes the GDB stub rarely used as I imagine most
are developing in a virual machine. Also, the GDB stub does not work in UEFI
so if anyone is using it on real hardware, they are doing so on pretty old
machines. The lack of a GDB stub does not seem to be a pain point because
no one has got it working on UEFI.

This patch gets the backtrace command working on x86_64-efi in QEMU for me.
However, it hangs when run on my laptop. Not sure what's going on there.

Also, I've noticed that recents builds have caused a big slow down in the
already slow LUKS unlock code on real hardware, but not noticeable in QEMU.
It seems to occur with and without this patch. I mention this if others
would be interested in testing this.

Glenn
---
 configure.ac | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/configure.ac b/configure.ac
index 278e5a81f805..545cf69c272a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1020,6 +1020,19 @@ if test x"$target_cpu" = xsparc64 ; then
   TARGET_LDFLAGS="$TARGET_LDFLAGS $grub_cv_target_cc_mno_relax"
 fi
 
+# The backtrace module relies on frame pointers and the default optimization
+# level, -Os, omits them. Make sure they are enabled.
+AC_CACHE_CHECK([whether -fno-omit-frame-pointer works], [grub_cv_cc_fno_omit_frame_pointer], [
+  CFLAGS="$TARGET_CFLAGS -fno-omit-frame-pointer"
+  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[]])],
+      [grub_cv_cc_fno_omit_frame_pointer=yes],
+      [grub_cv_cc_fno_omit_frame_pointer=no])
+])
+
+if test "x$grub_cv_cc_fno_omit_frame_pointer" = xyes; then
+  TARGET_CFLAGS="$TARGET_CFLAGS -fno-omit-frame-pointer"
+fi
+
 # By default, GCC 4.4 generates .eh_frame sections containing unwind
 # information in some cases where it previously did not. GRUB doesn't need
 # these and they just use up vital space. Restore the old compiler
-- 
2.34.1


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [RFC PATCH] configure: Enable -fno-omit-frame-pointer for backtrace module

[Vladimir 'phcoder' Serbinenko]

[-- Attachment #1.1: Type: text/plain, Size: 3178 bytes --]

Le sam. 12 août 2023, 00:22, Glenn Washburn <development@efficientek.com> a
écrit :

> The backtrace module is written assuming that the frame pointer is in %ebp.
> By default, -Os optimization level is used, which enables the gcc option
> -fomit-frame-pointer. This break the backtrace functionality. Enabling
> this may cause an unnoticeable performance cost and virtually no size
> increase.
>
Do you have numbers on core.img size on i386-pc?

>
> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
> The backtrace command on x86_64 and probably i386 is broken due to the
> above rationale. I've not verified, but presumably the backtrace that used
> to be printed for an unhandled CPU exception is also broken. Do any distros
> handle this?
>
> Considering that (to my knowledge) no one has complained about this in the
> over 13 years that -Os has been used, has this code actually been useful?
> Is it worth disabling -fomit-frame-pointer? Though, I don't see much
> downside
> right now in disabling it. Alternatively, we could disable/remove the
> backtrace code. I think it would be nice to keep it and have it working.
>
> Nowadays, presumably QEMU makes the GDB stub rarely used as I imagine most
> are developing in a virual machine. Also, the GDB stub does not work in
> UEFI
> so if anyone is using it on real hardware, they are doing so on pretty old
> machines. The lack of a GDB stub does not seem to be a pain point because
> no one has got it working on UEFI.
>
> This patch gets the backtrace command working on x86_64-efi in QEMU for me.
> However, it hangs when run on my laptop. Not sure what's going on there.
>
> Also, I've noticed that recents builds have caused a big slow down in the
> already slow LUKS unlock code on real hardware, but not noticeable in QEMU.
> It seems to occur with and without this patch. I mention this if others
> would be interested in testing this.
>
> Glenn
> ---
>  configure.ac | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/configure.ac b/configure.ac
> index 278e5a81f805..545cf69c272a 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1020,6 +1020,19 @@ if test x"$target_cpu" = xsparc64 ; then
>    TARGET_LDFLAGS="$TARGET_LDFLAGS $grub_cv_target_cc_mno_relax"
>  fi
>
> +# The backtrace module relies on frame pointers and the default
> optimization
> +# level, -Os, omits them. Make sure they are enabled.
> +AC_CACHE_CHECK([whether -fno-omit-frame-pointer works],
> [grub_cv_cc_fno_omit_frame_pointer], [
> +  CFLAGS="$TARGET_CFLAGS -fno-omit-frame-pointer"
> +  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[]])],
> +      [grub_cv_cc_fno_omit_frame_pointer=yes],
> +      [grub_cv_cc_fno_omit_frame_pointer=no])
> +])
> +
> +if test "x$grub_cv_cc_fno_omit_frame_pointer" = xyes; then
> +  TARGET_CFLAGS="$TARGET_CFLAGS -fno-omit-frame-pointer"
> +fi
> +
>  # By default, GCC 4.4 generates .eh_frame sections containing unwind
>  # information in some cases where it previously did not. GRUB doesn't need
>  # these and they just use up vital space. Restore the old compiler
> --
> 2.34.1
>
>

[-- Attachment #1.2: Type: text/html, Size: 4335 bytes --]

[-- Attachment #2: Type: text/plain, Size: 141 bytes --]

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [RFC PATCH] configure: Enable -fno-omit-frame-pointer for backtrace module

[Glenn Washburn]

On Sat, 12 Aug 2023 01:57:42 +0200
"Vladimir 'phcoder' Serbinenko" <phcoder@gmail.com> wrote:

> Le sam. 12 août 2023, 00:22, Glenn Washburn <development@efficientek.com> a
> écrit :
> 
> > The backtrace module is written assuming that the frame pointer is in %ebp.
> > By default, -Os optimization level is used, which enables the gcc option
> > -fomit-frame-pointer. This break the backtrace functionality. Enabling
> > this may cause an unnoticeable performance cost and virtually no size
> > increase.
> >
> Do you have numbers on core.img size on i386-pc?

No, I don't because I'm not really using i386-pc. However, do these numbers work
for you instead? Or are you wanting to see if the core.img gets too large?

==== With frame pointer
$ du -bc grub-i386-pc/lib/grub/i386-pc/*.mod | tail -n 1
1932852 total
$ du -bc grub-i386-pc/lib/grub/i386-pc/kernel.img
34672   grub-i386-pc/lib/grub/i386-pc/kernel.img
43152   total

==== No frame pointer
$ du -bc grub-i386-pc/lib/grub/i386-pc/*.mod | tail -n 1
1923236 total
$ du -bc grub-i386-pc/lib/grub/i386-pc/kernel.img
34512   grub-i386-pc/lib/grub/i386-pc/kernel.img

Glenn

> >
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > ---
> > The backtrace command on x86_64 and probably i386 is broken due to the
> > above rationale. I've not verified, but presumably the backtrace that used
> > to be printed for an unhandled CPU exception is also broken. Do any distros
> > handle this?
> >
> > Considering that (to my knowledge) no one has complained about this in the
> > over 13 years that -Os has been used, has this code actually been useful?
> > Is it worth disabling -fomit-frame-pointer? Though, I don't see much
> > downside
> > right now in disabling it. Alternatively, we could disable/remove the
> > backtrace code. I think it would be nice to keep it and have it working.
> >
> > Nowadays, presumably QEMU makes the GDB stub rarely used as I imagine most
> > are developing in a virual machine. Also, the GDB stub does not work in
> > UEFI
> > so if anyone is using it on real hardware, they are doing so on pretty old
> > machines. The lack of a GDB stub does not seem to be a pain point because
> > no one has got it working on UEFI.
> >
> > This patch gets the backtrace command working on x86_64-efi in QEMU for me.
> > However, it hangs when run on my laptop. Not sure what's going on there.
> >
> > Also, I've noticed that recents builds have caused a big slow down in the
> > already slow LUKS unlock code on real hardware, but not noticeable in QEMU.
> > It seems to occur with and without this patch. I mention this if others
> > would be interested in testing this.
> >
> > Glenn
> > ---
> >  configure.ac | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 278e5a81f805..545cf69c272a 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -1020,6 +1020,19 @@ if test x"$target_cpu" = xsparc64 ; then
> >    TARGET_LDFLAGS="$TARGET_LDFLAGS $grub_cv_target_cc_mno_relax"
> >  fi
> >
> > +# The backtrace module relies on frame pointers and the default
> > optimization
> > +# level, -Os, omits them. Make sure they are enabled.
> > +AC_CACHE_CHECK([whether -fno-omit-frame-pointer works],
> > [grub_cv_cc_fno_omit_frame_pointer], [
> > +  CFLAGS="$TARGET_CFLAGS -fno-omit-frame-pointer"
> > +  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[]])],
> > +      [grub_cv_cc_fno_omit_frame_pointer=yes],
> > +      [grub_cv_cc_fno_omit_frame_pointer=no])
> > +])
> > +
> > +if test "x$grub_cv_cc_fno_omit_frame_pointer" = xyes; then
> > +  TARGET_CFLAGS="$TARGET_CFLAGS -fno-omit-frame-pointer"
> > +fi
> > +
> >  # By default, GCC 4.4 generates .eh_frame sections containing unwind
> >  # information in some cases where it previously did not. GRUB doesn't need
> >  # these and they just use up vital space. Restore the old compiler
> > --
> > 2.34.1
> >
> >

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [RFC PATCH] configure: Enable -fno-omit-frame-pointer for backtrace module

[Daniel Kiper]

On Mon, Aug 14, 2023 at 12:47:33AM -0500, Glenn Washburn wrote:
> On Sat, 12 Aug 2023 01:57:42 +0200
> "Vladimir 'phcoder' Serbinenko" <phcoder@gmail.com> wrote:
>
> > Le sam. 12 août 2023, 00:22, Glenn Washburn <development@efficientek.com> a
> > écrit :
> >
> > > The backtrace module is written assuming that the frame pointer is in %ebp.
> > > By default, -Os optimization level is used, which enables the gcc option
> > > -fomit-frame-pointer. This break the backtrace functionality. Enabling
> > > this may cause an unnoticeable performance cost and virtually no size
> > > increase.
> > >
> > Do you have numbers on core.img size on i386-pc?
>
> No, I don't because I'm not really using i386-pc. However, do these numbers work
> for you instead? Or are you wanting to see if the core.img gets too large?
>
> ==== With frame pointer
> $ du -bc grub-i386-pc/lib/grub/i386-pc/*.mod | tail -n 1
> 1932852 total
> $ du -bc grub-i386-pc/lib/grub/i386-pc/kernel.img
> 34672   grub-i386-pc/lib/grub/i386-pc/kernel.img
> 43152   total
>
> ==== No frame pointer
> $ du -bc grub-i386-pc/lib/grub/i386-pc/*.mod | tail -n 1
> 1923236 total
> $ du -bc grub-i386-pc/lib/grub/i386-pc/kernel.img
> 34512   grub-i386-pc/lib/grub/i386-pc/kernel.img

I would not bother much here because we do not support small MBR gaps for
quite long time. So, Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>...

Though...

> Glenn
>
> > >
> > > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > > ---
> > > The backtrace command on x86_64 and probably i386 is broken due to the
> > > above rationale. I've not verified, but presumably the backtrace that used
> > > to be printed for an unhandled CPU exception is also broken. Do any distros
> > > handle this?
> > >
> > > Considering that (to my knowledge) no one has complained about this in the
> > > over 13 years that -Os has been used, has this code actually been useful?
> > > Is it worth disabling -fomit-frame-pointer? Though, I don't see much
> > > downside
> > > right now in disabling it. Alternatively, we could disable/remove the
> > > backtrace code. I think it would be nice to keep it and have it working.
> > >
> > > Nowadays, presumably QEMU makes the GDB stub rarely used as I imagine most
> > > are developing in a virual machine. Also, the GDB stub does not work in
> > > UEFI
> > > so if anyone is using it on real hardware, they are doing so on pretty old
> > > machines. The lack of a GDB stub does not seem to be a pain point because
> > > no one has got it working on UEFI.
> > >
> > > This patch gets the backtrace command working on x86_64-efi in QEMU for me.
> > > However, it hangs when run on my laptop. Not sure what's going on there.

... I would move everything up until here to the commit message. If you are
OK with it I can do it for you.

> > > Also, I've noticed that recents builds have caused a big slow down in the
> > > already slow LUKS unlock code on real hardware, but not noticeable in QEMU.
> > > It seems to occur with and without this patch. I mention this if others
> > > would be interested in testing this.

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [RFC PATCH] configure: Enable -fno-omit-frame-pointer for backtrace module

[Glenn Washburn]

On Thu, 14 Sep 2023 17:04:08 +0200
Daniel Kiper <dkiper@net-space.pl> wrote:

> On Mon, Aug 14, 2023 at 12:47:33AM -0500, Glenn Washburn wrote:
> > On Sat, 12 Aug 2023 01:57:42 +0200
> > "Vladimir 'phcoder' Serbinenko" <phcoder@gmail.com> wrote:
> >
> > > Le sam. 12 août 2023, 00:22, Glenn Washburn <development@efficientek.com> a
> > > écrit :
> > >
> > > > The backtrace module is written assuming that the frame pointer is in %ebp.
> > > > By default, -Os optimization level is used, which enables the gcc option
> > > > -fomit-frame-pointer. This break the backtrace functionality. Enabling
> > > > this may cause an unnoticeable performance cost and virtually no size
> > > > increase.
> > > >
> > > Do you have numbers on core.img size on i386-pc?
> >
> > No, I don't because I'm not really using i386-pc. However, do these numbers work
> > for you instead? Or are you wanting to see if the core.img gets too large?
> >
> > ==== With frame pointer
> > $ du -bc grub-i386-pc/lib/grub/i386-pc/*.mod | tail -n 1
> > 1932852 total
> > $ du -bc grub-i386-pc/lib/grub/i386-pc/kernel.img
> > 34672   grub-i386-pc/lib/grub/i386-pc/kernel.img
> > 43152   total
> >
> > ==== No frame pointer
> > $ du -bc grub-i386-pc/lib/grub/i386-pc/*.mod | tail -n 1
> > 1923236 total
> > $ du -bc grub-i386-pc/lib/grub/i386-pc/kernel.img
> > 34512   grub-i386-pc/lib/grub/i386-pc/kernel.img
> 
> I would not bother much here because we do not support small MBR gaps for
> quite long time. So, Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>...
> 
> Though...
> 
> > Glenn
> >
> > > >
> > > > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > > > ---
> > > > The backtrace command on x86_64 and probably i386 is broken due to the
> > > > above rationale. I've not verified, but presumably the backtrace that used
> > > > to be printed for an unhandled CPU exception is also broken. Do any distros
> > > > handle this?
> > > >
> > > > Considering that (to my knowledge) no one has complained about this in the
> > > > over 13 years that -Os has been used, has this code actually been useful?
> > > > Is it worth disabling -fomit-frame-pointer? Though, I don't see much
> > > > downside
> > > > right now in disabling it. Alternatively, we could disable/remove the
> > > > backtrace code. I think it would be nice to keep it and have it working.
> > > >
> > > > Nowadays, presumably QEMU makes the GDB stub rarely used as I imagine most
> > > > are developing in a virual machine. Also, the GDB stub does not work in
> > > > UEFI
> > > > so if anyone is using it on real hardware, they are doing so on pretty old
> > > > machines. The lack of a GDB stub does not seem to be a pain point because
> > > > no one has got it working on UEFI.
> > > >
> > > > This patch gets the backtrace command working on x86_64-efi in QEMU for me.
> > > > However, it hangs when run on my laptop. Not sure what's going on there.
> 
> ... I would move everything up until here to the commit message. If you are
> OK with it I can do it for you.

I'll let you take care of it, thanks.

Glenn

> 
> > > > Also, I've noticed that recents builds have caused a big slow down in the
> > > > already slow LUKS unlock code on real hardware, but not noticeable in QEMU.
> > > > It seems to occur with and without this patch. I mention this if others
> > > > would be interested in testing this.
> 
> Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel