Skip to contentSkip to conversation

Remote GDB debugging stub (for i386)

PATCH In review Lubomir Rintel 3 participants 7 messages May 1, 2008 – May 16, 2008

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Remote GDB debugging stub (for i386)

Re-sending the GDB stub patch. The GDB protocol part, though being largely rewritten, originates from GDB source tarball, which refers to it as a Public Domain code.

Applies against today's CVS.

Is not much of use without debugging symbols, part 2 of patch [1] preserves those. Neither is it of much use without being able to resolve load addresses of modules, I will submit the GDB scripts shortly.

[1] http://lists.gnu.org/archive/html/grub-devel/2008-04/msg00080.html

Note that I found this hunk [2] lying around from 2006 when I wrote this on NetBSD. I am not completely sure what does it do, but the changelog from then suggests, that "grub_gdb_trapvec" from the gdb module was for some reason resolved to 0 instead of its actual address. It might have been dependent on the toolchain used, as I am not able to reproduce the behavior on current Fedora 9.

[2] http://pastebin.com/f5c791966

2008-05-01: Lubomir Rintel <[email protected]>

* conf/i386-pc.rmk: Add gdb module to the list of modules to be built.

* term/i386/pc/serial.c (serial_hw_put): Let some useful serial driver functions be exported.
(grub_serial_getkey): Likewise

* kern/dl.c (grub_dl_resolve_symbols): Let the trapvec symbol be resolved correctly (instead of 0).

* kern/i386/pc/startup.S: (prot_idt) Add region descriptor to hold protected mode IDT.
(real_idt) IDT region dsecriptor that works in real mode (real_to_prot) Load protected mode idt IDT when returning to protected mode.

* kern/i386/realmode.S: (prot_to_real) Save protected mode IDT and load real mode one before switching to real mode.

* gdb/cstub.c: New file.
* gdb/gdb.c: New file.
* gdb/i386/idt.c: New file.
* gdb/i386/signal.c: New file.
* gdb/i386/machdep.S: New file.
* include/grub/gdb.h: New file.
* include/grub/i386/gdb.h: New file.
* include/grub/i386/pc/kernel.h: New file.

--
Lubomir Rintel (Was: Kundrak)

2008-05-01: Lubomir Rintel <[email protected]>

* conf/i386-pc.rmk: Add gdb module to the list of modules to be built.

* term/i386/pc/serial.c (serial_hw_put): Let some useful serial driver functions be exported.
(grub_serial_getkey): Likewise

* kern/dl.c (grub_dl_resolve_symbols): Let the trapvec symbol be resolved correctly (instead of 0).

* kern/i386/pc/startup.S: (prot_idt) Add region descriptor to hold protected mode IDT.
(real_idt) IDT region dsecriptor that works in real mode (real_to_prot) Load protected mode idt IDT when returning to protected mode.

* kern/i386/realmode.S: (prot_to_real) Save protected mode IDT and load real mode one before switching to real mode.

* gdb/cstub.c: New file.
* gdb/gdb.c: New file.
* gdb/i386/idt.c: New file.
* gdb/i386/signal.c: New file.
* gdb/i386/machdep.S: New file.
* include/grub/gdb.h: New file.
* include/grub/i386/gdb.h: New file.
* include/grub/i386/pc/kernel.h: New file.

conf/i386-pc.rmk +8 -1
@@ -155,7 +155,7 @@ pkglib_MODULES = biosdisk.mod _chain.mod
 	vbe.mod vbetest.mod vbeinfo.mod video.mod gfxterm.mod \
 	videotest.mod play.mod bitmap.mod tga.mod cpuid.mod serial.mod	\
 	ata.mod vga.mod memdisk.mod jpeg.mod png.mod pci.mod lspci.mod \
-	aout.mod _bsd.mod bsd.mod
+	aout.mod _bsd.mod bsd.mod gdb.mod
 
 # For biosdisk.mod.
 biosdisk_mod_SOURCES = disk/i386/pc/biosdisk.c
@@ -322,4 +322,11 @@ bsd_mod_SOURCES = loader/i386/bsd_normal
 bsd_mod_CFLAGS = $(COMMON_CFLAGS)
 bsd_mod_LDFLAGS = $(COMMON_LDFLAGS)
 
+# For gdb.mod
+gdb_mod_SOURCES = gdb/cstub.c gdb/gdb.c gdb/i386/idt.c \
+	gdb/i386/machdep.S gdb/i386/signal.c
+gdb_mod_CFLAGS = $(COMMON_CFLAGS)
+gdb_mod_LDFLAGS = $(COMMON_LDFLAGS)
+gdb_mod_ASFLAGS = $(COMMON_ASFLAGS)
+
 include $(srcdir)/conf/common.mk
gdb/cstub.c +362 -0

362 lines not on this page · raw message

gdb/gdb.c +61 -0

61 lines not on this page · raw message

gdb/i386/idt.c +61 -0

61 lines not on this page · raw message

gdb/i386/machdep.S +193 -0

193 lines not on this page · raw message

gdb/i386/signal.c +53 -0

53 lines not on this page · raw message

genmk.rb +7 -3
@@ -101,10 +101,11 @@ class PModule
     mod_obj = mod_src.suffix('o')
     defsym = 'def-' + @name.suffix('lst')
     undsym = 'und-' + @name.suffix('lst')
+    exec = @name.suffix('elf')
     mod_name = File.basename(@name, '.mod')
     symbolic_name = mod_name.sub(/\.[^\.]*$/, '')
     
-    "CLEANFILES += #{@name} #{mod_obj} #{mod_src} #{pre_obj} #{objs_str} #{undsym}
+    "CLEANFILES += #{@name} #{mod_obj} #{mod_src} #{pre_obj} #{objs_str} #{undsym} #{exec}
 ifneq ($(#{prefix}_EXPORTS),no)
 CLEANFILES += #{defsym}
 DEFSYMFILES += #{defsym}
@@ -112,10 +113,13 @@ endif
 MOSTLYCLEANFILES += #{deps_str}
 UNDSYMFILES += #{undsym}
 
-#{@name}: #{pre_obj} #{mod_obj}
+#{@name}: #{exec}
+	-rm -f $@
+	$(OBJCOPY) --strip-unneeded -K grub_mod_init -K grub_mod_fini -R .note -R .comment $^ $@
+
+#{exec}: #{pre_obj} #{mod_obj}
 	-rm -f $@
 	$(TARGET_CC) $(#{prefix}_LDFLAGS) $(TARGET_LDFLAGS) -Wl,-r,-d -o $@ $^
-	$(STRIP) --strip-unneeded -K grub_mod_init -K grub_mod_fini -R .note -R .comment $@
 
 #{pre_obj}: $(#{prefix}_DEPENDENCIES) #{objs_str}
 	-rm -f $@
include/grub/gdb.h +38 -0
@@ -0,0 +1,38 @@
+/* gdb.h - Various definitions for the remote GDB stub */
+/*
+ *  Copyright (C) 2006  Lubomir Kundrak
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#ifndef GRUB_GDB_HEADER
+#define GRUB_GDB_HEADER		1
+
+#define SIGFPE		8
+#define SIGTRAP		5
+#define SIGABRT		6
+#define SIGSEGV		11
+#define SIGILL		4
+#define SIGUSR1		30
+/* We probably don't need other ones.  */
+
+extern int (*grub_gdb_getchar) ();
+extern void (*grub_gdb_putchar) (int);
+
+void grub_gdb_breakpoint ();
+int grub_gdb_trap2sig (int);
+
+#endif /* ! GRUB_GDB_HEADER */
+
include/grub/i386/gdb.h +80 -0

80 lines not on this page · raw message

include/grub/i386/pc/kernel.h +1 -1
@@ -44,7 +44,7 @@
 #define GRUB_KERNEL_MACHINE_DATA_END		0x50
 
 /* The size of the first region which won't be compressed.  */
-#define GRUB_KERNEL_MACHINE_RAW_SIZE		0x4A0
+#define GRUB_KERNEL_MACHINE_RAW_SIZE		0x4BC
 
 #ifndef ASM_FILE
 
kern/i386/pc/startup.S +12 -1
@@ -276,6 +276,15 @@ VARIABLE(grub_apm_bios_info)
 	.word	0	/* cseg_len */
 	.word	0	/* cseg_16_len */
 	.word	0	/* dseg_16_len */
+
+/* IDT region descriptors */
+VARIABLE(prot_idt)
+	.word	0xffff			/* limit */
+	.long	0x0			/* addr */
+
+VARIABLE(real_idt)
+	.word	0xffff			/* limit */
+	.long	0x0			/* addr */
 	
 	.p2align	2	/* force 4-byte alignment */
 	
@@ -327,6 +336,9 @@ protcseg:
 	/* zero %eax */
 	xorl	%eax, %eax
 
+	/* real mode IDT is no longer pertinent */
+	lidt prot_idt
+
 	/* return on the old (or initialized) stack! */
 	ret
 
@@ -1097,7 +1109,6 @@ xsmap:
 	popl	%ebp
 	ret
 
-	
 /*
  * void grub_console_real_putchar (int c)
  *
kern/i386/realmode.S +4 -0
@@ -120,6 +120,10 @@ prot_to_real:
 	/* just in case, set GDT */
 	lgdt	gdtdesc
 
+	/* our protected mode IDT would cause trouble in real mode */
+	sidt prot_idt
+	lidt real_idt
+
 	/* save the protected mode stack */
 	movl	%esp, %eax
 	movl	%eax, protstack
term/i386/pc/serial.c +3 -3
@@ -79,7 +79,7 @@ serial_hw_get_port (const unsigned short
 }
 
 /* Fetch a key.  */
-static int
+int
 serial_hw_fetch (void)
 {
   if (grub_inb (serial_settings.port + UART_LSR) & UART_DATA_READY)
@@ -89,7 +89,7 @@ serial_hw_fetch (void)
 }
 
 /* Put a character.  */
-static void
+void
 serial_hw_put (const int c)
 {
   unsigned int timeout = 100000;
@@ -247,7 +247,7 @@ grub_serial_checkkey (void)
 }
 
 /* The serial version of getkey.  */
-static int
+int
 grub_serial_getkey (void)
 {
   int c;
#1 lore raw

gdb/cstub.c line 3

Quoted 6 lines · Lubomir Rintel
gdb/cstub.c +3 -0
@@ -0,0 +1,362 @@
+/* cstub.c - machine independent portion of remote GDB stub */
+/*
+ *  Copyright (C) 2006  Lubomir Kundrak

Please put the FSF in copyright notice (since copyright is being assigned).

Also acknowledge whatever copyright/license notice there was in code copied from gdb (though if it was public domain, I think you can skip the latter).

gdb/cstub.c line 17

Quoted 13 lines
gdb/cstub.c +13 -0
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.

Please update to GPL 3. I suggest you copy the whole blob since there are improvements in the last paragraph.

gdb/cstub.c

Quoted 3 lines
gdb/cstub.c +1 -1
 /* The size of the first region which won't be compressed.  */
-#define GRUB_KERNEL_MACHINE_RAW_SIZE		0x4A0
+#define GRUB_KERNEL_MACHINE_RAW_SIZE		0x4BC

What caused this? Any way we can have this dynamically calculated?

Signature · 5 lines
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)
#2 lore raw
Quoted 6 lines · Robert Millan

include/grub/i386/pc/kernel.h line 47

Quoted 3 lines

/* The size of the first region which won't be compressed. */

include/grub/i386/pc/kernel.h +1 -1
-#define GRUB_KERNEL_MACHINE_RAW_SIZE		0x4A0
+#define GRUB_KERNEL_MACHINE_RAW_SIZE		0x4BC

What caused this? Any way we can have this dynamically calculated?

More code before compression point in boot code. Should be automated :)

#3 lore raw
In reply to Robert Millan
Quoted 10 lines · Robert Millan

gdb/cstub.c line 3

Quoted 6 lines · Lubomir Rintel
gdb/cstub.c +3 -0
@@ -0,0 +1,362 @@
+/* cstub.c - machine independent portion of remote GDB stub */
+/*
+ *  Copyright (C) 2006  Lubomir Kundrak

Please put the FSF in copyright notice (since copyright is being assigned).

Quoted 2 lines

Also acknowledge whatever copyright/license notice there was in code copied from gdb (though if it was public domain, I think you can skip the latter).

Done.

Quoted 2 lines

Please update to GPL 3. I suggest you copy the whole blob since there are improvements in the last paragraph.

Done

Quoted 5 lines

gdb/cstub.c

Quoted 3 lines
gdb/cstub.c +1 -1
 /* The size of the first region which won't be compressed.  */
-#define GRUB_KERNEL_MACHINE_RAW_SIZE		0x4A0
+#define GRUB_KERNEL_MACHINE_RAW_SIZE		0x4BC

What caused this? Any way we can have this dynamically calculated?

Described in the changelog. No idea how could it be calculated, I just remember Okuji suggesting number that value to me when I was trouble loading IDT.

Signature · 2 lines
Lubomir Rintel (Was: Kundrak)
http://v3.sk/~lkundrak/
#4 lore raw

PATCH 1/1 GDB stuff updated

In reply to Lubomir Rintel

Updated GDB stub patch, just header text changes, changelog text fix and stripped of junk.

Also -- solved the mystery of the dl.c fix; for some reason I managed to generate a reversed diff. It is required to link the gdb module (symbols defined with .globl in assembler sources, IIRC)

I am not going to attach all the patches here.
All that's needed is here [1]:

grub2-dlsym-v4.patch
grub2-gdb-macros-v4.patch
grub2-gdb-stub-v4.patch
grub2-preserve-symbols-v4.patch

[1] http://fedorapeople.org/~lkundrak/grub2/

Signature · 2 lines
Lubomir Rintel (Was: Kundrak)
http://v3.sk/~lkundrak/
#5 lore raw

Re: [PATCH] GDB stuff updated

Hi Lubomir,

Quoted 9 lines · Lubomir Rintel

Updated GDB stub patch, just header text changes, changelog text fix and stripped of junk.

Also -- solved the mystery of the dl.c fix; for some reason I managed to generate a reversed diff. It is required to link the gdb module (symbols defined with .globl in assembler sources, IIRC)

I am not going to attach all the patches here.
All that's needed is here [1]:

Are those copyright years correct or result of copy paste error? Most of the files in this patch are new to GRUB 2 repository.

Quoted 1 line

grub2-dlsym-v4.patch

I am not big fan of fallthru's in switches. They can easily lead to programming errors. As code is rather short and I assume compiler will optimize it anyway, could you just make clean case for STT_NOTYPE without fallthru.

Quoted 1 line

grub2-gdb-macros-v4.patch

Should those be stored on folder like: contrib/gdb/

Quoted 1 line

grub2-gdb-stub-v4.patch

Do we want to support other communication mechanism than serial in future for GDB ? I am asking this because it could be prepared a bit with proper interfaces. That would also make code a bit more tidier.

+++ grub2-gdb/gdb/cstub.c 2008-05-07 10:15:52.000000000 +0200

+int (*grub_gdb_getchar) ();

Add void. Perhaps you could make typedef for it and declaring variable as static?

+static int
+hex (ch)
+ char ch;
+{

Let's be a bit more modern...

grub_gdb_getpacket (void):
It may be good idea to refer to documentation stating framing structure or adding comment about frame.

+++ grub2-gdb/gdb/gdb.c 2008-05-07 09:39:11.000000000 +0200

I do not really like this explicit attachment to serial device. Perhaps we should improve serial module and then make gdb to have communication interface.

+++ grub2-gdb/gdb/i386/idt.c 2008-05-07 09:39:11.000000000 +0200

Why not move this as own functional block to:
kern/i386/

Also removing trace for gdb :)

I think we need IDT for other features being planned.

+++ grub2-gdb/gdb/i386/machdep.S 2008-05-07 09:39:11.000000000 +0200
We do not do bunnies...

grub_idt_load: Why not move this to startup.S.

+++ grub2-gdb/include/grub/i386/gdb.h 2008-05-07 09:39:11.000000000 +0200

Seperate IDT stuff..

+++ grub2-gdb/include/grub/i386/pc/kernel.h 2008-05-07 09:39:11.000000000 +0200

+#define GRUB_KERNEL_MACHINE_RAW_SIZE 0x4BC

Ok... this has to be automated... lets see...

+++ grub2-gdb/kern/i386/pc/startup.S 2008-05-07 09:39:11.000000000 +0200

How about raving readmode IDT (or IDT what we started with) so we can recall it when we are calling BIOS services... just in case.

+++ grub2-gdb/term/i386/pc/serial.c 2008-05-07 09:39:11.000000000 +0200

Not really happy about this change. Lets try to figure out better interface...

Quoted 1 line

grub2-preserve-symbols-v4.patch

Ok... Needs fine tuning for changelog entry, but otherwise fine.

Quoted 3 lines

[1] http://fedorapeople.org/~lkundrak/grub2/

I hope those keep you busy for a while :)

Thanks,
Vesa Jääskeläinen

#6 lore raw

Re: [PATCH] GDB stuff updated

Quoted 3 lines · Vesa Jääskeläinen

+++ grub2-gdb/kern/i386/pc/startup.S 2008-05-07 09:39:11.000000000 +0200

How about raving readmode IDT (or IDT what we started with) so we can recall it when we are calling BIOS services... just in case.

That's saving real mode ;)