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