grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Glenn Washburn
Subject: Re: [PATCH] gdb: Add malloc and free symbols to kernel.exec to improve gdb functionality
Date: Fri, 18 Mar 2022 01:57:39 -0500

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]