[PATCH] gdb: Add malloc and free symbols to kernel.exec to improve gdb functionality

[PATCH] gdb: Add malloc and free symbols to kernel.exec to improve gdb functionality

[Glenn Washburn]

Add linker flags when linking kernel.exec to have malloc and free point to
grub_malloc and grub_free respectively. Some gdb functionality depends on
gdb locating the symbols "malloc" and "free", such as dynamically creating
strings for arguments to injected function calls. A trivial example would
the gdb command 'p strlen("astring")'.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
This should have been included in the gdb patch series I recently sent,
although its not required by nor requires any of those patches.

Glenn

---
 conf/Makefile.common | 1 +
 1 file changed, 1 insertion(+)

diff --git a/conf/Makefile.common b/conf/Makefile.common
index f0bb6e160a..069b428c1a 100644
--- a/conf/Makefile.common
+++ b/conf/Makefile.common
@@ -36,6 +36,7 @@ BUILD_CPPFLAGS += $(CPPFLAGS_DEFAULT)
 
 CFLAGS_KERNEL = $(CFLAGS_PLATFORM) -ffreestanding
 LDFLAGS_KERNEL = $(LDFLAGS_PLATFORM) -nostdlib $(TARGET_LDFLAGS_OLDMAGIC)
+LDFLAGS_KERNEL += -Wl,--defsym=malloc=grub_malloc -Wl,--defsym=free=grub_free
 CPPFLAGS_KERNEL = $(CPPFLAGS_CPU) $(CPPFLAGS_PLATFORM) -DGRUB_KERNEL=1
 CCASFLAGS_KERNEL = $(CCASFLAGS_CPU) $(CCASFLAGS_PLATFORM)
 STRIPFLAGS_KERNEL = -R .rel.dyn -R .reginfo -R .note -R .comment -R .drectve -R .note.gnu.gold-version -R .MIPS.abiflags -R .ARM.exidx
-- 
2.27.0

Re: [PATCH] gdb: Add malloc and free symbols to kernel.exec to improve gdb functionality

[Daniel Kiper]

On Wed, Mar 02, 2022 at 06:25:12PM -0600, Glenn Washburn wrote:
> Add linker flags when linking kernel.exec to have malloc and free point to
> grub_malloc and grub_free respectively. Some gdb functionality depends on
> gdb locating the symbols "malloc" and "free", such as dynamically creating
> strings for arguments to injected function calls. A trivial example would
> the gdb command 'p strlen("astring")'.
>
> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
> This should have been included in the gdb patch series I recently sent,
> although its not required by nor requires any of those patches.
>
> Glenn
>
> ---
>  conf/Makefile.common | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/conf/Makefile.common b/conf/Makefile.common
> index f0bb6e160a..069b428c1a 100644
> --- a/conf/Makefile.common
> +++ b/conf/Makefile.common
> @@ -36,6 +36,7 @@ BUILD_CPPFLAGS += $(CPPFLAGS_DEFAULT)
>
>  CFLAGS_KERNEL = $(CFLAGS_PLATFORM) -ffreestanding
>  LDFLAGS_KERNEL = $(LDFLAGS_PLATFORM) -nostdlib $(TARGET_LDFLAGS_OLDMAGIC)
> +LDFLAGS_KERNEL += -Wl,--defsym=malloc=grub_malloc -Wl,--defsym=free=grub_free

Could not we teach gdb somehow to use grub_malloc()/grub_free() instead
of malloc()/free()?

Daniel

Re: [PATCH] gdb: Add malloc and free symbols to kernel.exec to improve gdb functionality

[Glenn Washburn]

On Wed, 9 Mar 2022 16:49:57 +0100
Daniel Kiper <dkiper@net-space.pl> wrote:

> On Wed, Mar 02, 2022 at 06:25:12PM -0600, Glenn Washburn wrote:
> > Add linker flags when linking kernel.exec to have malloc and free point to
> > grub_malloc and grub_free respectively. Some gdb functionality depends on
> > gdb locating the symbols "malloc" and "free", such as dynamically creating
> > strings for arguments to injected function calls. A trivial example would
> > the gdb command 'p strlen("astring")'.
> >
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > ---
> > This should have been included in the gdb patch series I recently sent,
> > although its not required by nor requires any of those patches.
> >
> > Glenn
> >
> > ---
> >  conf/Makefile.common | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/conf/Makefile.common b/conf/Makefile.common
> > index f0bb6e160a..069b428c1a 100644
> > --- a/conf/Makefile.common
> > +++ b/conf/Makefile.common
> > @@ -36,6 +36,7 @@ BUILD_CPPFLAGS += $(CPPFLAGS_DEFAULT)
> >
> >  CFLAGS_KERNEL = $(CFLAGS_PLATFORM) -ffreestanding
> >  LDFLAGS_KERNEL = $(LDFLAGS_PLATFORM) -nostdlib $(TARGET_LDFLAGS_OLDMAGIC)
> > +LDFLAGS_KERNEL += -Wl,--defsym=malloc=grub_malloc -Wl,--defsym=free=grub_free
> 
> Could not we teach gdb somehow to use grub_malloc()/grub_free() instead
> of malloc()/free()?

Considering the tons of options that GDB has, I was hoping the same
thing. Unfortunately, it appears to be hardcoded[1]. So not without
changing the source.

Glenn

[1]
https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/valops.c;hb=HEAD#l169

> 
> Daniel

Re: [PATCH] gdb: Add malloc and free symbols to kernel.exec to improve gdb functionality

[Daniel Kiper]

On Wed, Mar 09, 2022 at 02:25:28PM -0600, Glenn Washburn wrote:
> On Wed, 9 Mar 2022 16:49:57 +0100
> Daniel Kiper <dkiper@net-space.pl> wrote:
>
> > On Wed, Mar 02, 2022 at 06:25:12PM -0600, Glenn Washburn wrote:
> > > Add linker flags when linking kernel.exec to have malloc and free point to
> > > grub_malloc and grub_free respectively. Some gdb functionality depends on
> > > gdb locating the symbols "malloc" and "free", such as dynamically creating
> > > strings for arguments to injected function calls. A trivial example would
> > > the gdb command 'p strlen("astring")'.
> > >
> > > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > > ---
> > > This should have been included in the gdb patch series I recently sent,
> > > although its not required by nor requires any of those patches.
> > >
> > > Glenn
> > >
> > > ---
> > >  conf/Makefile.common | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/conf/Makefile.common b/conf/Makefile.common
> > > index f0bb6e160a..069b428c1a 100644
> > > --- a/conf/Makefile.common
> > > +++ b/conf/Makefile.common
> > > @@ -36,6 +36,7 @@ BUILD_CPPFLAGS += $(CPPFLAGS_DEFAULT)
> > >
> > >  CFLAGS_KERNEL = $(CFLAGS_PLATFORM) -ffreestanding
> > >  LDFLAGS_KERNEL = $(LDFLAGS_PLATFORM) -nostdlib $(TARGET_LDFLAGS_OLDMAGIC)
> > > +LDFLAGS_KERNEL += -Wl,--defsym=malloc=grub_malloc -Wl,--defsym=free=grub_free
> >
> > Could not we teach gdb somehow to use grub_malloc()/grub_free() instead
> > of malloc()/free()?
>
> Considering the tons of options that GDB has, I was hoping the same
> thing. Unfortunately, it appears to be hardcoded[1]. So not without
> changing the source.

:-( I expected you checked that but wanted to be sure... :-)

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

Re: [PATCH] gdb: Add malloc and free symbols to kernel.exec to improve gdb functionality

[Daniel Kiper]

On Fri, Mar 11, 2022 at 12:09:58AM +0100, Daniel Kiper wrote:
> On Wed, Mar 09, 2022 at 02:25:28PM -0600, Glenn Washburn wrote:
> > On Wed, 9 Mar 2022 16:49:57 +0100
> > Daniel Kiper <dkiper@net-space.pl> wrote:
> >
> > > On Wed, Mar 02, 2022 at 06:25:12PM -0600, Glenn Washburn wrote:
> > > > Add linker flags when linking kernel.exec to have malloc and free point to
> > > > grub_malloc and grub_free respectively. Some gdb functionality depends on
> > > > gdb locating the symbols "malloc" and "free", such as dynamically creating
> > > > strings for arguments to injected function calls. A trivial example would
> > > > the gdb command 'p strlen("astring")'.
> > > >
> > > > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > > > ---
> > > > This should have been included in the gdb patch series I recently sent,
> > > > although its not required by nor requires any of those patches.
> > > >
> > > > Glenn
> > > >
> > > > ---
> > > >  conf/Makefile.common | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/conf/Makefile.common b/conf/Makefile.common
> > > > index f0bb6e160a..069b428c1a 100644
> > > > --- a/conf/Makefile.common
> > > > +++ b/conf/Makefile.common
> > > > @@ -36,6 +36,7 @@ BUILD_CPPFLAGS += $(CPPFLAGS_DEFAULT)
> > > >
> > > >  CFLAGS_KERNEL = $(CFLAGS_PLATFORM) -ffreestanding
> > > >  LDFLAGS_KERNEL = $(LDFLAGS_PLATFORM) -nostdlib $(TARGET_LDFLAGS_OLDMAGIC)
> > > > +LDFLAGS_KERNEL += -Wl,--defsym=malloc=grub_malloc -Wl,--defsym=free=grub_free
> > >
> > > Could not we teach gdb somehow to use grub_malloc()/grub_free() instead
> > > of malloc()/free()?
> >
> > Considering the tons of options that GDB has, I was hoping the same
> > thing. Unfortunately, it appears to be hardcoded[1]. So not without
> > changing the source.
>
> :-( I expected you checked that but wanted to be sure... :-)
>
> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Sadly this patch breaks Windows builds... :-(

i686-w64-mingw32-gcc -std=gnu99 -fno-common -Os -m32 -Wall -W -Wshadow -Wpointer-arith -Wundef -Wchar-subscripts -Wcomment -Wdeprecated-declarations -Wdisabled-optimization -Wdiv-by-zero -Wfloat-equal -Wformat-extra-args -Wformat-security -Wformat-y2k -Wimplicit -Wimplicit-function-declaration -Wimplicit-int -Wmain -Wmissing-braces -Wmissing-format-attribute -Wmultichar -Wparentheses -Wreturn-type -Wsequence-point -Wshadow -Wsign-compare -Wswitch -Wtrigraphs -Wunknown-pragmas -Wunused -Wunused-function -Wunused-label -Wunused-parameter -Wunused-value  -Wunused-variable -Wwrite-strings -Wnested-externs -Wstrict-prototypes -g -Wredundant-decls -Wmissing-prototypes -Wmissing-declarations  -Wextra -Wattributes -Wendif-labels -Winit-self -Wint-to-pointer-cast -Winvalid-pch -Wmissing-field-initializers -Wnonnull -Woverflow -Wvla -Wpointer-to-int-cast -Wstrict-aliasing -Wvariadic-macros -Wvolatile-register-var -Wpointer-sign -Wmissing-include-dirs -Wmissing-prototypes -Wmissing-declarations -Wformat=2 -march=i386 -falign-functions=1 -falign-loops=1 -falign-jumps=1 -freg-struct-return -mno-mmx -mno-sse -mno-sse2 -mno-sse3 -mno-3dnow -msoft-float -fno-dwarf2-cfi-asm -fno-reorder-functions -mno-stack-arg-probe -fno-asynchronous-unwind-tables -fno-unwind-tables -fno-ident -mstack-protector-guard=global -fstack-protector -Wtrampolines -Werror   -ffreestanding   -m32 -Wl,-mi386pe  -nostdlib -Wl,-N -Wl,--defsym=malloc=grub_malloc -Wl,--defsym=free=grub_free -Wl,-r   -o kernel.exec.exe kern/i386/efi/kernel_exec-startup.o kern/i386/efi/kernel_exec-tsc.o kern/i386/kernel_exec-tsc_pmtimer.o kern/i386/efi/kernel_exec-init.o bus/kernel_exec-pci.o kern/i386/kernel_exec-dl.o kern/i386/kernel_exec-tsc.o kern/i386/kernel_exec-tsc_pit.o disk/efi/kernel_exec-efidisk.o kern/efi/kernel_exec-efi.o kern/efi/kernel_exec-init.o kern/efi/kernel_exec-mm.o term/efi/kernel_exec-console.o kern/kernel_exec-acpi.o kern/efi/kernel_exec-acpi.o kern/efi/kernel_exec-sb.o kern/kernel_exec-lockdown.o kern/kernel_exec-compiler-rt.o kern/kernel_exec-mm.o kern/kernel_exec-time.o kern/generic/kernel_exec-millisleep.o kern/kernel_exec-buffer.o kern/kernel_exec-command.o kern/kernel_exec-corecmd.o kern/kernel_exec-device.o kern/kernel_exec-disk.o kern/kernel_exec-dl.o kern/kernel_exec-env.o kern/kernel_exec-err.o kern/kernel_exec-file.o kern/kernel_exec-fs.o kern/kernel_exec-list.o kern/kernel_exec-main.o kern/kernel_exec-misc.o kern/kernel_exec-parser.o kern/kernel_exec-partition.o kern/kernel_exec-rescue_parser.o kern/kernel_exec-rescue_reader.o kern/kernel_exec-term.o kern/kernel_exec-verifiers.o kernel_exec-symlist.o
--defsym:1: undefined symbol `grub_malloc' referenced in expression
collect2: error: ld returned 1 exit status
Makefile:29017: recipe for target 'kernel.exec.exe' failed

The GCC and ld versions do not matter much.

i686-w64-mingw32-gcc (GCC) 10-win32 20220113
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

GNU ld (GNU Binutils) 2.37
Copyright (C) 2021 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) a later version.
This program has absolutely no warranty.

i686-w64-mingw32-gcc (GCC) 6.3.0 20170516
Copyright (C) 2016 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

GNU ld (GNU Binutils) 2.28
Copyright (C) 2017 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) a later version.
This program has absolutely no warranty.

Daniel

Re: [PATCH] gdb: Add malloc and free symbols to kernel.exec to improve gdb functionality

[Glenn Washburn]

On Tue, 15 Mar 2022 14:23:48 +0100
Daniel Kiper <dkiper@net-space.pl> wrote:

> On Fri, Mar 11, 2022 at 12:09:58AM +0100, Daniel Kiper wrote:
> > On Wed, Mar 09, 2022 at 02:25:28PM -0600, Glenn Washburn wrote:
> > > On Wed, 9 Mar 2022 16:49:57 +0100
> > > Daniel Kiper <dkiper@net-space.pl> wrote:
> > >
> > > > On Wed, Mar 02, 2022 at 06:25:12PM -0600, Glenn Washburn wrote:
> > > > > Add linker flags when linking kernel.exec to have malloc and free point to
> > > > > grub_malloc and grub_free respectively. Some gdb functionality depends on
> > > > > gdb locating the symbols "malloc" and "free", such as dynamically creating
> > > > > strings for arguments to injected function calls. A trivial example would
> > > > > the gdb command 'p strlen("astring")'.
> > > > >
> > > > > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > > > > ---
> > > > > This should have been included in the gdb patch series I recently sent,
> > > > > although its not required by nor requires any of those patches.
> > > > >
> > > > > Glenn
> > > > >
> > > > > ---
> > > > >  conf/Makefile.common | 1 +
> > > > >  1 file changed, 1 insertion(+)
> > > > >
> > > > > diff --git a/conf/Makefile.common b/conf/Makefile.common
> > > > > index f0bb6e160a..069b428c1a 100644
> > > > > --- a/conf/Makefile.common
> > > > > +++ b/conf/Makefile.common
> > > > > @@ -36,6 +36,7 @@ BUILD_CPPFLAGS += $(CPPFLAGS_DEFAULT)
> > > > >
> > > > >  CFLAGS_KERNEL = $(CFLAGS_PLATFORM) -ffreestanding
> > > > >  LDFLAGS_KERNEL = $(LDFLAGS_PLATFORM) -nostdlib $(TARGET_LDFLAGS_OLDMAGIC)
> > > > > +LDFLAGS_KERNEL += -Wl,--defsym=malloc=grub_malloc -Wl,--defsym=free=grub_free
> > > >
> > > > Could not we teach gdb somehow to use grub_malloc()/grub_free() instead
> > > > of malloc()/free()?
> > >
> > > Considering the tons of options that GDB has, I was hoping the same
> > > thing. Unfortunately, it appears to be hardcoded[1]. So not without
> > > changing the source.
> >
> > :-( I expected you checked that but wanted to be sure... :-)
> >
> > Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
> 
> Sadly this patch breaks Windows builds... :-(
> 
> i686-w64-mingw32-gcc -std=gnu99 -fno-common -Os -m32 -Wall -W -Wshadow -Wpointer-arith -Wundef -Wchar-subscripts -Wcomment -Wdeprecated-declarations -Wdisabled-optimization -Wdiv-by-zero -Wfloat-equal -Wformat-extra-args -Wformat-security -Wformat-y2k -Wimplicit -Wimplicit-function-declaration -Wimplicit-int -Wmain -Wmissing-braces -Wmissing-format-attribute -Wmultichar -Wparentheses -Wreturn-type -Wsequence-point -Wshadow -Wsign-compare -Wswitch -Wtrigraphs -Wunknown-pragmas -Wunused -Wunused-function -Wunused-label -Wunused-parameter -Wunused-value  -Wunused-variable -Wwrite-strings -Wnested-externs -Wstrict-prototypes -g -Wredundant-decls -Wmissing-prototypes -Wmissing-declarations  -Wextra -Wattributes -Wendif-labels -Winit-self -Wint-to-pointer-cast -Winvalid-pch -Wmissing-field-initializers -Wnonnull -Woverflow -Wvla -Wpointer-to-int-cast -Wstrict-aliasing -Wvariadic-macros -Wvolatile-register-var -Wpointer-sign -Wmissing-include-dirs -Wmissing-prototypes -Wmissing-declarations -Wformat=2 -march=i386 -falign-functions=1 -falign-loops=1 -falign-jumps=1 -freg-struct-return -mno-mmx -mno-sse -mno-sse2 -mno-sse3 -mno-3dnow -msoft-float -fno-dwarf2-cfi-asm -fno-reorder-functions -mno-stack-arg-probe -fno-asynchronous-unwind-tables -fno-unwind-tables -fno-ident -mstack-protector-guard=global -fstack-protector -Wtrampolines -Werror   -ffreestanding   -m32 -Wl,-mi386pe  -nostdlib -Wl,-N -Wl,--defsym=malloc=grub_malloc -Wl,--defsym=free=grub_free -Wl,-r   -o kernel.exec.exe kern/i386/efi/kernel_exec-startup.o kern/i386/efi/kernel_exec-tsc.o kern/i386/kernel_exec-tsc_pmtimer.o kern/i386/efi/kernel_exec-init.o bus/kernel_exec-pci.o kern/i386/kernel_exec-dl.o kern/i386/kernel_exec-tsc.o kern/i386/kernel_exec-tsc_pit.o disk/efi/kernel_exec-efidisk.o kern/efi/kernel_exec-efi.o kern/efi/kernel_exec-init.o kern/efi/kernel_exec-mm.o term/efi/kernel_exec-console.o kern/kernel_exec-acpi.o kern/efi/kernel_exec-acpi.o kern/efi/kernel_exec-sb.o kern/kernel_exec-lockdown.o kern/kernel_exec-compiler-rt.o kern/kernel_exec-mm.o kern/kernel_exec-time.o kern/generic/kernel_exec-millisleep.o kern/kernel_exec-buffer.o kern/kernel_exec-command.o kern/kernel_exec-corecmd.o kern/kernel_exec-device.o kern/kernel_exec-disk.o kern/kernel_exec-dl.o kern/kernel_exec-env.o kern/kernel_exec-err.o kern/kernel_exec-file.o kern/kernel_exec-fs.o kern/kernel_exec-list.o kern/kernel_exec-main.o kern/kernel_exec-misc.o kern/kernel_exec-parser.o kern/kernel_exec-partition.o kern/kernel_exec-rescue_parser.o kern/kernel_exec-rescue_reader.o kern/kernel_exec-term.o kern/kernel_exec-verifiers.o kernel_exec-symlist.o
> --defsym:1: undefined symbol `grub_malloc' referenced in expression
> collect2: error: ld returned 1 exit status
> Makefile:29017: recipe for target 'kernel.exec.exe' failed

I'll look into it. At first glance, I had to say its odd that there is
no grub_malloc symbol found as this must be exported for modules to
use.

Also, it looks like you're building for i386-efi, which seems strange.
Do you get this error with both x86_64-efi and i386-pc? Regardless, I'll
look in to replicating it on a windows cross-compiler on Linux. Also,
it looks like the build machine is windows, as opposed to on Linux
with a wnidows cross compiler. What exactly was your build and host
machine type as detected by configure?

Would you find it acceptable to only conditionally include those
arguments? I'm thinking excluding builds where the build or host are
windows environments. I'd be surprised to hear anyone using GDB on
windows to debug GRUB.

BTW, as you can probably tell, I don't test building for windows and
probably should. Although, I don't want to do it where the build
machine is windows (I think its a good idea, I just don't want to work
on that).

Glenn

> 
> The GCC and ld versions do not matter much.
> 
> i686-w64-mingw32-gcc (GCC) 10-win32 20220113
> Copyright (C) 2020 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> 
> GNU ld (GNU Binutils) 2.37
> Copyright (C) 2021 Free Software Foundation, Inc.
> This program is free software; you may redistribute it under the terms of
> the GNU General Public License version 3 or (at your option) a later version.
> This program has absolutely no warranty.
> 
> i686-w64-mingw32-gcc (GCC) 6.3.0 20170516
> Copyright (C) 2016 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> 
> GNU ld (GNU Binutils) 2.28
> Copyright (C) 2017 Free Software Foundation, Inc.
> This program is free software; you may redistribute it under the terms of
> the GNU General Public License version 3 or (at your option) a later version.
> This program has absolutely no warranty.
> 
> Daniel

Re: [PATCH] gdb: Add malloc and free symbols to kernel.exec to improve gdb functionality

[Daniel Kiper]

On Tue, Mar 15, 2022 at 02:36:12PM -0500, Glenn Washburn wrote:
> On Tue, 15 Mar 2022 14:23:48 +0100
> Daniel Kiper <dkiper@net-space.pl> wrote:
>
> > On Fri, Mar 11, 2022 at 12:09:58AM +0100, Daniel Kiper wrote:
> > > On Wed, Mar 09, 2022 at 02:25:28PM -0600, Glenn Washburn wrote:
> > > > On Wed, 9 Mar 2022 16:49:57 +0100
> > > > Daniel Kiper <dkiper@net-space.pl> wrote:
> > > >
> > > > > On Wed, Mar 02, 2022 at 06:25:12PM -0600, Glenn Washburn wrote:
> > > > > > Add linker flags when linking kernel.exec to have malloc and free point to
> > > > > > grub_malloc and grub_free respectively. Some gdb functionality depends on
> > > > > > gdb locating the symbols "malloc" and "free", such as dynamically creating
> > > > > > strings for arguments to injected function calls. A trivial example would
> > > > > > the gdb command 'p strlen("astring")'.
> > > > > >
> > > > > > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > > > > > ---
> > > > > > This should have been included in the gdb patch series I recently sent,
> > > > > > although its not required by nor requires any of those patches.
> > > > > >
> > > > > > Glenn
> > > > > >
> > > > > > ---
> > > > > >  conf/Makefile.common | 1 +
> > > > > >  1 file changed, 1 insertion(+)
> > > > > >
> > > > > > diff --git a/conf/Makefile.common b/conf/Makefile.common
> > > > > > index f0bb6e160a..069b428c1a 100644
> > > > > > --- a/conf/Makefile.common
> > > > > > +++ b/conf/Makefile.common
> > > > > > @@ -36,6 +36,7 @@ BUILD_CPPFLAGS += $(CPPFLAGS_DEFAULT)
> > > > > >
> > > > > >  CFLAGS_KERNEL = $(CFLAGS_PLATFORM) -ffreestanding
> > > > > >  LDFLAGS_KERNEL = $(LDFLAGS_PLATFORM) -nostdlib $(TARGET_LDFLAGS_OLDMAGIC)
> > > > > > +LDFLAGS_KERNEL += -Wl,--defsym=malloc=grub_malloc -Wl,--defsym=free=grub_free
> > > > >
> > > > > Could not we teach gdb somehow to use grub_malloc()/grub_free() instead
> > > > > of malloc()/free()?
> > > >
> > > > Considering the tons of options that GDB has, I was hoping the same
> > > > thing. Unfortunately, it appears to be hardcoded[1]. So not without
> > > > changing the source.
> > >
> > > :-( I expected you checked that but wanted to be sure... :-)
> > >
> > > Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
> >
> > Sadly this patch breaks Windows builds... :-(
> >
> > i686-w64-mingw32-gcc -std=gnu99 -fno-common -Os -m32 -Wall -W -Wshadow -Wpointer-arith -Wundef -Wchar-subscripts -Wcomment -Wdeprecated-declarations -Wdisabled-optimization -Wdiv-by-zero -Wfloat-equal -Wformat-extra-args -Wformat-security -Wformat-y2k -Wimplicit -Wimplicit-function-declaration -Wimplicit-int -Wmain -Wmissing-braces -Wmissing-format-attribute -Wmultichar -Wparentheses -Wreturn-type -Wsequence-point -Wshadow -Wsign-compare -Wswitch -Wtrigraphs -Wunknown-pragmas -Wunused -Wunused-function -Wunused-label -Wunused-parameter -Wunused-value  -Wunused-variable -Wwrite-strings -Wnested-externs -Wstrict-prototypes -g -Wredundant-decls -Wmissing-prototypes -Wmissing-declarations  -Wextra -Wattributes -Wendif-labels -Winit-self -Wint-to-pointer-cast -Winvalid-pch -Wmissing-field-initializers -Wnonnull -Woverflow -Wvla -Wpointer-to-int-cast -Wstrict-aliasing -Wvariadic-macros -Wvolatile-register-var -Wpointer-sign -Wmissing-include-dirs -Wmissing-prototypes -Wmissing-declarations -Wformat=2 -march=i386 -falign-functions=1 -falign-loops=1 -falign-jumps=1 -freg-struct-return -mno-mmx -mno-sse -mno-sse2 -mno-sse3 -mno-3dnow -msoft-float -fno-dwarf2-cfi-asm -fno-reorder-functions -mno-stack-arg-probe -fno-asynchronous-unwind-tables -fno-unwind-tables -fno-ident -mstack-protector-guard=global -fstack-protector -Wtrampolines -Werror   -ffreestanding   -m32 -Wl,-mi386pe  -nostdlib -Wl,-N -Wl,--defsym=malloc=grub_malloc -Wl,--defsym=free=grub_free -Wl,-r   -o kernel.exec.exe kern/i386/efi/kernel_exec-startup.o kern/i386/efi/kernel_exec-tsc.o kern/i386/kernel_exec-tsc_pmtimer.o kern/i386/efi/kernel_exec-init.o bus/kernel_exec-pci.o kern/i386/kernel_exec-dl.o kern/i386/kernel_exec-tsc.o kern/i386/kernel_exec-tsc_pit.o disk/efi/kernel_exec-efidisk.o kern/efi/kernel_exec-efi.o kern/efi/kernel_exec-init.o kern/efi/kernel_exec-mm.o term/efi/kernel_exec-console.o kern/kernel_exec-acpi.o kern/efi/kernel_exec-acpi.o kern/efi/kernel_exec-sb.o kern/kernel_exec-lockdown.o kern/kernel_exec-compiler-rt.o kern/kernel_exec-mm.o kern/kernel_exec-time.o kern/generic/kernel_exec-millisleep.o kern/kernel_exec-buffer.o kern/kernel_exec-command.o kern/kernel_exec-corecmd.o kern/kernel_exec-device.o kern/kernel_exec-disk.o kern/kernel_exec-dl.o kern/kernel_exec-env.o kern/kernel_exec-err.o kern/kernel_exec-file.o kern/kernel_exec-fs.o kern/kernel_exec-list.o kern/kernel_exec-main.o kern/kernel_exec-misc.o kern/kernel_exec-parser.o kern/kernel_exec-partition.o kern/kernel_exec-rescue_parser.o kern/kernel_exec-rescue_reader.o kern/kernel_exec-term.o kern/kernel_exec-verifiers.o kernel_exec-symlist.o
> > --defsym:1: undefined symbol `grub_malloc' referenced in expression
> > collect2: error: ld returned 1 exit status
> > Makefile:29017: recipe for target 'kernel.exec.exe' failed
>
> I'll look into it. At first glance, I had to say its odd that there is
> no grub_malloc symbol found as this must be exported for modules to
> use.
>
> Also, it looks like you're building for i386-efi, which seems strange.
> Do you get this error with both x86_64-efi and i386-pc? Regardless, I'll

I get this error for i386-efi and i386-pc but not for x86_64-efi.
It looks this work for i386-efi and i386-pc:

diff --git a/conf/Makefile.common b/conf/Makefile.common
index b343a038e..e278b3bd6 100644
--- a/conf/Makefile.common
+++ b/conf/Makefile.common
@@ -36,6 +36,7 @@ BUILD_CPPFLAGS += $(CPPFLAGS_DEFAULT)

 CFLAGS_KERNEL = $(CFLAGS_PLATFORM) -ffreestanding
 LDFLAGS_KERNEL = $(LDFLAGS_PLATFORM) -nostdlib $(TARGET_LDFLAGS_OLDMAGIC)
+LDFLAGS_KERNEL += -Wl,--defsym=_malloc=_grub_malloc -Wl,--defsym=_free=_grub_free
 CPPFLAGS_KERNEL = $(CPPFLAGS_CPU) $(CPPFLAGS_PLATFORM) -DGRUB_KERNEL=1
 CCASFLAGS_KERNEL = $(CCASFLAGS_CPU) $(CCASFLAGS_PLATFORM)
 STRIPFLAGS_KERNEL = -R .rel.dyn -R .reginfo -R .note -R .comment -R .drectve -R .note.gnu.gold-version -R .MIPS.abiflags -R .ARM.exidx

So, we need some ifdefery here.

> look in to replicating it on a windows cross-compiler on Linux. Also,
> it looks like the build machine is windows, as opposed to on Linux
> with a wnidows cross compiler. What exactly was your build and host
> machine type as detected by configure?

I cross compile GRUB for Windows on Debian. If you want to make the same just install
mingw-w64 and mingw-w64-tools packages and run following configure commands:
  - ./configure --target=x86_64-w64-mingw32 --with-platform=efi --host=x86_64-w64-mingw32 --enable-stack-protector
  - ./configure --target=i686-w64-mingw32 --with-platform=efi --host=i686-w64-mingw32 --enable-stack-protector
  - ./configure --target=i686-w64-mingw32 --with-platform=pc --host=i686-w64-mingw32

The make command works as usual.

Of course --enable-stack-protector is optional but I enable it.

> Would you find it acceptable to only conditionally include those
> arguments? I'm thinking excluding builds where the build or host are

Yep.

> windows environments. I'd be surprised to hear anyone using GDB on
> windows to debug GRUB.

Yeah, that would be interesting but why not... :-)

> BTW, as you can probably tell, I don't test building for windows and
> probably should. Although, I don't want to do it where the build
> machine is windows (I think its a good idea, I just don't want to work
> on that).

No worries, I am not going to ask you to install Windows... :-)

Daniel

Re: [PATCH] gdb: Add malloc and free symbols to kernel.exec to improve gdb functionality

[Glenn Washburn]

On Wed, 16 Mar 2022 21:59:18 +0100
Daniel Kiper <dkiper@net-space.pl> wrote:

> On Tue, Mar 15, 2022 at 02:36:12PM -0500, Glenn Washburn wrote:
> > On Tue, 15 Mar 2022 14:23:48 +0100
> > Daniel Kiper <dkiper@net-space.pl> wrote:
> >  
> > > On Fri, Mar 11, 2022 at 12:09:58AM +0100, Daniel Kiper wrote:  
> > > > On Wed, Mar 09, 2022 at 02:25:28PM -0600, Glenn Washburn wrote:  
> > > > > On Wed, 9 Mar 2022 16:49:57 +0100
> > > > > Daniel Kiper <dkiper@net-space.pl> wrote:
> > > > >  
> > > > > > On Wed, Mar 02, 2022 at 06:25:12PM -0600, Glenn Washburn wrote:  
> > > > > > > Add linker flags when linking kernel.exec to have malloc and free point to
> > > > > > > grub_malloc and grub_free respectively. Some gdb functionality depends on
> > > > > > > gdb locating the symbols "malloc" and "free", such as dynamically creating
> > > > > > > strings for arguments to injected function calls. A trivial example would
> > > > > > > the gdb command 'p strlen("astring")'.
> > > > > > >
> > > > > > > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > > > > > > ---
> > > > > > > This should have been included in the gdb patch series I recently sent,
> > > > > > > although its not required by nor requires any of those patches.
> > > > > > >
> > > > > > > Glenn
> > > > > > >
> > > > > > > ---
> > > > > > >  conf/Makefile.common | 1 +
> > > > > > >  1 file changed, 1 insertion(+)
> > > > > > >
> > > > > > > diff --git a/conf/Makefile.common b/conf/Makefile.common
> > > > > > > index f0bb6e160a..069b428c1a 100644
> > > > > > > --- a/conf/Makefile.common
> > > > > > > +++ b/conf/Makefile.common
> > > > > > > @@ -36,6 +36,7 @@ BUILD_CPPFLAGS += $(CPPFLAGS_DEFAULT)
> > > > > > >
> > > > > > >  CFLAGS_KERNEL = $(CFLAGS_PLATFORM) -ffreestanding
> > > > > > >  LDFLAGS_KERNEL = $(LDFLAGS_PLATFORM) -nostdlib $(TARGET_LDFLAGS_OLDMAGIC)
> > > > > > > +LDFLAGS_KERNEL += -Wl,--defsym=malloc=grub_malloc -Wl,--defsym=free=grub_free  
> > > > > >
> > > > > > Could not we teach gdb somehow to use grub_malloc()/grub_free() instead
> > > > > > of malloc()/free()?  
> > > > >
> > > > > Considering the tons of options that GDB has, I was hoping the same
> > > > > thing. Unfortunately, it appears to be hardcoded[1]. So not without
> > > > > changing the source.  
> > > >
> > > > :-( I expected you checked that but wanted to be sure... :-)
> > > >
> > > > Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>  
> > >
> > > Sadly this patch breaks Windows builds... :-(
> > >
> > > i686-w64-mingw32-gcc -std=gnu99 -fno-common -Os -m32 -Wall -W -Wshadow -Wpointer-arith -Wundef -Wchar-subscripts -Wcomment -Wdeprecated-declarations -Wdisabled-optimization -Wdiv-by-zero -Wfloat-equal -Wformat-extra-args -Wformat-security -Wformat-y2k -Wimplicit -Wimplicit-function-declaration -Wimplicit-int -Wmain -Wmissing-braces -Wmissing-format-attribute -Wmultichar -Wparentheses -Wreturn-type -Wsequence-point -Wshadow -Wsign-compare -Wswitch -Wtrigraphs -Wunknown-pragmas -Wunused -Wunused-function -Wunused-label -Wunused-parameter -Wunused-value  -Wunused-variable -Wwrite-strings -Wnested-externs -Wstrict-prototypes -g -Wredundant-decls -Wmissing-prototypes -Wmissing-declarations  -Wextra -Wattributes -Wendif-labels -Winit-self -Wint-to-pointer-cast -Winvalid-pch -Wmissing-field-initializers -Wnonnull -Woverflow -Wvla -Wpointer-to-int-cast -Wstrict-aliasing -Wvariadic-macros -Wvolatile-register-var -Wpointer-sign -Wmissing-include-dirs -Wmissing-prototypes -Wmissing-declarations -Wformat=2 -march=i386 -falign-functions=1 -falign-loops=1 -falign-jumps=1 -freg-struct-return -mno-mmx -mno-sse -mno-sse2 -mno-sse3 -mno-3dnow -msoft-float -fno-dwarf2-cfi-asm -fno-reorder-functions -mno-stack-arg-probe -fno-asynchronous-unwind-tables -fno-unwind-tables -fno-ident -mstack-protector-guard=global -fstack-protector -Wtrampolines -Werror   -ffreestanding   -m32 -Wl,-mi386pe  -nostdlib -Wl,-N -Wl,--defsym=malloc=grub_malloc -Wl,--defsym=free=grub_free -Wl,-r   -o kernel.exec.exe kern/i386/efi/kernel_exec-startup.o kern/i386/efi/kernel_exec-tsc.o kern/i386/kernel_exec-tsc_pmtimer.o kern/i386/efi/kernel_exec-init.o bus/kernel_exec-pci.o kern/i386/kernel_exec-dl.o kern/i386/kernel_exec-tsc.o kern/i386/kernel_exec-tsc_pit.o disk/efi/kernel_exec-efidisk.o kern/efi/kernel_exec-efi.o kern/efi/kernel_exec-init.o kern/efi/kernel_exec-mm.o term/efi/kernel_exec-console.o kern/kernel_exec-acpi.o kern/efi/kernel_exec-acpi.o kern/efi/kernel_exec-sb.o kern/kernel_exec-lockdown.o kern/kernel_exec-compiler-rt.o kern/kernel_exec-mm.o kern/kernel_exec-time.o kern/generic/kernel_exec-millisleep.o kern/kernel_exec-buffer.o kern/kernel_exec-command.o kern/kernel_exec-corecmd.o kern/kernel_exec-device.o kern/kernel_exec-disk.o kern/kernel_exec-dl.o kern/kernel_exec-env.o kern/kernel_exec-err.o kern/kernel_exec-file.o kern/kernel_exec-fs.o kern/kernel_exec-list.o kern/kernel_exec-main.o kern/kernel_exec-misc.o kern/kernel_exec-parser.o kern/kernel_exec-partition.o kern/kernel_exec-rescue_parser.o kern/kernel_exec-rescue_reader.o kern/kernel_exec-term.o kern/kernel_exec-verifiers.o kernel_exec-symlist.o
> > > --defsym:1: undefined symbol `grub_malloc' referenced in expression
> > > collect2: error: ld returned 1 exit status
> > > Makefile:29017: recipe for target 'kernel.exec.exe' failed  
> >
> > I'll look into it. At first glance, I had to say its odd that there is
> > no grub_malloc symbol found as this must be exported for modules to
> > use.
> >
> > Also, it looks like you're building for i386-efi, which seems strange.
> > Do you get this error with both x86_64-efi and i386-pc? Regardless, I'll  
> 
> I get this error for i386-efi and i386-pc but not for x86_64-efi.
> It looks this work for i386-efi and i386-pc:
> 
> diff --git a/conf/Makefile.common b/conf/Makefile.common
> index b343a038e..e278b3bd6 100644
> --- a/conf/Makefile.common
> +++ b/conf/Makefile.common
> @@ -36,6 +36,7 @@ BUILD_CPPFLAGS += $(CPPFLAGS_DEFAULT)
> 
>  CFLAGS_KERNEL = $(CFLAGS_PLATFORM) -ffreestanding
>  LDFLAGS_KERNEL = $(LDFLAGS_PLATFORM) -nostdlib $(TARGET_LDFLAGS_OLDMAGIC)
> +LDFLAGS_KERNEL += -Wl,--defsym=_malloc=_grub_malloc -Wl,--defsym=_free=_grub_free

Thanks, this helps a lot.

>  CPPFLAGS_KERNEL = $(CPPFLAGS_CPU) $(CPPFLAGS_PLATFORM) -DGRUB_KERNEL=1
>  CCASFLAGS_KERNEL = $(CCASFLAGS_CPU) $(CCASFLAGS_PLATFORM)
>  STRIPFLAGS_KERNEL = -R .rel.dyn -R .reginfo -R .note -R .comment -R .drectve -R .note.gnu.gold-version -R .MIPS.abiflags -R .ARM.exidx
> 
> So, we need some ifdefery here.
> 
> > look in to replicating it on a windows cross-compiler on Linux. Also,
> > it looks like the build machine is windows, as opposed to on Linux
> > with a wnidows cross compiler. What exactly was your build and host
> > machine type as detected by configure?  
> 
> I cross compile GRUB for Windows on Debian. If you want to make the same just install
> mingw-w64 and mingw-w64-tools packages and run following configure commands:
>   - ./configure --target=x86_64-w64-mingw32 --with-platform=efi --host=x86_64-w64-mingw32 --enable-stack-protector
>   - ./configure --target=i686-w64-mingw32 --with-platform=efi --host=i686-w64-mingw32 --enable-stack-protector
>   - ./configure --target=i686-w64-mingw32 --with-platform=pc --host=i686-w64-mingw32

Also very helpful. I've integrated the windows build into my test scripts.

> The make command works as usual.
> 
> Of course --enable-stack-protector is optional but I enable it.
> 
> > Would you find it acceptable to only conditionally include those
> > arguments? I'm thinking excluding builds where the build or host are  
> 
> Yep.
> 
> > windows environments. I'd be surprised to hear anyone using GDB on
> > windows to debug GRUB.  
> 
> Yeah, that would be interesting but why not... :-)

Actually, with your help, I figured out how to not have to exclude windows
builds, although I can only build test it.

Glenn

Re: [PATCH] gdb: Add malloc and free symbols to kernel.exec to improve gdb functionality

[Daniel Kiper]

On Fri, Mar 18, 2022 at 01:57:39AM -0500, Glenn Washburn wrote:

[...]

> Actually, with your help, I figured out how to not have to exclude windows
> builds, although I can only build test it.

I think that is enough for now. We can think about testing later...

Daniel