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

[Daniel Kiper]

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...

Daniel