All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 00/15] GDB script fixes and improvements
@ 2022-12-16  5:29 Glenn Washburn
  2022-12-16  5:29 ` [PATCH v4 01/15] gdb: Fix redirection issue in dump_module_sections Glenn Washburn
                   ` (14 more replies)
  0 siblings, 15 replies; 31+ messages in thread
From: Glenn Washburn @ 2022-12-16  5:29 UTC (permalink / raw)
  To: grub-devel @ gnu . org, Daniel Kiper; +Cc: Glenn Washburn

v4 changes:
  * Fix issue where load_all_modules was only loading the last already loaded
    module.
  * Drastically simply runtime_load_module by using hardware break instead
    of software break. The previous work around didn't seem to be reliable on
    QEMU either.
  * Move early initialization symbol loading for i386-pc into its own patch
v3 changes:
  * Rebase on to latest master
  * Use correct automake macro for target cpu

There's been a lot of changes since v1. There are more fixes and more
features. The majority of the shell code has been moved to an external
file named gdb_helper.sh, instead of being inline in the GDB script. The
one (direct) PERL dependency in GRUB has been removed and converted to
shell script. Also a section on debugging is added to the developer docs.

Glenn

Glenn Washburn (15):
  gdb: Fix redirection issue in dump_module_sections
  gdb: Prevent wrapping when writing to .segments.tmp
  gdb: If no modules have been loaded, do not try to load module symbols
  gdb: Move runtime module loading into runtime_load_module
  gdb: Reliably load modules in runtime_load_module
  gdb: Add functions to make loading from dynamically positioned targets
    easier
  gdb: Remove Perl dependency for GRUB GDB script
  gdb: If enabled, print line used to load EFI kernel symbols when using
    gdb_grub script
  gdb: Conditionally run GDB script logic for dynamically or statically
    positioned GRUB
  gdb: Only connect to remote target once when first sourced
  gdb: Allow user defined "onload_<modname>" command to be run when
    module is loaded
  gdb: Allow running user-defined commands at GRUB start
  gdb: Add extra early initialization symbols for i386-pc
  gdb: Add ability to turn on shell tracing for gdb helper script
  docs: Add debugging chapter to development documentation

 config.h.in                 |   3 +
 docs/grub-dev.texi          | 191 ++++++++++++++++++++++++++++++++++++
 grub-core/Makefile.core.def |   4 +-
 grub-core/gdb_grub.in       | 162 +++++++++++++++++++++++++-----
 grub-core/gdb_helper.sh.in  | 108 ++++++++++++++++++++
 grub-core/gmodule.pl.in     |  30 ------
 grub-core/kern/efi/efi.c    |   4 +-
 grub-core/kern/efi/init.c   |  19 +++-
 include/grub/efi/efi.h      |   2 +-
 9 files changed, 465 insertions(+), 58 deletions(-)
 create mode 100644 grub-core/gdb_helper.sh.in
 delete mode 100644 grub-core/gmodule.pl.in

Range-diff against v3:
 1:  ec2b71c403 !  1:  9f273b8fa5 gdb: Fix redirection issue in dump_module_sections
    @@ Commit message
         which does the redirection and undoes the redirection when it finishes
         regardless of any errors in the command.
     
    +    Also, remove .segments.tmp file prior to loading modules in case one was
    +    left from a previous run.
    +
      ## grub-core/gdb_grub.in ##
     @@
      ###
    @@ grub-core/gdb_grub.in: define dump_module_sections
     -	set logging off
     -	# FIXME: restore logging status
     +define dump_module_sections
    -+	pipe dump_module_sections_helper $arg0 | sh -c 'cat >.segments.tmp'
    ++	pipe dump_module_sections_helper $arg0 | sh -c 'cat >>.segments.tmp'
      end
      document dump_module_sections
      	Gather information about module whose mod structure was
    +@@ grub-core/gdb_grub.in: document load_module
    + end
    + 
    + define load_all_modules
    ++	shell rm -f .segments.tmp
    + 	set $this = grub_dl_head
    + 	while ($this != 0)
    + 		dump_module_sections $this
 2:  f350ddf3c9 !  2:  85f68a8369 gdb: Prevent wrapping when writing to .segments.tmp
    @@ grub-core/gdb_grub.in: define dump_module_sections_helper
     +	# to .segments.tmp
     +	with width 0 -- \
     +	with trace-commands off -- \
    - 	pipe dump_module_sections_helper $arg0 | sh -c 'cat >.segments.tmp'
    + 	pipe dump_module_sections_helper $arg0 | sh -c 'cat >>.segments.tmp'
      end
      document dump_module_sections
 3:  75949e0d8e =  3:  88b3973cdb gdb: If no modules have been loaded, do not try to load module symbols
 4:  ed5599b842 =  4:  c0d7da87a8 gdb: Move runtime module loading into runtime_load_module
 5:  e00aa463bb <  -:  ---------- gdb: Get correct mod variable value
 6:  c1e0439012 <  -:  ---------- gdb: Do not run load_module if module has already been loaded
 -:  ---------- >  5:  4712465374 gdb: Reliably load modules in runtime_load_module
 7:  dc8ce82e27 =  6:  283021b7b9 gdb: Add functions to make loading from dynamically positioned targets easier
 8:  dc7338f00a =  7:  8f4b7c3bbd gdb: Remove Perl dependency for GRUB GDB script
 9:  0ee5cb7cc1 =  8:  055e968779 gdb: If enabled, print line used to load EFI kernel symbols when using gdb_grub script
10:  7ec11bff7e =  9:  64eccfc37e gdb: Conditionally run GDB script logic for dynamically or statically positioned GRUB
11:  e62defbaa7 = 10:  5064458dfd gdb: Only connect to remote target once when first sourced
12:  0101c41233 = 11:  c33e8f57b4 gdb: Allow user defined "onload_<modname>" command to be run when module is loaded
13:  94f4707b14 ! 12:  f8a26f3a56 gdb: Allow running user-defined commands at GRUB start
    @@ grub-core/gdb_grub.in: end
      	# We may have been very late to loading the kernel.exec symbols and
      	# and modules may already be loaded. So load symbols for any already
      	# loaded.
    -@@ grub-core/gdb_grub.in: document num_modules
    - 	Given a module name print its address or NULL if not loaded.
    +@@ grub-core/gdb_grub.in: document runtime_load_module
    + 	Load module symbols at runtime as they are loaded.
      end
      
     +define run_on_start
    @@ grub-core/gdb_grub.in: document num_modules
      ###
      
      set confirm off
    -@@ grub-core/gdb_grub.in: set confirm off
    - # fail.
    - 
    - set $platform_efi = $_streq("@platform@", "efi")
    -+set $target = "@target_cpu@-@platform@"
    - 
    - if ! $runonce
    - 	if $platform_efi
    - 		# Only load the executable file, not the symbols
    +@@ grub-core/gdb_grub.in: if ! $runonce
      		exec-file kernel.exec
      	else
    -+		if $_streq($target, "i386-pc")
    -+			add-symbol-file boot.image
    -+			add-symbol-file diskboot.image
    -+			add-symbol-file lzma_decompress.image
    -+		end
      		file kernel.exec
     +		run_on_start
      		runtime_load_module
 -:  ---------- > 13:  fbd217a89c gdb: Add extra early initialization symbols for i386-pc
14:  5fbef49d07 = 14:  973f24a485 gdb: Add ability to turn on shell tracing for gdb helper script
15:  b1f6f5861b = 15:  d6c6947762 docs: Add debugging chapter to development documentation
-- 
2.34.1



^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v4 01/15] gdb: Fix redirection issue in dump_module_sections
  2022-12-16  5:29 [PATCH v4 00/15] GDB script fixes and improvements Glenn Washburn
@ 2022-12-16  5:29 ` Glenn Washburn
  2022-12-16  5:29 ` [PATCH v4 02/15] gdb: Prevent wrapping when writing to .segments.tmp Glenn Washburn
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Glenn Washburn @ 2022-12-16  5:29 UTC (permalink / raw)
  To: grub-devel @ gnu . org, Daniel Kiper; +Cc: Glenn Washburn

An error in any GDB command causes it to immediately abort with an error,
this includes any command that calls that command. This leads to an issue
in dump_module_sections where an error causes the command to exit without
turning off file redirection. The user then ends up with a GDB command
line where commands output nothing to the console.

Instead do the work of dump_module_sections in the command
dump_module_sections_helper and run the command using GDB's pipe command
which does the redirection and undoes the redirection when it finishes
regardless of any errors in the command.

Also, remove .segments.tmp file prior to loading modules in case one was
left from a previous run.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/gdb_grub.in | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
index e322d3dc10..4e45ad5622 100644
--- a/grub-core/gdb_grub.in
+++ b/grub-core/gdb_grub.in
@@ -10,15 +10,8 @@
 ###
 
 # Add section numbers and addresses to .segments.tmp
-define dump_module_sections
+define dump_module_sections_helper
 	set $mod = $arg0
-
-	# FIXME: save logging status
-	set logging file .segments.tmp
-	set logging redirect on
-	set logging overwrite off
-	set logging on
-
 	printf "%s", $mod->name
 	set $segment = $mod->segment
 	while ($segment)
@@ -26,9 +19,10 @@ define dump_module_sections
 		set $segment = $segment->next
 	end
 	printf "\n"
+end
 
-	set logging off
-	# FIXME: restore logging status
+define dump_module_sections
+	pipe dump_module_sections_helper $arg0 | sh -c 'cat >>.segments.tmp'
 end
 document dump_module_sections
 	Gather information about module whose mod structure was
@@ -59,6 +53,7 @@ document load_module
 end
 
 define load_all_modules
+	shell rm -f .segments.tmp
 	set $this = grub_dl_head
 	while ($this != 0)
 		dump_module_sections $this
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v4 02/15] gdb: Prevent wrapping when writing to .segments.tmp
  2022-12-16  5:29 [PATCH v4 00/15] GDB script fixes and improvements Glenn Washburn
  2022-12-16  5:29 ` [PATCH v4 01/15] gdb: Fix redirection issue in dump_module_sections Glenn Washburn
@ 2022-12-16  5:29 ` Glenn Washburn
  2022-12-16  5:29 ` [PATCH v4 03/15] gdb: If no modules have been loaded, do not try to load module symbols Glenn Washburn
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Glenn Washburn @ 2022-12-16  5:29 UTC (permalink / raw)
  To: grub-devel @ gnu . org, Daniel Kiper; +Cc: Glenn Washburn

GDB logging is redirected to write .segments.tmp, which means that GDB
will wrap lines longer than what it thinks is the screen width
(typically 80 characters). When wrapping does occur it causes gmodule.pl
to misbehave. So disable line wrapping by using GDB's "with" command so
that its guaranteed to return the width to the previous value upon
command completion.

Also disable command tracing when dumping the module sections because
that output will go to .segments.tmp and thus cause gmodule.pl to
misbehave.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/gdb_grub.in | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
index 4e45ad5622..edb5a8872c 100644
--- a/grub-core/gdb_grub.in
+++ b/grub-core/gdb_grub.in
@@ -22,6 +22,10 @@ define dump_module_sections_helper
 end
 
 define dump_module_sections
+	# Set unlimited width so that lines don't get wrapped writing
+	# to .segments.tmp
+	with width 0 -- \
+	with trace-commands off -- \
 	pipe dump_module_sections_helper $arg0 | sh -c 'cat >>.segments.tmp'
 end
 document dump_module_sections
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v4 03/15] gdb: If no modules have been loaded, do not try to load module symbols
  2022-12-16  5:29 [PATCH v4 00/15] GDB script fixes and improvements Glenn Washburn
  2022-12-16  5:29 ` [PATCH v4 01/15] gdb: Fix redirection issue in dump_module_sections Glenn Washburn
  2022-12-16  5:29 ` [PATCH v4 02/15] gdb: Prevent wrapping when writing to .segments.tmp Glenn Washburn
@ 2022-12-16  5:29 ` Glenn Washburn
  2022-12-16  5:29 ` [PATCH v4 04/15] gdb: Move runtime module loading into runtime_load_module Glenn Washburn
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Glenn Washburn @ 2022-12-16  5:29 UTC (permalink / raw)
  To: grub-devel @ gnu . org, Daniel Kiper; +Cc: Glenn Washburn

This prevents load_all_modules from failing when called before any
modules have been loaded. Failures in GDB user-defined functions cause
any function which called them to also fail.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/gdb_grub.in | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
index edb5a8872c..fc17e3d899 100644
--- a/grub-core/gdb_grub.in
+++ b/grub-core/gdb_grub.in
@@ -63,7 +63,9 @@ define load_all_modules
 		dump_module_sections $this
 		set $this = $this->next
 	end
-	match_and_load_symbols
+	if (grub_dl_head != 0)
+		match_and_load_symbols
+	end
 end
 document load_all_modules
 	Load debugging information for all loaded modules.
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v4 04/15] gdb: Move runtime module loading into runtime_load_module
  2022-12-16  5:29 [PATCH v4 00/15] GDB script fixes and improvements Glenn Washburn
                   ` (2 preceding siblings ...)
  2022-12-16  5:29 ` [PATCH v4 03/15] gdb: If no modules have been loaded, do not try to load module symbols Glenn Washburn
@ 2022-12-16  5:29 ` Glenn Washburn
  2022-12-21 14:57   ` Daniel Kiper
  2022-12-16  5:29 ` [PATCH v4 05/15] gdb: Reliably load modules in runtime_load_module Glenn Washburn
                   ` (10 subsequent siblings)
  14 siblings, 1 reply; 31+ messages in thread
From: Glenn Washburn @ 2022-12-16  5:29 UTC (permalink / raw)
  To: grub-devel @ gnu . org, Daniel Kiper; +Cc: Glenn Washburn

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/gdb_grub.in | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
index fc17e3d899..d525a5a11f 100644
--- a/grub-core/gdb_grub.in
+++ b/grub-core/gdb_grub.in
@@ -71,16 +71,22 @@ document load_all_modules
 	Load debugging information for all loaded modules.
 end
 
+define runtime_load_module
+	break grub_dl_add
+	commands
+		silent
+		load_module mod
+		cont
+	end
+end
+document runtime_load_module
+	Load module symbols at runtime as they are loaded.
+end
+
 ###
 
 set confirm off
 file kernel.exec
 target remote :1234
 
-# inform when module is loaded
-break grub_dl_add
-commands
-	silent
-	load_module mod
-	cont
-end
+runtime_load_module
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v4 05/15] gdb: Reliably load modules in runtime_load_module
  2022-12-16  5:29 [PATCH v4 00/15] GDB script fixes and improvements Glenn Washburn
                   ` (3 preceding siblings ...)
  2022-12-16  5:29 ` [PATCH v4 04/15] gdb: Move runtime module loading into runtime_load_module Glenn Washburn
@ 2022-12-16  5:29 ` Glenn Washburn
  2022-12-16  5:29 ` [PATCH v4 06/15] gdb: Add functions to make loading from dynamically positioned targets easier Glenn Washburn
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Glenn Washburn @ 2022-12-16  5:29 UTC (permalink / raw)
  To: grub-devel @ gnu . org, Daniel Kiper; +Cc: Glenn Washburn

For an unknown reason, GDB has been observed to not hit the grub_dl_add()
software breakpoint in QEMU. When it does, its been observed to stop before
the stack frame is setup causing values of passed arguments to be garbage.
So instead, use a hardware break point on grub_dl_add(), which has not
shown these issues.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/gdb_grub.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
index d525a5a11f..1fb9603ff8 100644
--- a/grub-core/gdb_grub.in
+++ b/grub-core/gdb_grub.in
@@ -72,7 +72,7 @@ document load_all_modules
 end
 
 define runtime_load_module
-	break grub_dl_add
+	hbreak grub_dl_add
 	commands
 		silent
 		load_module mod
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v4 06/15] gdb: Add functions to make loading from dynamically positioned targets easier
  2022-12-16  5:29 [PATCH v4 00/15] GDB script fixes and improvements Glenn Washburn
                   ` (4 preceding siblings ...)
  2022-12-16  5:29 ` [PATCH v4 05/15] gdb: Reliably load modules in runtime_load_module Glenn Washburn
@ 2022-12-16  5:29 ` Glenn Washburn
  2022-12-16  5:29 ` [PATCH v4 07/15] gdb: Remove Perl dependency for GRUB GDB script Glenn Washburn
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Glenn Washburn @ 2022-12-16  5:29 UTC (permalink / raw)
  To: grub-devel @ gnu . org, Daniel Kiper; +Cc: Glenn Washburn

Many targets, such as EFI, load GRUB at addresses that are determined at
runtime. So the load addresses in kernel.exec will almost certainly be
wrong. Given the address of the start of the text segment, these
functions will tell GDB to load the symbols at the proper locations. It
is left up to the user to determine how to get the text address.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/Makefile.core.def |  6 ++++
 grub-core/gdb_grub.in       | 27 ++++++++++++++++
 grub-core/gdb_helper.sh.in  | 62 +++++++++++++++++++++++++++++++++++++
 3 files changed, 95 insertions(+)
 create mode 100644 grub-core/gdb_helper.sh.in

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 95942fc8c9..253b9b1e47 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -24,6 +24,12 @@ transform_data = {
   common = gmodule.pl.in;
 };
 
+transform_data = {
+  installdir = platform;
+  name = gdb_helper.sh;
+  common = gdb_helper.sh.in;
+};
+
 transform_data = {
   installdir = platform;
   name = gdb_grub;
diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
index 1fb9603ff8..d97dbfdc0d 100644
--- a/grub-core/gdb_grub.in
+++ b/grub-core/gdb_grub.in
@@ -9,6 +9,33 @@
 ### Lubomir Kundrak <lkudrak@skosi.org>
 ###
 
+define dynamic_load_kernel_exec_symbols
+	shell rm -f .remove-kernel.exec.symfile.gdb
+	shell sh gdb_helper.sh gen_kernel_exec_loadsym $arg0 >.kernel.exec.loadsym.gdb
+	source .kernel.exec.loadsym.gdb
+end
+document dynamic_load_kernel_exec_symbols
+	Load debugging symbols from kernel.exec given the address of the
+	.text segment of the UEFI binary in memory.
+end
+
+define dynamic_load_symbols
+	dynamic_load_kernel_exec_symbols $arg0
+
+	# We may have been very late to loading the kernel.exec symbols and
+	# and modules may already be loaded. So load symbols for any already
+	# loaded.
+	load_all_modules
+
+	runtime_load_module
+end
+document dynamic_load_symbols
+	Load debugging symbols from kernel.exec and any loaded modules given
+	the address of the .text segment of the UEFI binary in memory. Also
+	setup session to automatically load module symbols for modules loaded
+	in the future.
+end
+
 # Add section numbers and addresses to .segments.tmp
 define dump_module_sections_helper
 	set $mod = $arg0
diff --git a/grub-core/gdb_helper.sh.in b/grub-core/gdb_helper.sh.in
new file mode 100644
index 0000000000..b37d5adfc2
--- /dev/null
+++ b/grub-core/gdb_helper.sh.in
@@ -0,0 +1,62 @@
+###
+### Helper functions for GRUB's GDB script.
+###
+
+alignup() {
+  PAD=1
+  if [ "$(($1%$2))" -eq 0 ]; then
+    PAD=0
+  fi
+  printf "0x%x\n" "$(((($1/$2)+$PAD)*$2))"
+}
+
+exp() {
+  BASE=${1%%\*\**}
+  EXP=${1##*\*\*}
+  RES=1
+  while [ "$EXP" -gt 0 ]; do
+    RES=$(($RES*$BASE))
+    EXP=$(($EXP - 1))
+  done
+  echo $RES
+}
+
+# Loading symbols is complicated by the fact that kernel.exec is an ELF
+# ELF binary, but the UEFI runtime is PE32+. All the data sections of
+# the ELF binary are concatenated (accounting for ELF section alignment)
+# and put into one .data section in the PE32+ runtime image. So given
+# the load address of the .data PE32+ section we can determine the
+# addresses each ELF data section maps to. The UEFI application is
+# loaded into memory just as it is laid out in the file. It is not
+# assumed that the binary is available, but it is known that the .text
+# section directly precedes the .data section and that .data is EFI
+# page aligned. Using this, the .data offset from .text can be found.
+gen_kernel_exec_loadsym() {
+  PE_SECTION_ALIGN=$((1<<12))
+  PE_TEXT=$1
+  TSIZE=$((0x$(objdump -h kernel.exec | grep -E " \.text\b" | \
+                  (read _ _ SIZE _; echo $SIZE))))
+  PE_DATA_OFF=$(printf "0x%x" $(($(alignup ${TSIZE} ${PE_SECTION_ALIGN}))))
+
+  printf "add-symbol-file kernel.exec ${PE_TEXT}"
+  objdump -h kernel.exec | tail -n +6 | \
+    while read IDX NAME SIZE _ _ OFFSET ALIGN; do
+      read FLAGS
+      if [ -n "$FLAGS" ] && [ -z "${FLAGS%%*DATA*}" -o "$NAME" = .bss ]; then
+        OFF=$(alignup ${OFF:-0} $(exp $ALIGN))
+	printf " -s $NAME (${PE_TEXT}+${PE_DATA_OFF}+0x%x)" "$OFF"
+        OFF=$((${OFF} + 0x${SIZE}))
+      fi
+    done
+}
+
+if type "$1" 2>/dev/null | grep -q 'is a shell function'; then
+  if [ "x${GRUB_GDB_TRACE}" = "xy" ]; then
+    exec 2>>gdb_helper.trace
+    set -x
+  fi
+
+  "$@"
+else
+  exit 1
+fi
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v4 07/15] gdb: Remove Perl dependency for GRUB GDB script
  2022-12-16  5:29 [PATCH v4 00/15] GDB script fixes and improvements Glenn Washburn
                   ` (5 preceding siblings ...)
  2022-12-16  5:29 ` [PATCH v4 06/15] gdb: Add functions to make loading from dynamically positioned targets easier Glenn Washburn
@ 2022-12-16  5:29 ` Glenn Washburn
  2022-12-16  5:29 ` [PATCH v4 08/15] gdb: If enabled, print line used to load EFI kernel symbols when using gdb_grub script Glenn Washburn
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Glenn Washburn @ 2022-12-16  5:29 UTC (permalink / raw)
  To: grub-devel @ gnu . org, Daniel Kiper; +Cc: Glenn Washburn

Remove gmodule.pl and rewrite as a shell function in gdb_helper.sh.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/Makefile.core.def |  6 ------
 grub-core/gdb_grub.in       |  4 ++--
 grub-core/gdb_helper.sh.in  | 41 +++++++++++++++++++++++++++++++++++++
 grub-core/gmodule.pl.in     | 30 ---------------------------
 4 files changed, 43 insertions(+), 38 deletions(-)
 delete mode 100644 grub-core/gmodule.pl.in

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 253b9b1e47..c5feae285c 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -18,12 +18,6 @@ transform_data = {
   common = modinfo.sh.in;
 };
 
-transform_data = {
-  installdir = platform;
-  name = gmodule.pl;
-  common = gmodule.pl.in;
-};
-
 transform_data = {
   installdir = platform;
   name = gdb_helper.sh;
diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
index d97dbfdc0d..f901975e15 100644
--- a/grub-core/gdb_grub.in
+++ b/grub-core/gdb_grub.in
@@ -1,6 +1,6 @@
 ###
 ### Load debuging information about GNU GRUB 2 modules into GDB
-### automatically. Needs readelf, Perl and gmodule.pl script
+### automatically. Needs readelf, objdump and gdb_helper.sh script
 ###
 ### Has to be launched from the writable and trusted
 ### directory containing *.image and *.module
@@ -63,7 +63,7 @@ end
 # Generate and execute GDB commands and delete temporary files
 # afterwards
 define match_and_load_symbols
-	shell perl gmodule.pl <.segments.tmp >.loadsym.gdb
+	shell sh gdb_helper.sh gen_module_loadsym <.segments.tmp >.loadsym.gdb
 	source .loadsym.gdb
 	shell rm -f .segments.tmp .loadsym.gdb
 end
diff --git a/grub-core/gdb_helper.sh.in b/grub-core/gdb_helper.sh.in
index b37d5adfc2..1eaa976fb7 100644
--- a/grub-core/gdb_helper.sh.in
+++ b/grub-core/gdb_helper.sh.in
@@ -21,6 +21,21 @@ exp() {
   echo $RES
 }
 
+get_section_names() {
+  readelf -SW "$1" | tail -n +5 | \
+    while read LINE; do
+      LINE=${LINE#*\[??*\] }
+      NAME=${LINE%% *}
+      echo -n "${NAME:-<NULL>} "
+    done
+}
+
+get_word() {
+  i=$2
+  ( set -- $1; eval echo "\${$i}"; )
+}
+
+
 # Loading symbols is complicated by the fact that kernel.exec is an ELF
 # ELF binary, but the UEFI runtime is PE32+. All the data sections of
 # the ELF binary are concatenated (accounting for ELF section alignment)
@@ -50,6 +65,32 @@ gen_kernel_exec_loadsym() {
     done
 }
 
+# Generate GDB commands, that load symbols for specified modules from stdin,
+# with proper section relocations.
+gen_module_loadsym() {
+  while read NAME SECTIONMAP; do
+    echo -n "add-symbol-file $NAME.module"
+    (
+      SECNAMES=$(get_section_names "$NAME.mod")
+
+      set -- $SECTIONMAP
+      while [ "$#" -gt 0 ]; do
+	SECIDX=$1
+	SECADDR=$2
+	SECNAME=$(get_word "$SECNAMES" $((${SECIDX}+1)))
+	shift; shift
+
+	if [ "$SECNAME" = ".text" ]; then
+	  echo -n " ${SECADDR}"
+	else
+	  echo -n " -s ${SECNAME} ${SECADDR}"
+	fi
+      done
+    )
+    echo
+  done
+}
+
 if type "$1" 2>/dev/null | grep -q 'is a shell function'; then
   if [ "x${GRUB_GDB_TRACE}" = "xy" ]; then
     exec 2>>gdb_helper.trace
diff --git a/grub-core/gmodule.pl.in b/grub-core/gmodule.pl.in
deleted file mode 100644
index 78aa1e64eb..0000000000
--- a/grub-core/gmodule.pl.in
+++ /dev/null
@@ -1,30 +0,0 @@
-###
-### Generate GDB commands, that load symbols for specified module,
-### with proper section relocations. See .gdbinit
-###
-### $Id: gmodule.pl,v 1.2 2006/05/14 11:38:42 lkundrak Exp lkundrak $
-### Lubomir Kundrak <lkudrak@skosi.org>
-###
-
-use strict;
-
-while (<>) {
-	my ($name, %sections) = split;
-
-	print "add-symbol-file $name.module";
-
-	open (READELF, "readelf -S $name.mod |") or die;
-	while (<READELF>) {
-		/\[\s*(\d+)\]\s+(\.\S+)/ or next;
-
-		if ($2 eq '.text') {
-			print " $sections{$1}";
-			next;
-		}
-
-		print " -s $2 $sections{$1}"
-			if ($sections{$1} ne '0x0' and $sections{$1} ne '');
-	};
-	close (READELF);
-	print "\n";
-}
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v4 08/15] gdb: If enabled, print line used to load EFI kernel symbols when using gdb_grub script
  2022-12-16  5:29 [PATCH v4 00/15] GDB script fixes and improvements Glenn Washburn
                   ` (6 preceding siblings ...)
  2022-12-16  5:29 ` [PATCH v4 07/15] gdb: Remove Perl dependency for GRUB GDB script Glenn Washburn
@ 2022-12-16  5:29 ` Glenn Washburn
  2022-12-21 15:20   ` Daniel Kiper
  2022-12-16  5:29 ` [PATCH v4 09/15] gdb: Conditionally run GDB script logic for dynamically or statically positioned GRUB Glenn Washburn
                   ` (6 subsequent siblings)
  14 siblings, 1 reply; 31+ messages in thread
From: Glenn Washburn @ 2022-12-16  5:29 UTC (permalink / raw)
  To: grub-devel @ gnu . org, Daniel Kiper; +Cc: Glenn Washburn, Peter Jones

If the macro PRINT_GDB_SYM_LOAD_CMD is non-zero, compile code which will
print the command needed to load symbols for the GRUB EFI kernel. This is
needed because EFI firmware determines where to load the GRUB EFI at
runtime, and so the relevant addresses are not known ahead of time.

The command is a custom command defined in the gdb_grub GDB script. So
GDB should be started with the script as an argument to the -x option or
sourced into an active GDB session before running the outputted command.

Co-developed-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 config.h.in               |  3 +++
 grub-core/kern/efi/efi.c  |  4 ++--
 grub-core/kern/efi/init.c | 19 ++++++++++++++++++-
 include/grub/efi/efi.h    |  2 +-
 4 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/config.h.in b/config.h.in
index 4d1e50eba7..c31eee1bca 100644
--- a/config.h.in
+++ b/config.h.in
@@ -13,6 +13,9 @@
 #define MM_DEBUG @MM_DEBUG@
 #endif
 
+/* Define to 1 to enable printing of gdb command to load module symbols.  */
+#define PRINT_GDB_SYM_LOAD_CMD 0
+
 /* Define to 1 to enable disk cache statistics.  */
 #define DISK_CACHE_STATS @DISK_CACHE_STATS@
 #define BOOT_TIME_STATS @BOOT_TIME_STATS@
diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
index cf49d6357e..17bd06f7e6 100644
--- a/grub-core/kern/efi/efi.c
+++ b/grub-core/kern/efi/efi.c
@@ -299,7 +299,7 @@ grub_efi_get_variable (const char *var, const grub_efi_guid_t *guid,
 /* Search the mods section from the PE32/PE32+ image. This code uses
    a PE32 header, but should work with PE32+ as well.  */
 grub_addr_t
-grub_efi_modules_addr (void)
+grub_efi_section_addr (const char *section_name)
 {
   grub_efi_loaded_image_t *image;
   struct grub_msdos_image_header *header;
@@ -328,7 +328,7 @@ grub_efi_modules_addr (void)
        i < coff_header->num_sections;
        i++, section++)
     {
-      if (grub_strcmp (section->name, "mods") == 0)
+      if (grub_strcmp (section->name, section_name) == 0)
 	break;
     }
 
diff --git a/grub-core/kern/efi/init.c b/grub-core/kern/efi/init.c
index b67bc73a1b..ed3d463c8f 100644
--- a/grub-core/kern/efi/init.c
+++ b/grub-core/kern/efi/init.c
@@ -101,10 +101,24 @@ stack_protector_init (void)
 
 grub_addr_t grub_modbase;
 
+#if PRINT_GDB_SYM_LOAD_CMD
+static void
+grub_efi_print_gdb_info (void)
+{
+  grub_addr_t text;
+
+  text = grub_efi_section_addr (".text");
+  if (!text)
+    return;
+
+  grub_printf ("dynamic_load_symbols %p\n", (void *)text);
+}
+#endif
+
 void
 grub_efi_init (void)
 {
-  grub_modbase = grub_efi_modules_addr ();
+  grub_modbase = grub_efi_section_addr ("mods");
   /* First of all, initialize the console so that GRUB can display
      messages.  */
   grub_console_init ();
@@ -127,6 +141,9 @@ grub_efi_init (void)
   efi_call_4 (grub_efi_system_table->boot_services->set_watchdog_timer,
 	      0, 0, 0, NULL);
 
+#if PRINT_GDB_SYM_LOAD_CMD
+  grub_efi_print_gdb_info ();
+#endif
   grub_efidisk_init ();
 }
 
diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
index e61272de53..586ac856b5 100644
--- a/include/grub/efi/efi.h
+++ b/include/grub/efi/efi.h
@@ -109,7 +109,7 @@ grub_err_t grub_arch_efi_linux_boot_image(grub_addr_t addr, grub_size_t size,
                                            char *args);
 #endif
 
-grub_addr_t grub_efi_modules_addr (void);
+grub_addr_t grub_efi_section_addr (const char *section);
 
 void grub_efi_mm_init (void);
 void grub_efi_mm_fini (void);
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v4 09/15] gdb: Conditionally run GDB script logic for dynamically or statically positioned GRUB
  2022-12-16  5:29 [PATCH v4 00/15] GDB script fixes and improvements Glenn Washburn
                   ` (7 preceding siblings ...)
  2022-12-16  5:29 ` [PATCH v4 08/15] gdb: If enabled, print line used to load EFI kernel symbols when using gdb_grub script Glenn Washburn
@ 2022-12-16  5:29 ` Glenn Washburn
  2022-12-16  5:29 ` [PATCH v4 10/15] gdb: Only connect to remote target once when first sourced Glenn Washburn
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Glenn Washburn @ 2022-12-16  5:29 UTC (permalink / raw)
  To: grub-devel @ gnu . org, Daniel Kiper; +Cc: Glenn Washburn

There are broadly two classes of targets to consider when loading symbols
for GRUB, targets that determine where to load GRUB at runtime
(dynamically positioned) and those that do not (statically positioned).
For statically poisitioned targets, symbol loading is determined at link
time, so nothing more needs to be known to load the symbols. For
dynamically positioned targets, such as EFI targets, at runtime symbols
should be offset by an amount that depends on where the runtime chose to
load GRUB.

It is important to not load symbols statically for dynamic targets
because then when subsequently loading the symbols correctly one must
take care to remove the existing static symbols, otherwise there will be
two sets of symbols and GDB seems to prefer the ones loaded first (ie the
static ones).

Use autoconf variables to generate a gdb_grub for a particular target,
which conditionally run startup code depending on if the target uses
static or dynamic loading.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/gdb_grub.in | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
index f901975e15..1a3dcbd57d 100644
--- a/grub-core/gdb_grub.in
+++ b/grub-core/gdb_grub.in
@@ -113,7 +113,20 @@ end
 ###
 
 set confirm off
-file kernel.exec
-target remote :1234
 
-runtime_load_module
+# Note: On EFI and other platforms that load GRUB to an address that is
+# determined at runtime, the symbols in kernel.exec will be wrong.
+# However, we must start by loading some executable file or GDB will
+# fail.
+
+set $platform_efi = $_streq("@platform@", "efi")
+
+if $platform_efi
+	# Only load the executable file, not the symbols
+	exec-file kernel.exec
+else
+	file kernel.exec
+	runtime_load_module
+end
+
+target remote :1234
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v4 10/15] gdb: Only connect to remote target once when first sourced
  2022-12-16  5:29 [PATCH v4 00/15] GDB script fixes and improvements Glenn Washburn
                   ` (8 preceding siblings ...)
  2022-12-16  5:29 ` [PATCH v4 09/15] gdb: Conditionally run GDB script logic for dynamically or statically positioned GRUB Glenn Washburn
@ 2022-12-16  5:29 ` Glenn Washburn
  2022-12-16  5:29 ` [PATCH v4 11/15] gdb: Allow user defined "onload_<modname>" command to be run when module is loaded Glenn Washburn
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Glenn Washburn @ 2022-12-16  5:29 UTC (permalink / raw)
  To: grub-devel @ gnu . org, Daniel Kiper; +Cc: Glenn Washburn

The gdb_grub script was originally meant to be run once when GDB first
starts up via the -x argument. So it runs commands unconditionally
assuming that the script has not been run before. Its nice to be able
to source the script again when developing the script to modify/add
commands. So only run the commands not defined in user-defined commands,
if a variable $runonce has already been set and when those commands have
been run to set $runonce.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/gdb_grub.in | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
index 1a3dcbd57d..a0c6b1ae07 100644
--- a/grub-core/gdb_grub.in
+++ b/grub-core/gdb_grub.in
@@ -121,12 +121,15 @@ set confirm off
 
 set $platform_efi = $_streq("@platform@", "efi")
 
-if $platform_efi
-	# Only load the executable file, not the symbols
-	exec-file kernel.exec
-else
-	file kernel.exec
-	runtime_load_module
-end
+if ! $runonce
+	if $platform_efi
+		# Only load the executable file, not the symbols
+		exec-file kernel.exec
+	else
+		file kernel.exec
+		runtime_load_module
+	end
 
-target remote :1234
+	target remote :1234
+	set $runonce = 1
+end
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v4 11/15] gdb: Allow user defined "onload_<modname>" command to be run when module is loaded
  2022-12-16  5:29 [PATCH v4 00/15] GDB script fixes and improvements Glenn Washburn
                   ` (9 preceding siblings ...)
  2022-12-16  5:29 ` [PATCH v4 10/15] gdb: Only connect to remote target once when first sourced Glenn Washburn
@ 2022-12-16  5:29 ` Glenn Washburn
  2022-12-16  5:29 ` [PATCH v4 12/15] gdb: Allow running user-defined commands at GRUB start Glenn Washburn
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Glenn Washburn @ 2022-12-16  5:29 UTC (permalink / raw)
  To: grub-devel @ gnu . org, Daniel Kiper; +Cc: Glenn Washburn

When debugging a module it can be desirable to set break points on code
in the module. This is difficult in GRUB because, at GDB start, the
module is not loaded and on EFI platforms its not known ahead of time
where the module will be loaded. So allow users to create an
"onload_<modname>" command which will be run when the module with name
"modname" is loaded.

Create new command "is_user_command" which sets $ret to true value if
the first argument is the name of a user-defined command.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/gdb_grub.in | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
index a0c6b1ae07..8ae6344edf 100644
--- a/grub-core/gdb_grub.in
+++ b/grub-core/gdb_grub.in
@@ -9,6 +9,20 @@
 ### Lubomir Kundrak <lkudrak@skosi.org>
 ###
 
+define is_user_command
+	eval "pipe help user-defined | grep -q '%s'", $arg0
+	set $ret = ! $_shell_exitcode
+end
+document is_user_command
+	Set $ret to true value if first argument is the name of a user-defined
+	command.
+end
+
+define is_onload_command
+	eval "set $is_onload_command_name = \"onload_%s\"", $arg0
+	is_user_command $is_onload_command_name
+end
+
 define dynamic_load_kernel_exec_symbols
 	shell rm -f .remove-kernel.exec.symfile.gdb
 	shell sh gdb_helper.sh gen_kernel_exec_loadsym $arg0 >.kernel.exec.loadsym.gdb
@@ -76,8 +90,18 @@ end
 ###
 
 define load_module
+	set $load_module_onload_cmd = ""
+	is_onload_command $arg0->name
+	if $ret
+		eval "set $load_module_onload_cmd = \"onload_%s (grub_dl_t)%p\"", $arg0->name, $arg0
+	end
+
 	dump_module_sections $arg0
 	match_and_load_symbols
+
+	if ! $_streq($load_module_onload_cmd, "")
+		eval "%s", $load_module_onload_cmd
+	end
 end
 document load_module
 	Load debugging information for module given as argument.
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v4 12/15] gdb: Allow running user-defined commands at GRUB start
  2022-12-16  5:29 [PATCH v4 00/15] GDB script fixes and improvements Glenn Washburn
                   ` (10 preceding siblings ...)
  2022-12-16  5:29 ` [PATCH v4 11/15] gdb: Allow user defined "onload_<modname>" command to be run when module is loaded Glenn Washburn
@ 2022-12-16  5:29 ` Glenn Washburn
  2022-12-21 15:27   ` Daniel Kiper
  2022-12-16  5:29 ` [PATCH v4 13/15] gdb: Add extra early initialization symbols for i386-pc Glenn Washburn
                   ` (2 subsequent siblings)
  14 siblings, 1 reply; 31+ messages in thread
From: Glenn Washburn @ 2022-12-16  5:29 UTC (permalink / raw)
  To: grub-devel @ gnu . org, Daniel Kiper; +Cc: Glenn Washburn

A new command, run_on_start, is created which handles some complexities
of the EFI platform when breaking on GRUB start. If GRUB start is hooked,
run "onstart" command if it is defned.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/gdb_grub.in | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
index 8ae6344edf..3b3cea1a4d 100644
--- a/grub-core/gdb_grub.in
+++ b/grub-core/gdb_grub.in
@@ -36,6 +36,8 @@ end
 define dynamic_load_symbols
 	dynamic_load_kernel_exec_symbols $arg0
 
+	run_on_start
+
 	# We may have been very late to loading the kernel.exec symbols and
 	# and modules may already be loaded. So load symbols for any already
 	# loaded.
@@ -134,6 +136,41 @@ document runtime_load_module
 	Load module symbols at runtime as they are loaded.
 end
 
+define run_on_start
+	# TODO: Add check to see if _start symbol is defined, if not, then
+	# the symbols have not yet been loaded and this command will not work.
+	watch *_start
+	set $break_efi_start_bpnum = $bpnum
+	commands
+		silent
+		delete $break_efi_start_bpnum
+		break _start
+		commands
+			silent
+			delete $break_efi_start_bpnum
+			set $onstart_name = "onstart"
+			is_user_command $onstart_name
+			if $ret
+				onstart
+			end
+			continue
+		end
+		set $break_efi_start_bpnum = $bpnum
+		continue
+	end
+end
+document run_on_start
+	On some targets, such as x86_64-efi, even if you know where the
+	firmware will load the grub image, you can not simply set a break
+	point before the image is loaded because loading the image
+	overwrites the break point in memory. So setup a hardware watch
+	point, which does not have that problem, and if that gets triggered,
+	then reset the break point. If a user-defined command named
+	"onstart" exists it will be run after the start is hit.
+	NOTE: This assumes symbols have already been correctly loaded for
+	the EFI application.
+end
+
 ###
 
 set confirm off
@@ -151,6 +188,7 @@ if ! $runonce
 		exec-file kernel.exec
 	else
 		file kernel.exec
+		run_on_start
 		runtime_load_module
 	end
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v4 13/15] gdb: Add extra early initialization symbols for i386-pc
  2022-12-16  5:29 [PATCH v4 00/15] GDB script fixes and improvements Glenn Washburn
                   ` (11 preceding siblings ...)
  2022-12-16  5:29 ` [PATCH v4 12/15] gdb: Allow running user-defined commands at GRUB start Glenn Washburn
@ 2022-12-16  5:29 ` Glenn Washburn
  2022-12-21 15:28   ` Daniel Kiper
  2022-12-16  5:29 ` [PATCH v4 14/15] gdb: Add ability to turn on shell tracing for gdb helper script Glenn Washburn
  2022-12-16  5:29 ` [PATCH v4 15/15] docs: Add debugging chapter to development documentation Glenn Washburn
  14 siblings, 1 reply; 31+ messages in thread
From: Glenn Washburn @ 2022-12-16  5:29 UTC (permalink / raw)
  To: grub-devel @ gnu . org, Daniel Kiper; +Cc: Glenn Washburn

Add symbols for boot.image, disk.image, and lzma_decompress.image if the
target is i386-pc.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/gdb_grub.in | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
index 3b3cea1a4d..a9c9d00430 100644
--- a/grub-core/gdb_grub.in
+++ b/grub-core/gdb_grub.in
@@ -181,12 +181,18 @@ set confirm off
 # fail.
 
 set $platform_efi = $_streq("@platform@", "efi")
+set $target = "@target_cpu@-@platform@"
 
 if ! $runonce
 	if $platform_efi
 		# Only load the executable file, not the symbols
 		exec-file kernel.exec
 	else
+		if $_streq($target, "i386-pc")
+			add-symbol-file boot.image
+			add-symbol-file diskboot.image
+			add-symbol-file lzma_decompress.image
+		end
 		file kernel.exec
 		run_on_start
 		runtime_load_module
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v4 14/15] gdb: Add ability to turn on shell tracing for gdb helper script
  2022-12-16  5:29 [PATCH v4 00/15] GDB script fixes and improvements Glenn Washburn
                   ` (12 preceding siblings ...)
  2022-12-16  5:29 ` [PATCH v4 13/15] gdb: Add extra early initialization symbols for i386-pc Glenn Washburn
@ 2022-12-16  5:29 ` Glenn Washburn
  2022-12-16  5:29 ` [PATCH v4 15/15] docs: Add debugging chapter to development documentation Glenn Washburn
  14 siblings, 0 replies; 31+ messages in thread
From: Glenn Washburn @ 2022-12-16  5:29 UTC (permalink / raw)
  To: grub-devel @ gnu . org, Daniel Kiper; +Cc: Glenn Washburn

If envvar GRUB_GDB_TRACE_FILE is set, turn on shell tracing and write
stderr messages, which includes trace messages, to path specified in the
value of the envvar.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/gdb_helper.sh.in | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/grub-core/gdb_helper.sh.in b/grub-core/gdb_helper.sh.in
index 1eaa976fb7..13f79bfb07 100644
--- a/grub-core/gdb_helper.sh.in
+++ b/grub-core/gdb_helper.sh.in
@@ -2,6 +2,11 @@
 ### Helper functions for GRUB's GDB script.
 ###
 
+if [ -n "$GRUB_GDB_TRACE_FILE" ]; then
+  set -x
+  exec 2>"$GRUB_GDB_TRACE_FILE"
+fi
+
 alignup() {
   PAD=1
   if [ "$(($1%$2))" -eq 0 ]; then
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v4 15/15] docs: Add debugging chapter to development documentation
  2022-12-16  5:29 [PATCH v4 00/15] GDB script fixes and improvements Glenn Washburn
                   ` (13 preceding siblings ...)
  2022-12-16  5:29 ` [PATCH v4 14/15] gdb: Add ability to turn on shell tracing for gdb helper script Glenn Washburn
@ 2022-12-16  5:29 ` Glenn Washburn
  2022-12-21  3:07   ` Jeremy Szu
  2022-12-21 15:50   ` Daniel Kiper
  14 siblings, 2 replies; 31+ messages in thread
From: Glenn Washburn @ 2022-12-16  5:29 UTC (permalink / raw)
  To: grub-devel @ gnu . org, Daniel Kiper; +Cc: Glenn Washburn

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 docs/grub-dev.texi | 191 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 191 insertions(+)

diff --git a/docs/grub-dev.texi b/docs/grub-dev.texi
index f76fc658bf..8171e91c33 100644
--- a/docs/grub-dev.texi
+++ b/docs/grub-dev.texi
@@ -79,6 +79,7 @@ This edition documents version @value{VERSION}.
 * Contributing Changes::
 * Setting up and running test suite::
 * Updating External Code::
+* Debugging::
 * Porting::
 * Error Handling::
 * Stack and heap size::
@@ -595,6 +596,196 @@ cp minilzo-2.10/*.[hc] grub-core/lib/minilzo
 rm -r minilzo-2.10*
 @end example
 
+@node Debugging
+@chapter Debugging
+
+GRUB2 can be difficult to debug because it runs on the bare-metal and thus
+does not have the debugging facilities normally provided by an operating
+system. This chapter aims to provide useful information on some ways to
+debug GRUB2 for some architectures. It by no means intends to be exhaustive.
+The focus will be one X86_64 and i386 architectures. Luckily for some issues
+virtual machines have made the ability to debug GRUB2 much easier, and this
+chapter will focus debugging via the QEMU virtual machine. We will not be
+going over debugging of the userland tools (eg. grub-install), there are
+many tutorials on debugging programs in userland.
+
+You will need GDB and the QEMU binaries for your system, on Debian these
+can be installed with the @samp{gdb} and @samp{qemu-system-x86} packages.
+Also it is assumed that you have already successfully compiled GRUB2 from
+source for the target specified in the section below and have some
+familiarity with GDB. When GRUB2 is built it will create many different
+binaries. The ones of concern will be in the @file{grub-core}
+directory of the GRUB2 build dir. To aide in debugging we will want the
+debugging symbols generated during the build because these symbols are not
+kept in the binaries which get installed to the boot location. The build
+process outputs two sets of binaries, one without symbols which gets executed
+at boot, and another set of ELF images with debugging symbols. The built
+images with debugging symbols will have a @file{.image} suffix, and the ones
+without a @file{.img} suffix. Similarly, loadable modules with debugging
+symbols will have a @file{.module} suffix, and ones without a @file{.mod}
+suffix. In the case of the kernel the binary with symbols is named
+@file{kernel.exec}.
+
+In the following sections, information will be provided on debugging on
+various targets using @command{gdb} and the @samp{gdb_grub} GDB script.
+
+@menu
+* i386-pc::
+* x86_64-efi::
+@end menu
+
+@node i386-pc
+@section i386-pc
+
+The i386-pc target is a good place to start when first debugging GRUB2
+because in some respects its easier than EFI platforms. The reason being
+that the initial load address is always known in advance. To start
+debugging GRUB2 first QEMU must be started in GDB stub mode. The following
+command is a simple illustration:
+
+@example
+qemu-system-i386 -drive file=disk.img,format=raw \
+    -device virtio-scsi-pci,id=scsi0,num_queues=4 -S -s
+@end example
+
+This will start a QEMU instance booting from @file{disk.img}. It will pause
+at start waiting for a GDB instance to attach to it. You should change
+@file{disk.img} to something more appropriate. A block device can be used,
+but you may need to run QEMU as a privileged user.
+
+To connect to this QEMU instance with GDB, the @code{target remote} GDB
+command must be used. We also need to load a binary image, preferably with
+symbols. This can be done using the GDB command @code{file kernel.exec}, if
+GDB is started from the @file{grub-core} directory in the GRUB2 build
+directory. GRUB2 developers have made this more simple by including a GDB
+script which does much of the setup. This file at @file{grub-core/gdb_grub}
+of the build directory and is also installed via @command{make install}.
+If not building GRUB, the distribution may have a package which installs
+this GDB script along with debug symbol binaries, such as Debian's
+@samp{grub-pc-dbg} package. The GDB scripts is intended to by used
+like so, assuming:
+
+@example
+cd $(dirname /path/to/script/gdb_grub)
+gdb -x gdb_grub
+@end example
+
+Once GDB has been started with the @file{gdb_grub} script it will
+automatically connect to the QEMU instance. You can then do things you
+normally would in GDB like set a break point on @var{grub_main}.
+
+Setting breakpoints in modules is trickier since they haven't been loaded
+yet and are loaded at addresses determined at runtime. The module could be
+loaded to different addresses in different QEMU instances. The debug symbols
+in the modules @file{.module} binary, thus are always wrong, and GDB needs
+to be told where to load the symbols to. But this must happen at runtime
+after GRUB2 has determined where the module will get loaded. Luckily the
+@file{gdb_grub} script takes care of this with the @command{runtime_load_module}
+command, which configures GDB to watch for GRUB2 module loading and when
+it does add the module symbols with the appropriate offset.
+
+@node x86_64-efi
+@section x86_64-efi
+
+Using GDB to debug GRUB2 for the x86_64-efi target has some similarities with
+the i386-pc target. Please read be familiar with the @ref{x86_64-efi} section
+when reading this one. Extra care must be used to run QEMU such that it boots
+a UEFI firmware. This usually involves either using the @samp{-bios} option
+with a UEFI firmware blob (eg. @file{OVMF.fd}) or loading the firmware via
+pflash. This document will not go further into how to do this as there are
+ample resource on the web.
+
+Like all EFI implementations, on x86_64-efi the (U)EFI firmware that loads
+the GRUB2 EFI application determines at runtime where the application will
+be loaded. This means that we do not know where to tell GDB to load the
+symbols for the GRUB2 core until the (U)EFI firmware determines it. There
+two good ways of figuring this out when running in QEMU: use a @ref{OVMF debug log,
+debug build of OVMF} and check the debug log or have GRUB2 say where it is
+loaded when it starts. Neither of these are ideal because they both
+generally give the information after GRUB2 is already running, which makes
+debugging early boot infeasible. Technically, the first method does give
+the load address before GRUB2 is run, but without debugging the EFI firmware
+with symbols, the author currently does not know how to cause the OVMF
+firmware to pause at that point to use the load address before GRUB2 is run.
+
+Even after getting the application load address, the loading of core symbols
+is complicated by the fact that the debugging symbols for the kernel are in
+an ELF binary named @file{kernel.exec} while what is in memory are sections
+for the PE32+ EFI binary. When @command{grub-mkimage} creates the PE32+
+binary it condenses several segments from the ELF kernel binary into one
+.data section in the PE32+ binary. This must be taken into account to
+properly load the other non-text sections. Otherwise, GDB will work as
+expected when breaking on functions, but, for instance, global variables
+will point to the wrong address in memory and thus give incorrect values
+(which can be difficult to debug).
+
+The calculating of the correct offsets for sections when loading symbol
+files are taken care of when loading the kernel symbols via the user-defined
+GDB command @command{dynamic_load_kernel_exec_symbols}, which takes one
+argument, the address where the text section is loaded, as determined by
+one of the methods above. Alternatively, the command @command{dynamic_load_symbols}
+with the text section address as an agrument can be called to load the
+kernel symbols and setup loading the module symbols as they are loaded at
+runtime.
+
+In the author's experience, when debugging with QEMU and OVMF, to have
+debugging symbols loaded at the start of GRUB2 execution the GRUB2 EFI
+application must be run via QEMU at least once prior in order to get the
+load address. Two methods for obtaining the load address are described in
+two subsections below. Generally speaking, the load address does not change
+between QEMU runs. There are exceptions to this, namely that different
+GRUB2 EFI Applications can be run at different addresses. Also, its been
+observed that after running the EFI application for the first time, the
+second run will many times have a different load address, but subsequent
+runs of the same EFI application will have the same load address as the
+second run. This predictability allows us to asume the load address on
+subsequent runs and thus load the symbols before GRUB2 starts. The following
+command illustrates this, assuming that QEMU is running and waiting for
+a debugger connection and the current working directory is where
+@file{gdb_grub} resides:
+
+@example
+gdb -x gdb_grub -ex 'dynamic_load_symbols @var{load address}'
+@end example
+
+If you load the symbols in this manner and, after continuing execution, do
+not see output showing the loading of modules symbol, then its very likely
+that the load address was incorrect.
+
+
+@node OVMF debug log
+@subsection OVMF debug log
+
+In order to get the GRUB2 load address from OVMF, first, a debug build
+of OVMF must be obtained (@uref{https://github.com/retrage/edk2-nightly/raw/master/bin/DEBUGX64_OVMF.fd,
+here is one} which is not officially recommended). OVMF will output debug
+messages to a special serial device, which we must add to QEMU. The following
+QEMU command will run the debug OVMF and write the debug messages to a
+file named @file{debug.log}. It is assumed that @file{disk.img} is a disk
+image or block device that is setup to boot GRUB2 EFI.
+
+@example
+qemu-system-x86_64 -bios /path/to/debug/OVMF.fd \
+    -drive file=disk.img,format=raw \
+    -device virtio-scsi-pci,id=scsi0,num_queues=4 \
+    -debugcon file:debug.log -global isa-debugcon.iobase=0x402
+@end example
+
+If GRUB2 was started by the (U)EFI firmware, then in the @file{debug.log}
+file one of the last lines should be a log message like:
+@code{Loading driver at 0x00006AEE000 EntryPoint=0x00006AEE756}. This
+means that the GRUB2 EFI application was loaded at @code{0x00006AEE000} and
+its .text section is at @code{0x00006AEE756}.
+
+@node Build GRUB2 to print out the load address
+@subsection Build GRUB2 to print out the load address
+
+GRUB2 can be specially built to output the address of its .text section in
+memory by defining @code{PRINT_GDB_SYM_LOAD_CMD} to @code{1} in @file{config.h.in}
+before running @command{configure}. The benefit of this method is that it
+will work on non-virtualized hardware where the (U)EFI firmware may not
+be modifiable.
+
 @node Porting
 @chapter Porting
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH v4 15/15] docs: Add debugging chapter to development documentation
  2022-12-16  5:29 ` [PATCH v4 15/15] docs: Add debugging chapter to development documentation Glenn Washburn
@ 2022-12-21  3:07   ` Jeremy Szu
  2022-12-22  6:12     ` Glenn Washburn
  2022-12-21 15:50   ` Daniel Kiper
  1 sibling, 1 reply; 31+ messages in thread
From: Jeremy Szu @ 2022-12-21  3:07 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: Daniel Kiper, Glenn Washburn

On Fri, Dec 16, 2022 at 1:33 PM Glenn Washburn
<development@efficientek.com> wrote:
>
> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
>  docs/grub-dev.texi | 191 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 191 insertions(+)
>
> diff --git a/docs/grub-dev.texi b/docs/grub-dev.texi
> index f76fc658bf..8171e91c33 100644
> --- a/docs/grub-dev.texi
> +++ b/docs/grub-dev.texi
> @@ -79,6 +79,7 @@ This edition documents version @value{VERSION}.
>  * Contributing Changes::
>  * Setting up and running test suite::
>  * Updating External Code::
> +* Debugging::
>  * Porting::
>  * Error Handling::
>  * Stack and heap size::
> @@ -595,6 +596,196 @@ cp minilzo-2.10/*.[hc] grub-core/lib/minilzo
>  rm -r minilzo-2.10*
>  @end example
>
> +@node Debugging
> +@chapter Debugging
> +
> +GRUB2 can be difficult to debug because it runs on the bare-metal and thus
> +does not have the debugging facilities normally provided by an operating
> +system. This chapter aims to provide useful information on some ways to
> +debug GRUB2 for some architectures. It by no means intends to be exhaustive.
> +The focus will be one X86_64 and i386 architectures. Luckily for some issues
> +virtual machines have made the ability to debug GRUB2 much easier, and this
> +chapter will focus debugging via the QEMU virtual machine. We will not be
> +going over debugging of the userland tools (eg. grub-install), there are
> +many tutorials on debugging programs in userland.
> +
> +You will need GDB and the QEMU binaries for your system, on Debian these
> +can be installed with the @samp{gdb} and @samp{qemu-system-x86} packages.
> +Also it is assumed that you have already successfully compiled GRUB2 from
> +source for the target specified in the section below and have some
> +familiarity with GDB. When GRUB2 is built it will create many different
> +binaries. The ones of concern will be in the @file{grub-core}
> +directory of the GRUB2 build dir. To aide in debugging we will want the
> +debugging symbols generated during the build because these symbols are not
> +kept in the binaries which get installed to the boot location. The build
> +process outputs two sets of binaries, one without symbols which gets executed
> +at boot, and another set of ELF images with debugging symbols. The built
> +images with debugging symbols will have a @file{.image} suffix, and the ones
> +without a @file{.img} suffix. Similarly, loadable modules with debugging
> +symbols will have a @file{.module} suffix, and ones without a @file{.mod}
> +suffix. In the case of the kernel the binary with symbols is named
> +@file{kernel.exec}.
> +
> +In the following sections, information will be provided on debugging on
> +various targets using @command{gdb} and the @samp{gdb_grub} GDB script.
> +
> +@menu
> +* i386-pc::
> +* x86_64-efi::
> +@end menu
> +
> +@node i386-pc
> +@section i386-pc
> +
> +The i386-pc target is a good place to start when first debugging GRUB2
> +because in some respects its easier than EFI platforms. The reason being
> +that the initial load address is always known in advance. To start
> +debugging GRUB2 first QEMU must be started in GDB stub mode. The following
> +command is a simple illustration:
> +
> +@example
> +qemu-system-i386 -drive file=disk.img,format=raw \
> +    -device virtio-scsi-pci,id=scsi0,num_queues=4 -S -s
> +@end example
> +
> +This will start a QEMU instance booting from @file{disk.img}. It will pause
> +at start waiting for a GDB instance to attach to it. You should change
> +@file{disk.img} to something more appropriate. A block device can be used,
> +but you may need to run QEMU as a privileged user.
> +
> +To connect to this QEMU instance with GDB, the @code{target remote} GDB
> +command must be used. We also need to load a binary image, preferably with
> +symbols. This can be done using the GDB command @code{file kernel.exec}, if
> +GDB is started from the @file{grub-core} directory in the GRUB2 build
> +directory. GRUB2 developers have made this more simple by including a GDB
> +script which does much of the setup. This file at @file{grub-core/gdb_grub}
> +of the build directory and is also installed via @command{make install}.
> +If not building GRUB, the distribution may have a package which installs
> +this GDB script along with debug symbol binaries, such as Debian's
> +@samp{grub-pc-dbg} package. The GDB scripts is intended to by used
> +like so, assuming:
> +
> +@example
> +cd $(dirname /path/to/script/gdb_grub)
> +gdb -x gdb_grub
> +@end example
> +
> +Once GDB has been started with the @file{gdb_grub} script it will
> +automatically connect to the QEMU instance. You can then do things you
> +normally would in GDB like set a break point on @var{grub_main}.
> +
> +Setting breakpoints in modules is trickier since they haven't been loaded
> +yet and are loaded at addresses determined at runtime. The module could be
> +loaded to different addresses in different QEMU instances. The debug symbols
> +in the modules @file{.module} binary, thus are always wrong, and GDB needs
> +to be told where to load the symbols to. But this must happen at runtime
> +after GRUB2 has determined where the module will get loaded. Luckily the
> +@file{gdb_grub} script takes care of this with the @command{runtime_load_module}
> +command, which configures GDB to watch for GRUB2 module loading and when
> +it does add the module symbols with the appropriate offset.
> +
> +@node x86_64-efi
> +@section x86_64-efi
> +
> +Using GDB to debug GRUB2 for the x86_64-efi target has some similarities with
> +the i386-pc target. Please read be familiar with the @ref{x86_64-efi} section
> +when reading this one. Extra care must be used to run QEMU such that it boots
> +a UEFI firmware. This usually involves either using the @samp{-bios} option
> +with a UEFI firmware blob (eg. @file{OVMF.fd}) or loading the firmware via
> +pflash. This document will not go further into how to do this as there are
> +ample resource on the web.
> +
> +Like all EFI implementations, on x86_64-efi the (U)EFI firmware that loads
> +the GRUB2 EFI application determines at runtime where the application will
> +be loaded. This means that we do not know where to tell GDB to load the
> +symbols for the GRUB2 core until the (U)EFI firmware determines it. There
> +two good ways of figuring this out when running in QEMU: use a @ref{OVMF debug log,
> +debug build of OVMF} and check the debug log or have GRUB2 say where it is
> +loaded when it starts. Neither of these are ideal because they both
> +generally give the information after GRUB2 is already running, which makes
> +debugging early boot infeasible. Technically, the first method does give
> +the load address before GRUB2 is run, but without debugging the EFI firmware
> +with symbols, the author currently does not know how to cause the OVMF
> +firmware to pause at that point to use the load address before GRUB2 is run.
> +
> +Even after getting the application load address, the loading of core symbols
> +is complicated by the fact that the debugging symbols for the kernel are in
> +an ELF binary named @file{kernel.exec} while what is in memory are sections
> +for the PE32+ EFI binary. When @command{grub-mkimage} creates the PE32+
> +binary it condenses several segments from the ELF kernel binary into one
> +.data section in the PE32+ binary. This must be taken into account to
> +properly load the other non-text sections. Otherwise, GDB will work as
> +expected when breaking on functions, but, for instance, global variables
> +will point to the wrong address in memory and thus give incorrect values
> +(which can be difficult to debug).
> +
> +The calculating of the correct offsets for sections when loading symbol
> +files are taken care of when loading the kernel symbols via the user-defined
> +GDB command @command{dynamic_load_kernel_exec_symbols}, which takes one
> +argument, the address where the text section is loaded, as determined by
> +one of the methods above. Alternatively, the command @command{dynamic_load_symbols}
> +with the text section address as an agrument can be called to load the
> +kernel symbols and setup loading the module symbols as they are loaded at
> +runtime.
> +
> +In the author's experience, when debugging with QEMU and OVMF, to have
> +debugging symbols loaded at the start of GRUB2 execution the GRUB2 EFI
> +application must be run via QEMU at least once prior in order to get the
> +load address. Two methods for obtaining the load address are described in
> +two subsections below. Generally speaking, the load address does not change
> +between QEMU runs. There are exceptions to this, namely that different
> +GRUB2 EFI Applications can be run at different addresses. Also, its been
> +observed that after running the EFI application for the first time, the
> +second run will many times have a different load address, but subsequent
> +runs of the same EFI application will have the same load address as the
> +second run. This predictability allows us to asume the load address on
> +subsequent runs and thus load the symbols before GRUB2 starts. The following
> +command illustrates this, assuming that QEMU is running and waiting for
> +a debugger connection and the current working directory is where
> +@file{gdb_grub} resides:
> +
> +@example
> +gdb -x gdb_grub -ex 'dynamic_load_symbols @var{load address}'
> +@end example
> +
> +If you load the symbols in this manner and, after continuing execution, do
> +not see output showing the loading of modules symbol, then its very likely
> +that the load address was incorrect.
> +
> +
> +@node OVMF debug log
> +@subsection OVMF debug log
> +
> +In order to get the GRUB2 load address from OVMF, first, a debug build
> +of OVMF must be obtained (@uref{https://github.com/retrage/edk2-nightly/raw/master/bin/DEBUGX64_OVMF.fd,
> +here is one} which is not officially recommended). OVMF will output debug
> +messages to a special serial device, which we must add to QEMU. The following
> +QEMU command will run the debug OVMF and write the debug messages to a
> +file named @file{debug.log}. It is assumed that @file{disk.img} is a disk
> +image or block device that is setup to boot GRUB2 EFI.
> +
> +@example
> +qemu-system-x86_64 -bios /path/to/debug/OVMF.fd \
> +    -drive file=disk.img,format=raw \
> +    -device virtio-scsi-pci,id=scsi0,num_queues=4 \
> +    -debugcon file:debug.log -global isa-debugcon.iobase=0x402
> +@end example
> +
> +If GRUB2 was started by the (U)EFI firmware, then in the @file{debug.log}
> +file one of the last lines should be a log message like:
> +@code{Loading driver at 0x00006AEE000 EntryPoint=0x00006AEE756}. This
> +means that the GRUB2 EFI application was loaded at @code{0x00006AEE000} and
> +its .text section is at @code{0x00006AEE756}.
> +
> +@node Build GRUB2 to print out the load address
> +@subsection Build GRUB2 to print out the load address
> +
> +GRUB2 can be specially built to output the address of its .text section in
> +memory by defining @code{PRINT_GDB_SYM_LOAD_CMD} to @code{1} in @file{config.h.in}
> +before running @command{configure}. The benefit of this method is that it
> +will work on non-virtualized hardware where the (U)EFI firmware may not
> +be modifiable.
> +
>  @node Porting
>  @chapter Porting
>
> --
> 2.34.1
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

I definitely support this.
Last time I tried a lot of time on trying the same thing as Glenn
documented here.
The symbol rule, debug built edk2 with verbose, the explanation of
loading modules are really helpful for the grub newbie such as me!
There was not much information on gdb grub online.
Adding these detailed sections indeed helps more people to contribute grub.
BTW, in my experience, we usually need to use hb instead of b when gdb
grub in qemu.


--
Sincerely,
Jeremy Su


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v4 04/15] gdb: Move runtime module loading into runtime_load_module
  2022-12-16  5:29 ` [PATCH v4 04/15] gdb: Move runtime module loading into runtime_load_module Glenn Washburn
@ 2022-12-21 14:57   ` Daniel Kiper
  0 siblings, 0 replies; 31+ messages in thread
From: Daniel Kiper @ 2022-12-21 14:57 UTC (permalink / raw)
  To: Glenn Washburn; +Cc: grub-devel

On Thu, Dec 15, 2022 at 11:29:27PM -0600, Glenn Washburn wrote:

Why? Could you add that to the commit message?

> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
>  grub-core/gdb_grub.in | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
> index fc17e3d899..d525a5a11f 100644
> --- a/grub-core/gdb_grub.in
> +++ b/grub-core/gdb_grub.in
> @@ -71,16 +71,22 @@ document load_all_modules
>  	Load debugging information for all loaded modules.
>  end
>
> +define runtime_load_module
> +	break grub_dl_add
> +	commands
> +		silent
> +		load_module mod
> +		cont
> +	end
> +end
> +document runtime_load_module
> +	Load module symbols at runtime as they are loaded.
> +end
> +
>  ###
>
>  set confirm off
>  file kernel.exec
>  target remote :1234
>
> -# inform when module is loaded
> -break grub_dl_add
> -commands
> -	silent
> -	load_module mod
> -	cont
> -end
> +runtime_load_module

Daniel


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v4 08/15] gdb: If enabled, print line used to load EFI kernel symbols when using gdb_grub script
  2022-12-16  5:29 ` [PATCH v4 08/15] gdb: If enabled, print line used to load EFI kernel symbols when using gdb_grub script Glenn Washburn
@ 2022-12-21 15:20   ` Daniel Kiper
  2022-12-21 17:57     ` Glenn Washburn
  0 siblings, 1 reply; 31+ messages in thread
From: Daniel Kiper @ 2022-12-21 15:20 UTC (permalink / raw)
  To: Glenn Washburn; +Cc: grub-devel, Peter Jones, Robbie Harwood

Adding Robbie...

Please CC him next time when you post these patches. I would want to
hear his opinion too. Or at least he is aware what is happening here...

On Thu, Dec 15, 2022 at 11:29:31PM -0600, Glenn Washburn wrote:
> If the macro PRINT_GDB_SYM_LOAD_CMD is non-zero, compile code which will
> print the command needed to load symbols for the GRUB EFI kernel. This is
> needed because EFI firmware determines where to load the GRUB EFI at
> runtime, and so the relevant addresses are not known ahead of time.
>
> The command is a custom command defined in the gdb_grub GDB script. So
> GDB should be started with the script as an argument to the -x option or
> sourced into an active GDB session before running the outputted command.

I think this functionality should be disabled when lockdown is enforced,
e.g. on UEFI platforms with Secure Boot enabled.

> Co-developed-by: Peter Jones <pjones@redhat.com>
> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
>  config.h.in               |  3 +++
>  grub-core/kern/efi/efi.c  |  4 ++--
>  grub-core/kern/efi/init.c | 19 ++++++++++++++++++-
>  include/grub/efi/efi.h    |  2 +-
>  4 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/config.h.in b/config.h.in
> index 4d1e50eba7..c31eee1bca 100644
> --- a/config.h.in
> +++ b/config.h.in
> @@ -13,6 +13,9 @@
>  #define MM_DEBUG @MM_DEBUG@
>  #endif
>
> +/* Define to 1 to enable printing of gdb command to load module symbols.  */
> +#define PRINT_GDB_SYM_LOAD_CMD 0
> +
>  /* Define to 1 to enable disk cache statistics.  */
>  #define DISK_CACHE_STATS @DISK_CACHE_STATS@
>  #define BOOT_TIME_STATS @BOOT_TIME_STATS@
> diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
> index cf49d6357e..17bd06f7e6 100644
> --- a/grub-core/kern/efi/efi.c
> +++ b/grub-core/kern/efi/efi.c
> @@ -299,7 +299,7 @@ grub_efi_get_variable (const char *var, const grub_efi_guid_t *guid,
>  /* Search the mods section from the PE32/PE32+ image. This code uses
>     a PE32 header, but should work with PE32+ as well.  */
>  grub_addr_t
> -grub_efi_modules_addr (void)
> +grub_efi_section_addr (const char *section_name)
>  {
>    grub_efi_loaded_image_t *image;
>    struct grub_msdos_image_header *header;
> @@ -328,7 +328,7 @@ grub_efi_modules_addr (void)
>         i < coff_header->num_sections;
>         i++, section++)
>      {
> -      if (grub_strcmp (section->name, "mods") == 0)
> +      if (grub_strcmp (section->name, section_name) == 0)
>  	break;
>      }
>
> diff --git a/grub-core/kern/efi/init.c b/grub-core/kern/efi/init.c
> index b67bc73a1b..ed3d463c8f 100644
> --- a/grub-core/kern/efi/init.c
> +++ b/grub-core/kern/efi/init.c
> @@ -101,10 +101,24 @@ stack_protector_init (void)
>
>  grub_addr_t grub_modbase;
>
> +#if PRINT_GDB_SYM_LOAD_CMD
> +static void
> +grub_efi_print_gdb_info (void)
> +{
> +  grub_addr_t text;
> +
> +  text = grub_efi_section_addr (".text");
> +  if (!text)
> +    return;
> +
> +  grub_printf ("dynamic_load_symbols %p\n", (void *)text);
> +}
> +#endif
> +
>  void
>  grub_efi_init (void)
>  {
> -  grub_modbase = grub_efi_modules_addr ();
> +  grub_modbase = grub_efi_section_addr ("mods");
>    /* First of all, initialize the console so that GRUB can display
>       messages.  */
>    grub_console_init ();
> @@ -127,6 +141,9 @@ grub_efi_init (void)
>    efi_call_4 (grub_efi_system_table->boot_services->set_watchdog_timer,
>  	      0, 0, 0, NULL);
>
> +#if PRINT_GDB_SYM_LOAD_CMD
> +  grub_efi_print_gdb_info ();
> +#endif
>    grub_efidisk_init ();
>  }
>
> diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
> index e61272de53..586ac856b5 100644
> --- a/include/grub/efi/efi.h
> +++ b/include/grub/efi/efi.h
> @@ -109,7 +109,7 @@ grub_err_t grub_arch_efi_linux_boot_image(grub_addr_t addr, grub_size_t size,
>                                             char *args);
>  #endif
>
> -grub_addr_t grub_efi_modules_addr (void);
> +grub_addr_t grub_efi_section_addr (const char *section);
>
>  void grub_efi_mm_init (void);
>  void grub_efi_mm_fini (void);

Daniel


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v4 12/15] gdb: Allow running user-defined commands at GRUB start
  2022-12-16  5:29 ` [PATCH v4 12/15] gdb: Allow running user-defined commands at GRUB start Glenn Washburn
@ 2022-12-21 15:27   ` Daniel Kiper
  2022-12-21 18:19     ` Glenn Washburn
  0 siblings, 1 reply; 31+ messages in thread
From: Daniel Kiper @ 2022-12-21 15:27 UTC (permalink / raw)
  To: Glenn Washburn; +Cc: grub-devel

On Thu, Dec 15, 2022 at 11:29:35PM -0600, Glenn Washburn wrote:
> A new command, run_on_start, is created which handles some complexities
> of the EFI platform when breaking on GRUB start. If GRUB start is hooked,
> run "onstart" command if it is defned.
>
> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
>  grub-core/gdb_grub.in | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
> index 8ae6344edf..3b3cea1a4d 100644
> --- a/grub-core/gdb_grub.in
> +++ b/grub-core/gdb_grub.in
> @@ -36,6 +36,8 @@ end
>  define dynamic_load_symbols
>  	dynamic_load_kernel_exec_symbols $arg0
>
> +	run_on_start
> +
>  	# We may have been very late to loading the kernel.exec symbols and
>  	# and modules may already be loaded. So load symbols for any already
>  	# loaded.
> @@ -134,6 +136,41 @@ document runtime_load_module
>  	Load module symbols at runtime as they are loaded.
>  end
>
> +define run_on_start
> +	# TODO: Add check to see if _start symbol is defined, if not, then
> +	# the symbols have not yet been loaded and this command will not work.
> +	watch *_start
> +	set $break_efi_start_bpnum = $bpnum
> +	commands
> +		silent
> +		delete $break_efi_start_bpnum
> +		break _start

s/break/hbreak/?

> +		commands
> +			silent
> +			delete $break_efi_start_bpnum
> +			set $onstart_name = "onstart"
> +			is_user_command $onstart_name
> +			if $ret
> +				onstart
> +			end
> +			continue
> +		end
> +		set $break_efi_start_bpnum = $bpnum
> +		continue
> +	end
> +end
> +document run_on_start
> +	On some targets, such as x86_64-efi, even if you know where the
> +	firmware will load the grub image, you can not simply set a break

Nit, s/grub/GRUB/...

> +	point before the image is loaded because loading the image
> +	overwrites the break point in memory. So setup a hardware watch
> +	point, which does not have that problem, and if that gets triggered,
> +	then reset the break point. If a user-defined command named
> +	"onstart" exists it will be run after the start is hit.
> +	NOTE: This assumes symbols have already been correctly loaded for
> +	the EFI application.
> +end
> +
>  ###
>
>  set confirm off
> @@ -151,6 +188,7 @@ if ! $runonce
>  		exec-file kernel.exec
>  	else
>  		file kernel.exec
> +		run_on_start
>  		runtime_load_module
>  	end

Daniel


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v4 13/15] gdb: Add extra early initialization symbols for i386-pc
  2022-12-16  5:29 ` [PATCH v4 13/15] gdb: Add extra early initialization symbols for i386-pc Glenn Washburn
@ 2022-12-21 15:28   ` Daniel Kiper
  2022-12-21 18:21     ` Glenn Washburn
  0 siblings, 1 reply; 31+ messages in thread
From: Daniel Kiper @ 2022-12-21 15:28 UTC (permalink / raw)
  To: Glenn Washburn; +Cc: grub-devel

On Thu, Dec 15, 2022 at 11:29:36PM -0600, Glenn Washburn wrote:
> Add symbols for boot.image, disk.image, and lzma_decompress.image if the
> target is i386-pc.

Why?

> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
>  grub-core/gdb_grub.in | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
> index 3b3cea1a4d..a9c9d00430 100644
> --- a/grub-core/gdb_grub.in
> +++ b/grub-core/gdb_grub.in
> @@ -181,12 +181,18 @@ set confirm off
>  # fail.
>
>  set $platform_efi = $_streq("@platform@", "efi")
> +set $target = "@target_cpu@-@platform@"
>
>  if ! $runonce
>  	if $platform_efi
>  		# Only load the executable file, not the symbols
>  		exec-file kernel.exec
>  	else
> +		if $_streq($target, "i386-pc")
> +			add-symbol-file boot.image
> +			add-symbol-file diskboot.image
> +			add-symbol-file lzma_decompress.image
> +		end
>  		file kernel.exec
>  		run_on_start
>  		runtime_load_module

Daniel


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v4 15/15] docs: Add debugging chapter to development documentation
  2022-12-16  5:29 ` [PATCH v4 15/15] docs: Add debugging chapter to development documentation Glenn Washburn
  2022-12-21  3:07   ` Jeremy Szu
@ 2022-12-21 15:50   ` Daniel Kiper
  1 sibling, 0 replies; 31+ messages in thread
From: Daniel Kiper @ 2022-12-21 15:50 UTC (permalink / raw)
  To: Glenn Washburn; +Cc: grub-devel

On Thu, Dec 15, 2022 at 11:29:38PM -0600, Glenn Washburn wrote:
> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
>  docs/grub-dev.texi | 191 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 191 insertions(+)
>
> diff --git a/docs/grub-dev.texi b/docs/grub-dev.texi
> index f76fc658bf..8171e91c33 100644
> --- a/docs/grub-dev.texi
> +++ b/docs/grub-dev.texi
> @@ -79,6 +79,7 @@ This edition documents version @value{VERSION}.
>  * Contributing Changes::
>  * Setting up and running test suite::
>  * Updating External Code::
> +* Debugging::
>  * Porting::
>  * Error Handling::
>  * Stack and heap size::
> @@ -595,6 +596,196 @@ cp minilzo-2.10/*.[hc] grub-core/lib/minilzo
>  rm -r minilzo-2.10*
>  @end example
>
> +@node Debugging
> +@chapter Debugging
> +
> +GRUB2 can be difficult to debug because it runs on the bare-metal and thus
> +does not have the debugging facilities normally provided by an operating
> +system. This chapter aims to provide useful information on some ways to
> +debug GRUB2 for some architectures. It by no means intends to be exhaustive.
> +The focus will be one X86_64 and i386 architectures. Luckily for some issues

s/X86_64/x86_64/

> +virtual machines have made the ability to debug GRUB2 much easier, and this
> +chapter will focus debugging via the QEMU virtual machine. We will not be
> +going over debugging of the userland tools (eg. grub-install), there are
> +many tutorials on debugging programs in userland.
> +
> +You will need GDB and the QEMU binaries for your system, on Debian these
> +can be installed with the @samp{gdb} and @samp{qemu-system-x86} packages.
> +Also it is assumed that you have already successfully compiled GRUB2 from
> +source for the target specified in the section below and have some
> +familiarity with GDB. When GRUB2 is built it will create many different
> +binaries. The ones of concern will be in the @file{grub-core}
> +directory of the GRUB2 build dir. To aide in debugging we will want the
> +debugging symbols generated during the build because these symbols are not
> +kept in the binaries which get installed to the boot location. The build
> +process outputs two sets of binaries, one without symbols which gets executed
> +at boot, and another set of ELF images with debugging symbols. The built
> +images with debugging symbols will have a @file{.image} suffix, and the ones
> +without a @file{.img} suffix. Similarly, loadable modules with debugging
> +symbols will have a @file{.module} suffix, and ones without a @file{.mod}
> +suffix. In the case of the kernel the binary with symbols is named
> +@file{kernel.exec}.
> +
> +In the following sections, information will be provided on debugging on
> +various targets using @command{gdb} and the @samp{gdb_grub} GDB script.
> +
> +@menu
> +* i386-pc::
> +* x86_64-efi::
> +@end menu
> +
> +@node i386-pc
> +@section i386-pc
> +
> +The i386-pc target is a good place to start when first debugging GRUB2
> +because in some respects its easier than EFI platforms. The reason being
> +that the initial load address is always known in advance. To start
> +debugging GRUB2 first QEMU must be started in GDB stub mode. The following
> +command is a simple illustration:
> +
> +@example
> +qemu-system-i386 -drive file=disk.img,format=raw \
> +    -device virtio-scsi-pci,id=scsi0,num_queues=4 -S -s

I would drop "num_queues=4" as non-essential thing here. And num_queues
seems deprecated today...

> +@end example
> +
> +This will start a QEMU instance booting from @file{disk.img}. It will pause
> +at start waiting for a GDB instance to attach to it. You should change
> +@file{disk.img} to something more appropriate. A block device can be used,
> +but you may need to run QEMU as a privileged user.
> +
> +To connect to this QEMU instance with GDB, the @code{target remote} GDB
> +command must be used. We also need to load a binary image, preferably with
> +symbols. This can be done using the GDB command @code{file kernel.exec}, if
> +GDB is started from the @file{grub-core} directory in the GRUB2 build
> +directory. GRUB2 developers have made this more simple by including a GDB
> +script which does much of the setup. This file at @file{grub-core/gdb_grub}
> +of the build directory and is also installed via @command{make install}.
> +If not building GRUB, the distribution may have a package which installs
> +this GDB script along with debug symbol binaries, such as Debian's
> +@samp{grub-pc-dbg} package. The GDB scripts is intended to by used
> +like so, assuming:
> +
> +@example
> +cd $(dirname /path/to/script/gdb_grub)
> +gdb -x gdb_grub
> +@end example
> +
> +Once GDB has been started with the @file{gdb_grub} script it will
> +automatically connect to the QEMU instance. You can then do things you
> +normally would in GDB like set a break point on @var{grub_main}.
> +
> +Setting breakpoints in modules is trickier since they haven't been loaded
> +yet and are loaded at addresses determined at runtime. The module could be
> +loaded to different addresses in different QEMU instances. The debug symbols
> +in the modules @file{.module} binary, thus are always wrong, and GDB needs
> +to be told where to load the symbols to. But this must happen at runtime
> +after GRUB2 has determined where the module will get loaded. Luckily the
> +@file{gdb_grub} script takes care of this with the @command{runtime_load_module}
> +command, which configures GDB to watch for GRUB2 module loading and when
> +it does add the module symbols with the appropriate offset.
> +
> +@node x86_64-efi
> +@section x86_64-efi
> +
> +Using GDB to debug GRUB2 for the x86_64-efi target has some similarities with
> +the i386-pc target. Please read be familiar with the @ref{x86_64-efi} section
> +when reading this one. Extra care must be used to run QEMU such that it boots
> +a UEFI firmware. This usually involves either using the @samp{-bios} option
> +with a UEFI firmware blob (eg. @file{OVMF.fd}) or loading the firmware via
> +pflash. This document will not go further into how to do this as there are
> +ample resource on the web.
> +
> +Like all EFI implementations, on x86_64-efi the (U)EFI firmware that loads
> +the GRUB2 EFI application determines at runtime where the application will
> +be loaded. This means that we do not know where to tell GDB to load the
> +symbols for the GRUB2 core until the (U)EFI firmware determines it. There

s/There/There are/?

> +two good ways of figuring this out when running in QEMU: use a @ref{OVMF debug log,
> +debug build of OVMF} and check the debug log or have GRUB2 say where it is
> +loaded when it starts. Neither of these are ideal because they both
> +generally give the information after GRUB2 is already running, which makes
> +debugging early boot infeasible. Technically, the first method does give
> +the load address before GRUB2 is run, but without debugging the EFI firmware
> +with symbols, the author currently does not know how to cause the OVMF
> +firmware to pause at that point to use the load address before GRUB2 is run.
> +
> +Even after getting the application load address, the loading of core symbols
> +is complicated by the fact that the debugging symbols for the kernel are in
> +an ELF binary named @file{kernel.exec} while what is in memory are sections
> +for the PE32+ EFI binary. When @command{grub-mkimage} creates the PE32+
> +binary it condenses several segments from the ELF kernel binary into one
> +.data section in the PE32+ binary. This must be taken into account to
> +properly load the other non-text sections. Otherwise, GDB will work as
> +expected when breaking on functions, but, for instance, global variables
> +will point to the wrong address in memory and thus give incorrect values
> +(which can be difficult to debug).
> +
> +The calculating of the correct offsets for sections when loading symbol
> +files are taken care of when loading the kernel symbols via the user-defined
> +GDB command @command{dynamic_load_kernel_exec_symbols}, which takes one
> +argument, the address where the text section is loaded, as determined by
> +one of the methods above. Alternatively, the command @command{dynamic_load_symbols}
> +with the text section address as an agrument can be called to load the
> +kernel symbols and setup loading the module symbols as they are loaded at
> +runtime.
> +
> +In the author's experience, when debugging with QEMU and OVMF, to have
> +debugging symbols loaded at the start of GRUB2 execution the GRUB2 EFI
> +application must be run via QEMU at least once prior in order to get the
> +load address. Two methods for obtaining the load address are described in
> +two subsections below. Generally speaking, the load address does not change
> +between QEMU runs. There are exceptions to this, namely that different
> +GRUB2 EFI Applications can be run at different addresses. Also, its been
> +observed that after running the EFI application for the first time, the
> +second run will many times have a different load address, but subsequent
> +runs of the same EFI application will have the same load address as the
> +second run. This predictability allows us to asume the load address on
> +subsequent runs and thus load the symbols before GRUB2 starts. The following
> +command illustrates this, assuming that QEMU is running and waiting for
> +a debugger connection and the current working directory is where
> +@file{gdb_grub} resides:
> +
> +@example
> +gdb -x gdb_grub -ex 'dynamic_load_symbols @var{load address}'
> +@end example
> +
> +If you load the symbols in this manner and, after continuing execution, do
> +not see output showing the loading of modules symbol, then its very likely
> +that the load address was incorrect.
> +
> +
> +@node OVMF debug log
> +@subsection OVMF debug log
> +
> +In order to get the GRUB2 load address from OVMF, first, a debug build
> +of OVMF must be obtained (@uref{https://github.com/retrage/edk2-nightly/raw/master/bin/DEBUGX64_OVMF.fd,
> +here is one} which is not officially recommended). OVMF will output debug
> +messages to a special serial device, which we must add to QEMU. The following
> +QEMU command will run the debug OVMF and write the debug messages to a
> +file named @file{debug.log}. It is assumed that @file{disk.img} is a disk
> +image or block device that is setup to boot GRUB2 EFI.
> +
> +@example
> +qemu-system-x86_64 -bios /path/to/debug/OVMF.fd \
> +    -drive file=disk.img,format=raw \
> +    -device virtio-scsi-pci,id=scsi0,num_queues=4 \

Please drop num_queues here too...

> +    -debugcon file:debug.log -global isa-debugcon.iobase=0x402
> +@end example
> +
> +If GRUB2 was started by the (U)EFI firmware, then in the @file{debug.log}
> +file one of the last lines should be a log message like:
> +@code{Loading driver at 0x00006AEE000 EntryPoint=0x00006AEE756}. This
> +means that the GRUB2 EFI application was loaded at @code{0x00006AEE000} and
> +its .text section is at @code{0x00006AEE756}.
> +
> +@node Build GRUB2 to print out the load address
> +@subsection Build GRUB2 to print out the load address
> +
> +GRUB2 can be specially built to output the address of its .text section in
> +memory by defining @code{PRINT_GDB_SYM_LOAD_CMD} to @code{1} in @file{config.h.in}
> +before running @command{configure}. The benefit of this method is that it
> +will work on non-virtualized hardware where the (U)EFI firmware may not
> +be modifiable.
> +
>  @node Porting
>  @chapter Porting

Additionally, as Jeremy Szu pointed out in the other email it would be
beneficial at least shortly mention differences between "break"/"b" and
"hbreak"/"hb" GDB commands.

Otherwise +/- some minor things patches LGTM...

Thank you for doing this work!

Daniel


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v4 08/15] gdb: If enabled, print line used to load EFI kernel symbols when using gdb_grub script
  2022-12-21 15:20   ` Daniel Kiper
@ 2022-12-21 17:57     ` Glenn Washburn
  2022-12-22 18:17       ` Daniel Kiper
  0 siblings, 1 reply; 31+ messages in thread
From: Glenn Washburn @ 2022-12-21 17:57 UTC (permalink / raw)
  To: Daniel Kiper; +Cc: grub-devel, Peter Jones, Robbie Harwood

On Wed, 21 Dec 2022 16:20:17 +0100
Daniel Kiper <dkiper@net-space.pl> wrote:

> Adding Robbie...
> 
> Please CC him next time when you post these patches. I would want to
> hear his opinion too. Or at least he is aware what is happening
> here...

Sure, I CC'd him and Peter on the first couple of ones. But there was
never had a response in the 4 months since then, so I figured they
didn't care.

> 
> On Thu, Dec 15, 2022 at 11:29:31PM -0600, Glenn Washburn wrote:
> > If the macro PRINT_GDB_SYM_LOAD_CMD is non-zero, compile code which
> > will print the command needed to load symbols for the GRUB EFI
> > kernel. This is needed because EFI firmware determines where to
> > load the GRUB EFI at runtime, and so the relevant addresses are not
> > known ahead of time.
> >
> > The command is a custom command defined in the gdb_grub GDB script.
> > So GDB should be started with the script as an argument to the -x
> > option or sourced into an active GDB session before running the
> > outputted command.
> 
> I think this functionality should be disabled when lockdown is
> enforced, e.g. on UEFI platforms with Secure Boot enabled.

Since this is off by default and must be enabled at build time, then if
the builder enabled it, they really did want it, regardless of
lockdown. What you're worried about seems highly improbable to me (but
then I don't know the inner workings of the distros). The concern as I
understand it, is that someone doing an official release of a distro
which will be secure boot ready will accidentally have this build time
macro enabled. That's almost inconceivable to me, but I'm curious what
the others have to say (especially since Robbie posted a similar patch
that always printed this info as a debug message[1]). Or is it more
about a regular user signing with their own keys accidentally shooting
themselves in the foot by forgetting to disable this (after having
already enabled it) and then some physical attacker getting extra info
to do an evil maid attack?

Glenn

[1] https://lists.gnu.org/archive/html/grub-devel/2021-11/msg00008.html

> 
> > Co-developed-by: Peter Jones <pjones@redhat.com>
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > ---
> >  config.h.in               |  3 +++
> >  grub-core/kern/efi/efi.c  |  4 ++--
> >  grub-core/kern/efi/init.c | 19 ++++++++++++++++++-
> >  include/grub/efi/efi.h    |  2 +-
> >  4 files changed, 24 insertions(+), 4 deletions(-)
> >
> > diff --git a/config.h.in b/config.h.in
> > index 4d1e50eba7..c31eee1bca 100644
> > --- a/config.h.in
> > +++ b/config.h.in
> > @@ -13,6 +13,9 @@
> >  #define MM_DEBUG @MM_DEBUG@
> >  #endif
> >
> > +/* Define to 1 to enable printing of gdb command to load module
> > symbols.  */ +#define PRINT_GDB_SYM_LOAD_CMD 0
> > +
> >  /* Define to 1 to enable disk cache statistics.  */
> >  #define DISK_CACHE_STATS @DISK_CACHE_STATS@
> >  #define BOOT_TIME_STATS @BOOT_TIME_STATS@
> > diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
> > index cf49d6357e..17bd06f7e6 100644
> > --- a/grub-core/kern/efi/efi.c
> > +++ b/grub-core/kern/efi/efi.c
> > @@ -299,7 +299,7 @@ grub_efi_get_variable (const char *var, const
> > grub_efi_guid_t *guid, /* Search the mods section from the
> > PE32/PE32+ image. This code uses a PE32 header, but should work
> > with PE32+ as well.  */ grub_addr_t
> > -grub_efi_modules_addr (void)
> > +grub_efi_section_addr (const char *section_name)
> >  {
> >    grub_efi_loaded_image_t *image;
> >    struct grub_msdos_image_header *header;
> > @@ -328,7 +328,7 @@ grub_efi_modules_addr (void)
> >         i < coff_header->num_sections;
> >         i++, section++)
> >      {
> > -      if (grub_strcmp (section->name, "mods") == 0)
> > +      if (grub_strcmp (section->name, section_name) == 0)
> >  	break;
> >      }
> >
> > diff --git a/grub-core/kern/efi/init.c b/grub-core/kern/efi/init.c
> > index b67bc73a1b..ed3d463c8f 100644
> > --- a/grub-core/kern/efi/init.c
> > +++ b/grub-core/kern/efi/init.c
> > @@ -101,10 +101,24 @@ stack_protector_init (void)
> >
> >  grub_addr_t grub_modbase;
> >
> > +#if PRINT_GDB_SYM_LOAD_CMD
> > +static void
> > +grub_efi_print_gdb_info (void)
> > +{
> > +  grub_addr_t text;
> > +
> > +  text = grub_efi_section_addr (".text");
> > +  if (!text)
> > +    return;
> > +
> > +  grub_printf ("dynamic_load_symbols %p\n", (void *)text);
> > +}
> > +#endif
> > +
> >  void
> >  grub_efi_init (void)
> >  {
> > -  grub_modbase = grub_efi_modules_addr ();
> > +  grub_modbase = grub_efi_section_addr ("mods");
> >    /* First of all, initialize the console so that GRUB can display
> >       messages.  */
> >    grub_console_init ();
> > @@ -127,6 +141,9 @@ grub_efi_init (void)
> >    efi_call_4
> > (grub_efi_system_table->boot_services->set_watchdog_timer, 0, 0, 0,
> > NULL);
> >
> > +#if PRINT_GDB_SYM_LOAD_CMD
> > +  grub_efi_print_gdb_info ();
> > +#endif
> >    grub_efidisk_init ();
> >  }
> >
> > diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
> > index e61272de53..586ac856b5 100644
> > --- a/include/grub/efi/efi.h
> > +++ b/include/grub/efi/efi.h
> > @@ -109,7 +109,7 @@ grub_err_t
> > grub_arch_efi_linux_boot_image(grub_addr_t addr, grub_size_t size,
> > char *args); #endif
> >
> > -grub_addr_t grub_efi_modules_addr (void);
> > +grub_addr_t grub_efi_section_addr (const char *section);
> >
> >  void grub_efi_mm_init (void);
> >  void grub_efi_mm_fini (void);
> 
> Daniel


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v4 12/15] gdb: Allow running user-defined commands at GRUB start
  2022-12-21 15:27   ` Daniel Kiper
@ 2022-12-21 18:19     ` Glenn Washburn
  2022-12-22  6:08       ` Glenn Washburn
  0 siblings, 1 reply; 31+ messages in thread
From: Glenn Washburn @ 2022-12-21 18:19 UTC (permalink / raw)
  To: Daniel Kiper; +Cc: grub-devel

On Wed, 21 Dec 2022 16:27:40 +0100
Daniel Kiper <dkiper@net-space.pl> wrote:

> On Thu, Dec 15, 2022 at 11:29:35PM -0600, Glenn Washburn wrote:
> > A new command, run_on_start, is created which handles some
> > complexities of the EFI platform when breaking on GRUB start. If
> > GRUB start is hooked, run "onstart" command if it is defned.
> >
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > ---
> >  grub-core/gdb_grub.in | 38 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> >
> > diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
> > index 8ae6344edf..3b3cea1a4d 100644
> > --- a/grub-core/gdb_grub.in
> > +++ b/grub-core/gdb_grub.in
> > @@ -36,6 +36,8 @@ end
> >  define dynamic_load_symbols
> >  	dynamic_load_kernel_exec_symbols $arg0
> >
> > +	run_on_start
> > +
> >  	# We may have been very late to loading the kernel.exec
> > symbols and # and modules may already be loaded. So load symbols
> > for any already # loaded.
> > @@ -134,6 +136,41 @@ document runtime_load_module
> >  	Load module symbols at runtime as they are loaded.
> >  end
> >
> > +define run_on_start
> > +	# TODO: Add check to see if _start symbol is defined, if
> > not, then
> > +	# the symbols have not yet been loaded and this command
> > will not work.
> > +	watch *_start
> > +	set $break_efi_start_bpnum = $bpnum
> > +	commands
> > +		silent
> > +		delete $break_efi_start_bpnum
> > +		break _start
> 
> s/break/hbreak/?

A regular break works here for me. I want to avoid hbreak at all costs,
which is why I had a previous convoluted method using break (which I
thought worked, and then found it didn't quite). My understanding is
that the number of hardware breakpoints are limited and commonly its
around 4. Specifically, my understanding is that on x86-64 the number
is exactly 4, so I would prefer the user have usable as many as
possible.

Really, I'd like to figure out why sometimes break works and why
sometimes not, and then figure out a way to make it work for these
scripts. I recently had the idea that maybe the UEFI firmware sets up
the pages where it loads the .text section of the GRUB UEFI binary to
readonly in the page table structure. But I went through the structure
when %eip is at _start and the R/W bit is set on the pages I checked.
Even if the pages were set to readonly, I suspect the qemu gdb stub
allows writing to that memory anyway.

So I'm at a loss as to what could be preventing break from working. I'd
love to hear some ideas if anyone has some.

Glenn

> > +		commands
> > +			silent
> > +			delete $break_efi_start_bpnum
> > +			set $onstart_name = "onstart"
> > +			is_user_command $onstart_name
> > +			if $ret
> > +				onstart
> > +			end
> > +			continue
> > +		end
> > +		set $break_efi_start_bpnum = $bpnum
> > +		continue
> > +	end
> > +end
> > +document run_on_start
> > +	On some targets, such as x86_64-efi, even if you know
> > where the
> > +	firmware will load the grub image, you can not simply set
> > a break
> 
> Nit, s/grub/GRUB/...
> 
> > +	point before the image is loaded because loading the image
> > +	overwrites the break point in memory. So setup a hardware
> > watch
> > +	point, which does not have that problem, and if that gets
> > triggered,
> > +	then reset the break point. If a user-defined command named
> > +	"onstart" exists it will be run after the start is hit.
> > +	NOTE: This assumes symbols have already been correctly
> > loaded for
> > +	the EFI application.
> > +end
> > +
> >  ###
> >
> >  set confirm off
> > @@ -151,6 +188,7 @@ if ! $runonce
> >  		exec-file kernel.exec
> >  	else
> >  		file kernel.exec
> > +		run_on_start
> >  		runtime_load_module
> >  	end
> 
> Daniel


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v4 13/15] gdb: Add extra early initialization symbols for i386-pc
  2022-12-21 15:28   ` Daniel Kiper
@ 2022-12-21 18:21     ` Glenn Washburn
  2022-12-22 18:25       ` Daniel Kiper
  0 siblings, 1 reply; 31+ messages in thread
From: Glenn Washburn @ 2022-12-21 18:21 UTC (permalink / raw)
  To: Daniel Kiper; +Cc: grub-devel

On Wed, 21 Dec 2022 16:28:35 +0100
Daniel Kiper <dkiper@net-space.pl> wrote:

> On Thu, Dec 15, 2022 at 11:29:36PM -0600, Glenn Washburn wrote:
> > Add symbols for boot.image, disk.image, and lzma_decompress.image
> > if the target is i386-pc.
> 
> Why?

Why not? I'm not clear on what your objection is. This is for debugging
the code that has those symbols, eg. to break on those symbols.

Glenn

> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > ---
> >  grub-core/gdb_grub.in | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
> > index 3b3cea1a4d..a9c9d00430 100644
> > --- a/grub-core/gdb_grub.in
> > +++ b/grub-core/gdb_grub.in
> > @@ -181,12 +181,18 @@ set confirm off
> >  # fail.
> >
> >  set $platform_efi = $_streq("@platform@", "efi")
> > +set $target = "@target_cpu@-@platform@"
> >
> >  if ! $runonce
> >  	if $platform_efi
> >  		# Only load the executable file, not the symbols
> >  		exec-file kernel.exec
> >  	else
> > +		if $_streq($target, "i386-pc")
> > +			add-symbol-file boot.image
> > +			add-symbol-file diskboot.image
> > +			add-symbol-file lzma_decompress.image
> > +		end
> >  		file kernel.exec
> >  		run_on_start
> >  		runtime_load_module
> 
> Daniel


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v4 12/15] gdb: Allow running user-defined commands at GRUB start
  2022-12-21 18:19     ` Glenn Washburn
@ 2022-12-22  6:08       ` Glenn Washburn
  2022-12-22 18:21         ` Daniel Kiper
  0 siblings, 1 reply; 31+ messages in thread
From: Glenn Washburn @ 2022-12-22  6:08 UTC (permalink / raw)
  To: Daniel Kiper; +Cc: Jeremy Szu, grub-devel

On Wed, 21 Dec 2022 12:19:16 -0600
Glenn Washburn <development@efficientek.com> wrote:

> On Wed, 21 Dec 2022 16:27:40 +0100
> Daniel Kiper <dkiper@net-space.pl> wrote:
> 
> > On Thu, Dec 15, 2022 at 11:29:35PM -0600, Glenn Washburn wrote:
> > > A new command, run_on_start, is created which handles some
> > > complexities of the EFI platform when breaking on GRUB start. If
> > > GRUB start is hooked, run "onstart" command if it is defned.
> > >
> > > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > > ---
> > >  grub-core/gdb_grub.in | 38 ++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 38 insertions(+)
> > >
> > > diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
> > > index 8ae6344edf..3b3cea1a4d 100644
> > > --- a/grub-core/gdb_grub.in
> > > +++ b/grub-core/gdb_grub.in
> > > @@ -36,6 +36,8 @@ end
> > >  define dynamic_load_symbols
> > >  	dynamic_load_kernel_exec_symbols $arg0
> > >
> > > +	run_on_start
> > > +
> > >  	# We may have been very late to loading the kernel.exec
> > > symbols and # and modules may already be loaded. So load symbols
> > > for any already # loaded.
> > > @@ -134,6 +136,41 @@ document runtime_load_module
> > >  	Load module symbols at runtime as they are loaded.
> > >  end
> > >
> > > +define run_on_start
> > > +	# TODO: Add check to see if _start symbol is defined, if
> > > not, then
> > > +	# the symbols have not yet been loaded and this command
> > > will not work.
> > > +	watch *_start
> > > +	set $break_efi_start_bpnum = $bpnum
> > > +	commands
> > > +		silent
> > > +		delete $break_efi_start_bpnum
> > > +		break _start
> > 
> > s/break/hbreak/?
> 
> A regular break works here for me. I want to avoid hbreak at all
> costs, which is why I had a previous convoluted method using break
> (which I thought worked, and then found it didn't quite). My
> understanding is that the number of hardware breakpoints are limited
> and commonly its around 4. Specifically, my understanding is that on
> x86-64 the number is exactly 4, so I would prefer the user have
> usable as many as possible.
> 
> Really, I'd like to figure out why sometimes break works and why
> sometimes not, and then figure out a way to make it work for these
> scripts. I recently had the idea that maybe the UEFI firmware sets up
> the pages where it loads the .text section of the GRUB UEFI binary to
> readonly in the page table structure. But I went through the structure
> when %eip is at _start and the R/W bit is set on the pages I checked.
> Even if the pages were set to readonly, I suspect the qemu gdb stub
> allows writing to that memory anyway.
> 
> So I'm at a loss as to what could be preventing break from working.
> I'd love to hear some ideas if anyone has some.

Okay so its been a while since I was deep in this and I forgot some
stuff I already knew (and which I should incorporate into the
documentation patch). The reason that software break doesn't always
work, is that when its not working its because the breakpoint is being
set before the GRUB EFI app is loaded into memory. So happens is that
GDB sets a breakpoint (ie a 0xcc instruction where the symbol points
to), but it then gets over-written by the UEFI firmware as its loads
the GRUB EFI app. So the app loading effectively will clear all
software breakpoints. This doesn't affect the hardware breakpoints
because they are in CPU debug registers which are not affected by the
firmware loading the EFI app.

The reason that the above worked, and was written that way in the first
place, is that when the watch commands run the memory at _start has just
been modified. This happens when the UEFI firmware is loading the GRUB
EFI app. Then we can use software break without worrying about it being
cleared.

Revisiting this has given me some ideas for improving the patch series.

Glenn

> 
> > > +		commands
> > > +			silent
> > > +			delete $break_efi_start_bpnum
> > > +			set $onstart_name = "onstart"
> > > +			is_user_command $onstart_name
> > > +			if $ret
> > > +				onstart
> > > +			end
> > > +			continue
> > > +		end
> > > +		set $break_efi_start_bpnum = $bpnum
> > > +		continue
> > > +	end
> > > +end
> > > +document run_on_start
> > > +	On some targets, such as x86_64-efi, even if you know
> > > where the
> > > +	firmware will load the grub image, you can not simply set
> > > a break
> > 
> > Nit, s/grub/GRUB/...
> > 
> > > +	point before the image is loaded because loading the
> > > image
> > > +	overwrites the break point in memory. So setup a hardware
> > > watch
> > > +	point, which does not have that problem, and if that gets
> > > triggered,
> > > +	then reset the break point. If a user-defined command
> > > named
> > > +	"onstart" exists it will be run after the start is hit.
> > > +	NOTE: This assumes symbols have already been correctly
> > > loaded for
> > > +	the EFI application.
> > > +end
> > > +
> > >  ###
> > >
> > >  set confirm off
> > > @@ -151,6 +188,7 @@ if ! $runonce
> > >  		exec-file kernel.exec
> > >  	else
> > >  		file kernel.exec
> > > +		run_on_start
> > >  		runtime_load_module
> > >  	end
> > 
> > Daniel


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v4 15/15] docs: Add debugging chapter to development documentation
  2022-12-21  3:07   ` Jeremy Szu
@ 2022-12-22  6:12     ` Glenn Washburn
  0 siblings, 0 replies; 31+ messages in thread
From: Glenn Washburn @ 2022-12-22  6:12 UTC (permalink / raw)
  To: Jeremy Szu; +Cc: The development of GNU GRUB, Daniel Kiper

On Wed, 21 Dec 2022 11:07:38 +0800
Jeremy Szu <jeremy.szu@canonical.com> wrote:

> On Fri, Dec 16, 2022 at 1:33 PM Glenn Washburn
> <development@efficientek.com> wrote:
> >
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > ---
> >  docs/grub-dev.texi | 191
> > +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 191
> > insertions(+)
> >
> > diff --git a/docs/grub-dev.texi b/docs/grub-dev.texi
> > index f76fc658bf..8171e91c33 100644
> > --- a/docs/grub-dev.texi
> > +++ b/docs/grub-dev.texi
> > @@ -79,6 +79,7 @@ This edition documents version @value{VERSION}.
> >  * Contributing Changes::
> >  * Setting up and running test suite::
> >  * Updating External Code::
> > +* Debugging::
> >  * Porting::
> >  * Error Handling::
> >  * Stack and heap size::
> > @@ -595,6 +596,196 @@ cp minilzo-2.10/*.[hc] grub-core/lib/minilzo
> >  rm -r minilzo-2.10*
> >  @end example
> >
> > +@node Debugging
> > +@chapter Debugging
> > +
> > +GRUB2 can be difficult to debug because it runs on the bare-metal
> > and thus +does not have the debugging facilities normally provided
> > by an operating +system. This chapter aims to provide useful
> > information on some ways to +debug GRUB2 for some architectures. It
> > by no means intends to be exhaustive. +The focus will be one X86_64
> > and i386 architectures. Luckily for some issues +virtual machines
> > have made the ability to debug GRUB2 much easier, and this +chapter
> > will focus debugging via the QEMU virtual machine. We will not be
> > +going over debugging of the userland tools (eg. grub-install),
> > there are +many tutorials on debugging programs in userland. +
> > +You will need GDB and the QEMU binaries for your system, on Debian
> > these +can be installed with the @samp{gdb} and
> > @samp{qemu-system-x86} packages. +Also it is assumed that you have
> > already successfully compiled GRUB2 from +source for the target
> > specified in the section below and have some +familiarity with GDB.
> > When GRUB2 is built it will create many different +binaries. The
> > ones of concern will be in the @file{grub-core} +directory of the
> > GRUB2 build dir. To aide in debugging we will want the +debugging
> > symbols generated during the build because these symbols are not
> > +kept in the binaries which get installed to the boot location. The
> > build +process outputs two sets of binaries, one without symbols
> > which gets executed +at boot, and another set of ELF images with
> > debugging symbols. The built +images with debugging symbols will
> > have a @file{.image} suffix, and the ones +without a @file{.img}
> > suffix. Similarly, loadable modules with debugging +symbols will
> > have a @file{.module} suffix, and ones without a @file{.mod}
> > +suffix. In the case of the kernel the binary with symbols is named
> > +@file{kernel.exec}. + +In the following sections, information will
> > be provided on debugging on +various targets using @command{gdb}
> > and the @samp{gdb_grub} GDB script. +
> > +@menu
> > +* i386-pc::
> > +* x86_64-efi::
> > +@end menu
> > +
> > +@node i386-pc
> > +@section i386-pc
> > +
> > +The i386-pc target is a good place to start when first debugging
> > GRUB2 +because in some respects its easier than EFI platforms. The
> > reason being +that the initial load address is always known in
> > advance. To start +debugging GRUB2 first QEMU must be started in
> > GDB stub mode. The following +command is a simple illustration:
> > +
> > +@example
> > +qemu-system-i386 -drive file=disk.img,format=raw \
> > +    -device virtio-scsi-pci,id=scsi0,num_queues=4 -S -s
> > +@end example
> > +
> > +This will start a QEMU instance booting from @file{disk.img}. It
> > will pause +at start waiting for a GDB instance to attach to it.
> > You should change +@file{disk.img} to something more appropriate. A
> > block device can be used, +but you may need to run QEMU as a
> > privileged user. +
> > +To connect to this QEMU instance with GDB, the @code{target
> > remote} GDB +command must be used. We also need to load a binary
> > image, preferably with +symbols. This can be done using the GDB
> > command @code{file kernel.exec}, if +GDB is started from the
> > @file{grub-core} directory in the GRUB2 build +directory. GRUB2
> > developers have made this more simple by including a GDB +script
> > which does much of the setup. This file at
> > @file{grub-core/gdb_grub} +of the build directory and is also
> > installed via @command{make install}. +If not building GRUB, the
> > distribution may have a package which installs +this GDB script
> > along with debug symbol binaries, such as Debian's
> > +@samp{grub-pc-dbg} package. The GDB scripts is intended to by used
> > +like so, assuming: + +@example
> > +cd $(dirname /path/to/script/gdb_grub)
> > +gdb -x gdb_grub
> > +@end example
> > +
> > +Once GDB has been started with the @file{gdb_grub} script it will
> > +automatically connect to the QEMU instance. You can then do things
> > you +normally would in GDB like set a break point on
> > @var{grub_main}. +
> > +Setting breakpoints in modules is trickier since they haven't been
> > loaded +yet and are loaded at addresses determined at runtime. The
> > module could be +loaded to different addresses in different QEMU
> > instances. The debug symbols +in the modules @file{.module} binary,
> > thus are always wrong, and GDB needs +to be told where to load the
> > symbols to. But this must happen at runtime +after GRUB2 has
> > determined where the module will get loaded. Luckily the
> > +@file{gdb_grub} script takes care of this with the
> > @command{runtime_load_module} +command, which configures GDB to
> > watch for GRUB2 module loading and when +it does add the module
> > symbols with the appropriate offset. + +@node x86_64-efi
> > +@section x86_64-efi
> > +
> > +Using GDB to debug GRUB2 for the x86_64-efi target has some
> > similarities with +the i386-pc target. Please read be familiar with
> > the @ref{x86_64-efi} section +when reading this one. Extra care
> > must be used to run QEMU such that it boots +a UEFI firmware. This
> > usually involves either using the @samp{-bios} option +with a UEFI
> > firmware blob (eg. @file{OVMF.fd}) or loading the firmware via
> > +pflash. This document will not go further into how to do this as
> > there are +ample resource on the web. +
> > +Like all EFI implementations, on x86_64-efi the (U)EFI firmware
> > that loads +the GRUB2 EFI application determines at runtime where
> > the application will +be loaded. This means that we do not know
> > where to tell GDB to load the +symbols for the GRUB2 core until the
> > (U)EFI firmware determines it. There +two good ways of figuring
> > this out when running in QEMU: use a @ref{OVMF debug log, +debug
> > build of OVMF} and check the debug log or have GRUB2 say where it
> > is +loaded when it starts. Neither of these are ideal because they
> > both +generally give the information after GRUB2 is already
> > running, which makes +debugging early boot infeasible. Technically,
> > the first method does give +the load address before GRUB2 is run,
> > but without debugging the EFI firmware +with symbols, the author
> > currently does not know how to cause the OVMF +firmware to pause at
> > that point to use the load address before GRUB2 is run. + +Even
> > after getting the application load address, the loading of core
> > symbols +is complicated by the fact that the debugging symbols for
> > the kernel are in +an ELF binary named @file{kernel.exec} while
> > what is in memory are sections +for the PE32+ EFI binary. When
> > @command{grub-mkimage} creates the PE32+ +binary it condenses
> > several segments from the ELF kernel binary into one +.data section
> > in the PE32+ binary. This must be taken into account to +properly
> > load the other non-text sections. Otherwise, GDB will work as
> > +expected when breaking on functions, but, for instance, global
> > variables +will point to the wrong address in memory and thus give
> > incorrect values +(which can be difficult to debug). + +The
> > calculating of the correct offsets for sections when loading symbol
> > +files are taken care of when loading the kernel symbols via the
> > user-defined +GDB command
> > @command{dynamic_load_kernel_exec_symbols}, which takes one
> > +argument, the address where the text section is loaded, as
> > determined by +one of the methods above. Alternatively, the command
> > @command{dynamic_load_symbols} +with the text section address as an
> > agrument can be called to load the +kernel symbols and setup
> > loading the module symbols as they are loaded at +runtime. + +In
> > the author's experience, when debugging with QEMU and OVMF, to have
> > +debugging symbols loaded at the start of GRUB2 execution the GRUB2
> > EFI +application must be run via QEMU at least once prior in order
> > to get the +load address. Two methods for obtaining the load
> > address are described in +two subsections below. Generally
> > speaking, the load address does not change +between QEMU runs.
> > There are exceptions to this, namely that different +GRUB2 EFI
> > Applications can be run at different addresses. Also, its been
> > +observed that after running the EFI application for the first
> > time, the +second run will many times have a different load
> > address, but subsequent +runs of the same EFI application will have
> > the same load address as the +second run. This predictability
> > allows us to asume the load address on +subsequent runs and thus
> > load the symbols before GRUB2 starts. The following +command
> > illustrates this, assuming that QEMU is running and waiting for +a
> > debugger connection and the current working directory is where
> > +@file{gdb_grub} resides: + +@example +gdb -x gdb_grub -ex
> > 'dynamic_load_symbols @var{load address}' +@end example
> > +
> > +If you load the symbols in this manner and, after continuing
> > execution, do +not see output showing the loading of modules
> > symbol, then its very likely +that the load address was incorrect.
> > +
> > +
> > +@node OVMF debug log
> > +@subsection OVMF debug log
> > +
> > +In order to get the GRUB2 load address from OVMF, first, a debug
> > build +of OVMF must be obtained
> > (@uref{https://github.com/retrage/edk2-nightly/raw/master/bin/DEBUGX64_OVMF.fd,
> > +here is one} which is not officially recommended). OVMF will
> > output debug +messages to a special serial device, which we must
> > add to QEMU. The following +QEMU command will run the debug OVMF
> > and write the debug messages to a +file named @file{debug.log}. It
> > is assumed that @file{disk.img} is a disk +image or block device
> > that is setup to boot GRUB2 EFI. + +@example
> > +qemu-system-x86_64 -bios /path/to/debug/OVMF.fd \
> > +    -drive file=disk.img,format=raw \
> > +    -device virtio-scsi-pci,id=scsi0,num_queues=4 \
> > +    -debugcon file:debug.log -global isa-debugcon.iobase=0x402
> > +@end example
> > +
> > +If GRUB2 was started by the (U)EFI firmware, then in the
> > @file{debug.log} +file one of the last lines should be a log
> > message like: +@code{Loading driver at 0x00006AEE000
> > EntryPoint=0x00006AEE756}. This +means that the GRUB2 EFI
> > application was loaded at @code{0x00006AEE000} and +its .text
> > section is at @code{0x00006AEE756}. +
> > +@node Build GRUB2 to print out the load address
> > +@subsection Build GRUB2 to print out the load address
> > +
> > +GRUB2 can be specially built to output the address of its .text
> > section in +memory by defining @code{PRINT_GDB_SYM_LOAD_CMD} to
> > @code{1} in @file{config.h.in} +before running @command{configure}.
> > The benefit of this method is that it +will work on non-virtualized
> > hardware where the (U)EFI firmware may not +be modifiable.
> > +
> >  @node Porting
> >  @chapter Porting
> >
> > --
> > 2.34.1
> >
> >
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> I definitely support this.
> Last time I tried a lot of time on trying the same thing as Glenn
> documented here.
> The symbol rule, debug built edk2 with verbose, the explanation of
> loading modules are really helpful for the grub newbie such as me!
> There was not much information on gdb grub online.
> Adding these detailed sections indeed helps more people to contribute
> grub. BTW, in my experience, we usually need to use hb instead of b
> when gdb grub in qemu.

Actually software breakpoints work just fine. What was probably
happening for you was that you were setting software breakpoints before
the GRUB EFI app was loaded, so the breakpoints were getting erased and
it would have appeared as though they didn't work. This behavior should
be documented though, so thanks for the suggestion.

Glenn


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v4 08/15] gdb: If enabled, print line used to load EFI kernel symbols when using gdb_grub script
  2022-12-21 17:57     ` Glenn Washburn
@ 2022-12-22 18:17       ` Daniel Kiper
  2022-12-24  1:26         ` Glenn Washburn
  0 siblings, 1 reply; 31+ messages in thread
From: Daniel Kiper @ 2022-12-22 18:17 UTC (permalink / raw)
  To: Glenn Washburn; +Cc: grub-devel, Peter Jones, Robbie Harwood

On Wed, Dec 21, 2022 at 11:57:33AM -0600, Glenn Washburn wrote:
> On Wed, 21 Dec 2022 16:20:17 +0100
> Daniel Kiper <dkiper@net-space.pl> wrote:
>
> > Adding Robbie...
> >
> > Please CC him next time when you post these patches. I would want to
> > hear his opinion too. Or at least he is aware what is happening
> > here...
>
> Sure, I CC'd him and Peter on the first couple of ones. But there was

Thank you!

> never had a response in the 4 months since then, so I figured they
> didn't care.

Until somebody ask you to not include themselves in the thread please
CC them. AFAICT many people read emails often, like me, but jump into
discussion when something really important for them is happening.

> > On Thu, Dec 15, 2022 at 11:29:31PM -0600, Glenn Washburn wrote:
> > > If the macro PRINT_GDB_SYM_LOAD_CMD is non-zero, compile code which

Why is this not a flag, like e.g. --enable-mm-debug, for the configure?

> > > will print the command needed to load symbols for the GRUB EFI
> > > kernel. This is needed because EFI firmware determines where to
> > > load the GRUB EFI at runtime, and so the relevant addresses are not
> > > known ahead of time.
> > >
> > > The command is a custom command defined in the gdb_grub GDB script.
> > > So GDB should be started with the script as an argument to the -x
> > > option or sourced into an active GDB session before running the
> > > outputted command.
> >
> > I think this functionality should be disabled when lockdown is
> > enforced, e.g. on UEFI platforms with Secure Boot enabled.
>
> Since this is off by default and must be enabled at build time, then if
> the builder enabled it, they really did want it, regardless of
> lockdown. What you're worried about seems highly improbable to me (but
> then I don't know the inner workings of the distros). The concern as I
> understand it, is that someone doing an official release of a distro
> which will be secure boot ready will accidentally have this build time
> macro enabled. That's almost inconceivable to me, but I'm curious what
> the others have to say (especially since Robbie posted a similar patch
> that always printed this info as a debug message[1]). Or is it more
> about a regular user signing with their own keys accidentally shooting
> themselves in the foot by forgetting to disable this (after having
> already enabled it) and then some physical attacker getting extra info
> to do an evil maid attack?

I can imagine that a distro builds and signs GRUB with debug embedded and
then somebody in the wild wants to enable this feature to debug a problem.
Of course them cannot rebuild the GRUB image because it is signed. However,
them can disable UEFI Secure Boot and enable debugging. Of course this
probably will not work in all cases but should help solve most problems.

Daniel


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v4 12/15] gdb: Allow running user-defined commands at GRUB start
  2022-12-22  6:08       ` Glenn Washburn
@ 2022-12-22 18:21         ` Daniel Kiper
  0 siblings, 0 replies; 31+ messages in thread
From: Daniel Kiper @ 2022-12-22 18:21 UTC (permalink / raw)
  To: Glenn Washburn; +Cc: Jeremy Szu, grub-devel

On Thu, Dec 22, 2022 at 12:08:17AM -0600, Glenn Washburn wrote:
> On Wed, 21 Dec 2022 12:19:16 -0600
> Glenn Washburn <development@efficientek.com> wrote:
>
> > On Wed, 21 Dec 2022 16:27:40 +0100
> > Daniel Kiper <dkiper@net-space.pl> wrote:
> >
> > > On Thu, Dec 15, 2022 at 11:29:35PM -0600, Glenn Washburn wrote:
> > > > A new command, run_on_start, is created which handles some
> > > > complexities of the EFI platform when breaking on GRUB start. If
> > > > GRUB start is hooked, run "onstart" command if it is defned.
> > > >
> > > > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > > > ---
> > > >  grub-core/gdb_grub.in | 38 ++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 38 insertions(+)
> > > >
> > > > diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
> > > > index 8ae6344edf..3b3cea1a4d 100644
> > > > --- a/grub-core/gdb_grub.in
> > > > +++ b/grub-core/gdb_grub.in
> > > > @@ -36,6 +36,8 @@ end
> > > >  define dynamic_load_symbols
> > > >  	dynamic_load_kernel_exec_symbols $arg0
> > > >
> > > > +	run_on_start
> > > > +
> > > >  	# We may have been very late to loading the kernel.exec
> > > > symbols and # and modules may already be loaded. So load symbols
> > > > for any already # loaded.
> > > > @@ -134,6 +136,41 @@ document runtime_load_module
> > > >  	Load module symbols at runtime as they are loaded.
> > > >  end
> > > >
> > > > +define run_on_start
> > > > +	# TODO: Add check to see if _start symbol is defined, if
> > > > not, then
> > > > +	# the symbols have not yet been loaded and this command
> > > > will not work.
> > > > +	watch *_start
> > > > +	set $break_efi_start_bpnum = $bpnum
> > > > +	commands
> > > > +		silent
> > > > +		delete $break_efi_start_bpnum
> > > > +		break _start
> > >
> > > s/break/hbreak/?
> >
> > A regular break works here for me. I want to avoid hbreak at all
> > costs, which is why I had a previous convoluted method using break
> > (which I thought worked, and then found it didn't quite). My
> > understanding is that the number of hardware breakpoints are limited
> > and commonly its around 4. Specifically, my understanding is that on
> > x86-64 the number is exactly 4, so I would prefer the user have
> > usable as many as possible.
> >
> > Really, I'd like to figure out why sometimes break works and why
> > sometimes not, and then figure out a way to make it work for these
> > scripts. I recently had the idea that maybe the UEFI firmware sets up
> > the pages where it loads the .text section of the GRUB UEFI binary to
> > readonly in the page table structure. But I went through the structure
> > when %eip is at _start and the R/W bit is set on the pages I checked.
> > Even if the pages were set to readonly, I suspect the qemu gdb stub
> > allows writing to that memory anyway.
> >
> > So I'm at a loss as to what could be preventing break from working.
> > I'd love to hear some ideas if anyone has some.
>
> Okay so its been a while since I was deep in this and I forgot some
> stuff I already knew (and which I should incorporate into the
> documentation patch). The reason that software break doesn't always

Great!

> work, is that when its not working its because the breakpoint is being
> set before the GRUB EFI app is loaded into memory. So happens is that
> GDB sets a breakpoint (ie a 0xcc instruction where the symbol points
> to), but it then gets over-written by the UEFI firmware as its loads
> the GRUB EFI app. So the app loading effectively will clear all
> software breakpoints. This doesn't affect the hardware breakpoints
> because they are in CPU debug registers which are not affected by the
> firmware loading the EFI app.

Yeah, this is what I expected...

> The reason that the above worked, and was written that way in the first
> place, is that when the watch commands run the memory at _start has just
> been modified. This happens when the UEFI firmware is loading the GRUB
> EFI app. Then we can use software break without worrying about it being
> cleared.

Could you add this (whole) explanation to the commit message to make it
clear why we can use "break" here?

> Revisiting this has given me some ideas for improving the patch series.

Cool!

Thanks,

Daniel


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v4 13/15] gdb: Add extra early initialization symbols for i386-pc
  2022-12-21 18:21     ` Glenn Washburn
@ 2022-12-22 18:25       ` Daniel Kiper
  0 siblings, 0 replies; 31+ messages in thread
From: Daniel Kiper @ 2022-12-22 18:25 UTC (permalink / raw)
  To: Glenn Washburn; +Cc: grub-devel

On Wed, Dec 21, 2022 at 12:21:26PM -0600, Glenn Washburn wrote:
> On Wed, 21 Dec 2022 16:28:35 +0100
> Daniel Kiper <dkiper@net-space.pl> wrote:
>
> > On Thu, Dec 15, 2022 at 11:29:36PM -0600, Glenn Washburn wrote:
> > > Add symbols for boot.image, disk.image, and lzma_decompress.image
> > > if the target is i386-pc.
> >
> > Why?
>
> Why not? I'm not clear on what your objection is. This is for debugging
> the code that has those symbols, eg. to break on those symbols.

This is not an objection. I want you to explain in the commit message
why this patch is needed. This thing is missing...

Sorry if I was to vague...

Daniel


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v4 08/15] gdb: If enabled, print line used to load EFI kernel symbols when using gdb_grub script
  2022-12-22 18:17       ` Daniel Kiper
@ 2022-12-24  1:26         ` Glenn Washburn
  0 siblings, 0 replies; 31+ messages in thread
From: Glenn Washburn @ 2022-12-24  1:26 UTC (permalink / raw)
  To: Daniel Kiper; +Cc: grub-devel, Peter Jones, Robbie Harwood

On Thu, 22 Dec 2022 19:17:31 +0100
Daniel Kiper <dkiper@net-space.pl> wrote:

> On Wed, Dec 21, 2022 at 11:57:33AM -0600, Glenn Washburn wrote:
> > On Wed, 21 Dec 2022 16:20:17 +0100
> > Daniel Kiper <dkiper@net-space.pl> wrote:
> >
> > > Adding Robbie...
> > >
> > > Please CC him next time when you post these patches. I would want
> > > to hear his opinion too. Or at least he is aware what is happening
> > > here...
> >
> > Sure, I CC'd him and Peter on the first couple of ones. But there
> > was
> 
> Thank you!
> 
> > never had a response in the 4 months since then, so I figured they
> > didn't care.
> 
> Until somebody ask you to not include themselves in the thread please
> CC them. AFAICT many people read emails often, like me, but jump into
> discussion when something really important for them is happening.
> 
> > > On Thu, Dec 15, 2022 at 11:29:31PM -0600, Glenn Washburn wrote:
> > > > If the macro PRINT_GDB_SYM_LOAD_CMD is non-zero, compile code
> > > > which
> 
> Why is this not a flag, like e.g. --enable-mm-debug, for the
> configure?

I had thought about it, and honestly, I was hoping someone would have a
better idea on the user interface. It feels clunky to me, so I wasn't
really wanting to advertise it. I think I'll add it in the next version.

> 
> > > > will print the command needed to load symbols for the GRUB EFI
> > > > kernel. This is needed because EFI firmware determines where to
> > > > load the GRUB EFI at runtime, and so the relevant addresses are
> > > > not known ahead of time.
> > > >
> > > > The command is a custom command defined in the gdb_grub GDB
> > > > script. So GDB should be started with the script as an argument
> > > > to the -x option or sourced into an active GDB session before
> > > > running the outputted command.
> > >
> > > I think this functionality should be disabled when lockdown is
> > > enforced, e.g. on UEFI platforms with Secure Boot enabled.
> >
> > Since this is off by default and must be enabled at build time,
> > then if the builder enabled it, they really did want it, regardless
> > of lockdown. What you're worried about seems highly improbable to
> > me (but then I don't know the inner workings of the distros). The
> > concern as I understand it, is that someone doing an official
> > release of a distro which will be secure boot ready will
> > accidentally have this build time macro enabled. That's almost
> > inconceivable to me, but I'm curious what the others have to say
> > (especially since Robbie posted a similar patch that always printed
> > this info as a debug message[1]). Or is it more about a regular
> > user signing with their own keys accidentally shooting themselves
> > in the foot by forgetting to disable this (after having already
> > enabled it) and then some physical attacker getting extra info to
> > do an evil maid attack?
> 
> I can imagine that a distro builds and signs GRUB with debug embedded
> and then somebody in the wild wants to enable this feature to debug a
> problem. Of course them cannot rebuild the GRUB image because it is
> signed. However, them can disable UEFI Secure Boot and enable
> debugging. Of course this probably will not work in all cases but
> should help solve most problems.

As I understand it then, the main downside is that debugging in lockdown
mode doesn't get any easier to debug. I guess I don't see that
affecting me terribly, so I'm not opposed to it.

Thinking about this more, I think I should add a command called
"gdbinfo" which also prints out this info on-demand by the user. I'll
have it disabled in lockdown mode too. I think this will make it nicer
for someone debugging issues with GRUB on real hardware and where the
issue is not in early boot. As it is now, I think the output would too
quickly disappear from the physical screen for it to be useful.

Glenn


^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2022-12-24  1:26 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-16  5:29 [PATCH v4 00/15] GDB script fixes and improvements Glenn Washburn
2022-12-16  5:29 ` [PATCH v4 01/15] gdb: Fix redirection issue in dump_module_sections Glenn Washburn
2022-12-16  5:29 ` [PATCH v4 02/15] gdb: Prevent wrapping when writing to .segments.tmp Glenn Washburn
2022-12-16  5:29 ` [PATCH v4 03/15] gdb: If no modules have been loaded, do not try to load module symbols Glenn Washburn
2022-12-16  5:29 ` [PATCH v4 04/15] gdb: Move runtime module loading into runtime_load_module Glenn Washburn
2022-12-21 14:57   ` Daniel Kiper
2022-12-16  5:29 ` [PATCH v4 05/15] gdb: Reliably load modules in runtime_load_module Glenn Washburn
2022-12-16  5:29 ` [PATCH v4 06/15] gdb: Add functions to make loading from dynamically positioned targets easier Glenn Washburn
2022-12-16  5:29 ` [PATCH v4 07/15] gdb: Remove Perl dependency for GRUB GDB script Glenn Washburn
2022-12-16  5:29 ` [PATCH v4 08/15] gdb: If enabled, print line used to load EFI kernel symbols when using gdb_grub script Glenn Washburn
2022-12-21 15:20   ` Daniel Kiper
2022-12-21 17:57     ` Glenn Washburn
2022-12-22 18:17       ` Daniel Kiper
2022-12-24  1:26         ` Glenn Washburn
2022-12-16  5:29 ` [PATCH v4 09/15] gdb: Conditionally run GDB script logic for dynamically or statically positioned GRUB Glenn Washburn
2022-12-16  5:29 ` [PATCH v4 10/15] gdb: Only connect to remote target once when first sourced Glenn Washburn
2022-12-16  5:29 ` [PATCH v4 11/15] gdb: Allow user defined "onload_<modname>" command to be run when module is loaded Glenn Washburn
2022-12-16  5:29 ` [PATCH v4 12/15] gdb: Allow running user-defined commands at GRUB start Glenn Washburn
2022-12-21 15:27   ` Daniel Kiper
2022-12-21 18:19     ` Glenn Washburn
2022-12-22  6:08       ` Glenn Washburn
2022-12-22 18:21         ` Daniel Kiper
2022-12-16  5:29 ` [PATCH v4 13/15] gdb: Add extra early initialization symbols for i386-pc Glenn Washburn
2022-12-21 15:28   ` Daniel Kiper
2022-12-21 18:21     ` Glenn Washburn
2022-12-22 18:25       ` Daniel Kiper
2022-12-16  5:29 ` [PATCH v4 14/15] gdb: Add ability to turn on shell tracing for gdb helper script Glenn Washburn
2022-12-16  5:29 ` [PATCH v4 15/15] docs: Add debugging chapter to development documentation Glenn Washburn
2022-12-21  3:07   ` Jeremy Szu
2022-12-22  6:12     ` Glenn Washburn
2022-12-21 15:50   ` Daniel Kiper

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.