Re: [PATCH] gdb: Add gdbinfo command for printing the load address of the EFI application

[Daniel Kiper]

On Tue, May 30, 2023 at 04:29:08PM +0200, Daniel Kiper wrote:
> On Sat, May 13, 2023 at 09:25:38PM -0500, Glenn Washburn wrote:
> > EFI firmware determines where to load the GRUB EFI at runtime, and so the
> > addresses of debug symbols are not known ahead of time. There is a command
> > defined in the gdb_grub script which will load the debug symbols at the
> > appropriate addresses, if given given the application load address for GRUB.
>
> s/given given/given/
>
> > So add a command named "gdbinfo" to allow the user to print this GDB command
> > string with the application load address on-demand. For the outputted GDB
> > command to have any effect when entered into a GDB session, GDB should have
> > been started with the script as an argument to the -x option or sourced into
> > an active GDB session before running the outputted command.
> >
> > Documentation for the gdbinfo command is also added.
> >
> > Co-developed-by: Peter Jones <pjones@redhat.com>
> > Signed-off-by: Peter Jones <pjones@redhat.com>
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > ---
> >  docs/grub.texi              | 13 ++++++++++++
> >  grub-core/Makefile.core.def |  1 +
> >  grub-core/kern/efi/debug.c  | 38 ++++++++++++++++++++++++++++++++++
> >  grub-core/kern/efi/efi.c    |  4 ++--
> >  grub-core/kern/efi/init.c   |  5 ++++-
> >  include/grub/efi/debug.h    | 41 +++++++++++++++++++++++++++++++++++++
> >  include/grub/efi/efi.h      |  2 +-
> >  7 files changed, 100 insertions(+), 4 deletions(-)
> >  create mode 100644 grub-core/kern/efi/debug.c
> >  create mode 100644 include/grub/efi/debug.h
> >
> > diff --git a/docs/grub.texi b/docs/grub.texi
> > index 9d4adf0b8ab6..8669f76d176e 100644
> > --- a/docs/grub.texi
> > +++ b/docs/grub.texi
> > @@ -4319,6 +4319,7 @@ you forget a command, you can run the command 
> > @command{help}
> >  * export::                      Export an environment variable
> >  * false::                       Do nothing, unsuccessfully
> >  * fwsetup::                     Reboot into the firmware setup menu
> > +* gdbinfo::                     Provide info for debugging with GDB
> >  * gettext::                     Translate a string
> >  * gptsync::                     Fill an MBR based on GPT entries
> >  * halt::                        Shut down your computer
> > @@ -4851,6 +4852,18 @@ exit successfully if so.
> >  @end deffn
> >
> >
> > +@node gdbinfo
> > +@subsection gdbinfo
> > +
> > +@deffn Command gdbinfo
> > +Output text to be used as a GDB command for a GDB session using the 
> > gdb_grub
> > +script and attached to a running GRUB instance. The GDB command that is
> > +output will tell GDB how to load debugging symbols to their proper runtime
> > +address. Currently this is only available for EFI platforms. See the 
> > Debugging
> > +in the developer documentation for more information.
> > +@end deffn
> > +
> > +
> >  @node gettext
> >  @subsection gettext
> >
> > diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> > index fb4cc066c526..3344f15bd5d1 100644
> > --- a/grub-core/Makefile.core.def
> > +++ b/grub-core/Makefile.core.def
> > @@ -206,6 +206,7 @@ kernel = {
> >
> >    efi = disk/efi/efidisk.c;
> >    efi = kern/efi/efi.c;
> > +  efi = kern/efi/debug.c;
> >    efi = kern/efi/init.c;
> >    efi = kern/efi/mm.c;
> >    efi = term/efi/console.c;
> > diff --git a/grub-core/kern/efi/debug.c b/grub-core/kern/efi/debug.c
> > new file mode 100644
> > index 000000000000..506ad90dca06
> > --- /dev/null
> > +++ b/grub-core/kern/efi/debug.c
> > @@ -0,0 +1,38 @@
> > +/*
> > + *  GRUB  --  GRand Unified Bootloader
> > + *  Copyright (C) 2022  Free Software Foundation, Inc.
> > + *
> > + *  GRUB is free software: you can redistribute it and/or modify
> > + *  it under the terms of the GNU General Public License as published by
> > + *  the Free Software Foundation, either version 3 of the License, or
> > + *  (at your option) any later version.
> > + *
> > + *  GRUB is distributed in the hope that it will be useful,
> > + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + *  GNU General Public License for more details.
> > + *
> > + *  You should have received a copy of the GNU General Public License
> > + *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +/* debug.c - aides for debugging the EFI application */
> > +
> > +#include <grub/efi/debug.h>
> > +#include <grub/command.h>
> > +#include <grub/i18n.h>
> > +
> > +static grub_err_t
> > +grub_cmd_gdbinfo (struct grub_command *cmd __attribute__ ((unused)),
> > +             int argc __attribute__ ((unused)),
> > +             char **args __attribute__ ((unused)))
> > +{
> > +  grub_efi_print_gdb_info ();
> > +  return 0;
> > +}
> > +
> > +void
> > +grub_efi_register_debug_commands (void)
> > +{
> > +  grub_register_command_lockdown ("gdbinfo", grub_cmd_gdbinfo, 0,
> > +                             N_("Print infomation useful for GDB 
> > debugging"));
>
> I think we agreed after some discussion we can enable gdbinfo command
> even on UEFI platforms with Secure Boot enabled. So, I would do
> s/grub_register_command_lockdown/grub_register_command/
>
> Otherwise Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>...
>
> I can make above mentioned fixes for you before push...

Nah! This patch does not apply due to significant changes in the EFI
code. Could you rebase it on top of [1].

Daniel

[1] https://lists.gnu.org/archive/html/grub-devel/2023-05/msg00147.html