Skip to contentSkip to conversation

Make gdb debugging a bit less painful

PATCH In review Lubomir Kundrak 4 participants 16 messages Apr 14, 2008 – Apr 18, 2008

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Make gdb debugging a bit less painful

These are pretty self-explainable. I think I tried to sumbit these changes with the serial debugging patch year and some months ago and I was told to sign the copyright assignment. I have already done that now. :)

2008-04-15 Lubomir Kundrak <[email protected]>

* kern/main.c (grub_main) [!GRUB_UTIL]: Alias grub_main to main, so that gdb is able to find stack top.
* genmk.rb: replace strip with objcopy, so unstripped files are left untouched for debugging purposes.

Signature · 1 line
Lubomir Kundrak (Red Hat Security Response Team)
kern/main.c +8 -0
diff -rup grub-1.95.orig/kern/main.c grub-1.95/kern/main.c
@@ -106,6 +106,14 @@ grub_load_normal_mode (void)
   grub_errno = GRUB_ERR_NONE;
 }
 
+#ifndef GRUB_UTIL
+/*
+ * Let GDB find the stack top
+ */
+void
+main (void) __attribute__ ((alias("grub_main")));
+#endif
+
 /* The main routine.  */
 void
 grub_main (void)
genmk.rb +7 -3
@@ -101,10 +101,11 @@
     mod_obj = mod_src.suffix('o')
     defsym = 'def-' + @name.suffix('lst')
     undsym = 'und-' + @name.suffix('lst')
+    exec = @name.suffix('exec')
     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 @@
 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 -R .note.gnu.build-id $^ $@
+
+#{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 -R .note.gnu.build-id $@
 
 #{pre_obj}: $(#{prefix}_DEPENDENCIES) #{objs_str}
 	-rm -f $@
#1 lore raw
Quoted 11 lines · Lubomir Kundrak

These are pretty self-explainable. I think I tried to sumbit these changes with the serial debugging patch year and some months ago and I was told to sign the copyright assignment. I have already done that now. :)

2008-04-15 Lubomir Kundrak <[email protected]>

* kern/main.c (grub_main) [!GRUB_UTIL]: Alias grub_main to main, so that gdb is able to find stack top.
* genmk.rb: replace strip with objcopy, so unstripped files are left untouched for debugging purposes.

How do you use gdb with GRUB? QEMU? If you just fixed backtrace dumping with grub/qemu/gdb, that'd make me quite happy :-)

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
In reply to Lubomir Kundrak
Quoted 4 lines · Lubomir Kundrak

diff -rup grub-1.95.orig/kern/main.c grub-1.95/kern/main.c

kern/main.c grub_load_normal_mode (void) · line 115

Quoted 10 lines
kern/main.c +7 -0
@@ -106,6 +106,14 @@ grub_load_normal_mode (void)
   grub_errno = GRUB_ERR_NONE;
 }
 
+#ifndef GRUB_UTIL
+/*
+ * Let GDB find the stack top
+ */
+void
+main (void) __attribute__ ((alias("grub_main")));
+#endif

grub_main() isn't always the function we want for top of the stack. For example cmain() in kern/ieee1275/cmain.c does quite a bit of things already.

I'm not sure what would be an ellegant way to solve this... :-/

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 /.)
#3 lore raw
Quoted 19 lines · Robert Millan
Quoted 4 lines · Lubomir Kundrak

diff -rup grub-1.95.orig/kern/main.c grub-1.95/kern/main.c

kern/main.c grub_load_normal_mode (void) · line 115

Quoted 10 lines
kern/main.c +7 -0
@@ -106,6 +106,14 @@ grub_load_normal_mode (void)
   grub_errno = GRUB_ERR_NONE;
 }
 
+#ifndef GRUB_UTIL
+/*
+ * Let GDB find the stack top
+ */
+void
+main (void) __attribute__ ((alias("grub_main")));
+#endif

grub_main() isn't always the function we want for top of the stack. For example cmain() in kern/ieee1275/cmain.c does quite a bit of things already.

Quoted 1 line

I'm not sure what would be an ellegant way to solve this... :-/

Well, then more preprocessor conditionals come into play. Wold there be any problems with that?

Signature · 1 line
Lubomir Kundrak (Red Hat Security Response Team)
#4 lore raw
Quoted 21 lines · Lubomir Kundrak
Quoted 19 lines · Robert Millan
Quoted 4 lines · Lubomir Kundrak

diff -rup grub-1.95.orig/kern/main.c grub-1.95/kern/main.c

kern/main.c grub_load_normal_mode (void) · line 115

Quoted 10 lines
kern/main.c +7 -0
@@ -106,6 +106,14 @@ grub_load_normal_mode (void)
   grub_errno = GRUB_ERR_NONE;
 }
 
+#ifndef GRUB_UTIL
+/*
+ * Let GDB find the stack top
+ */
+void
+main (void) __attribute__ ((alias("grub_main")));
+#endif

grub_main() isn't always the function we want for top of the stack. For example cmain() in kern/ieee1275/cmain.c does quite a bit of things already.

Quoted 4 lines
Quoted 1 line

I'm not sure what would be an ellegant way to solve this... :-/

Well, then more preprocessor conditionals come into play. Wold there be any problems with that?

Now that I think of it, is there any reason for doing all the cmain() cruft before grub_main() ? Getting to grub_main() only requires a working stack, which we have, and grub_main() inmediately returns control back to architecture-specific code in grub_machine_init().

So why not move from:

_start -> cmain -> grub_main

to:

_start -> grub_main -> grub_machine_init -> cmain

?

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 /.)
#5 lore raw

Quoting Robert Millan <[email protected]>:

Quoted 7 lines

Now that I think of it, is there any reason for doing all the cmain() cruft before grub_main() ? Getting to grub_main() only requires a working stack, which we have, and grub_main() inmediately returns control back to architecture-specific code in grub_machine_init().

So why not move from:

Quoted 7 lines

_start -> cmain -> grub_main

to:

_start -> grub_main -> grub_machine_init -> cmain

?

I think it would be great. We should try to make the code for different architectures more uniform to simplify maintenance.

Signature · 2 lines
Regards,
Pavel Roskin
#6 lore raw
Quoted 11 lines · Pavel Roskin

Quoting Robert Millan <[email protected]>:

Quoted 9 lines

Now that I think of it, is there any reason for doing all the cmain() cruft before grub_main() ? Getting to grub_main() only requires a working stack, which we have, and grub_main() inmediately returns control back to architecture-specific code in grub_machine_init().

So why not move from:

_start -> cmain -> grub_main

Quoted 8 lines
Quoted 5 lines

to:

_start -> grub_main -> grub_machine_init -> cmain

?

I think it would be great. We should try to make the code for different architectures more uniform to simplify maintenance.

Here. Unfortunately I can't test it as I have no ieee1275 at hand currently.

Anyone?

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 /.)
include/grub/ieee1275/ieee1275.h renamed from grub2/include/grub/ieee1275/ieee1275.h +1 -0
diff -x configure -x config.h.in -x CVS -x '*~' -x '*.mk' -urp ../grub2/include/grub/ieee1275/ieee1275.h ./include/grub/ieee1275/ieee1275.h
@@ -97,6 +97,7 @@ extern void EXPORT_FUNC(grub_ieee1275_se
 
 
 
+void EXPORT_FUNC(grub_ieee1275_init) (void);
 int EXPORT_FUNC(grub_ieee1275_finddevice) (char *name,
 					   grub_ieee1275_phandle_t *phandlep);
 int EXPORT_FUNC(grub_ieee1275_get_property) (grub_ieee1275_phandle_t phandle,
kern/i386/ieee1275/startup.S renamed from grub2/kern/i386/ieee1275/startup.S +1 -1
diff -x configure -x config.h.in -x CVS -x '*~' -x '*.mk' -urp ../grub2/kern/i386/ieee1275/startup.S ./kern/i386/ieee1275/startup.S
@@ -39,7 +39,7 @@
 start:
 _start:
 	movl %eax, EXT_C(grub_ieee1275_entry_fn)
-	jmp EXT_C(cmain)
+	jmp EXT_C(grub_main)
 
 /*
  *  This call is special...  it never returns...  in fact it should simply
kern/ieee1275/cmain.c renamed from grub2/kern/ieee1275/cmain.c +1 -7
diff -x configure -x config.h.in -x CVS -x '*~' -x '*.mk' -urp ../grub2/kern/ieee1275/cmain.c ./kern/ieee1275/cmain.c
@@ -151,9 +151,8 @@ grub_ieee1275_find_options (void)
 #undef SF
 #undef OHW
 
-void cmain (void);
 void
-cmain (void)
+grub_ieee1275_init (void)
 {
   grub_ieee1275_finddevice ("/chosen", &grub_ieee1275_chosen);
 
@@ -162,9 +161,4 @@ cmain (void)
     grub_ieee1275_mmu = 0;
 
   grub_ieee1275_find_options ();
-
-  /* Now invoke the main function.  */
-  grub_main ();
-
-  /* Never reached.  */
 }
kern/ieee1275/init.c renamed from grub2/kern/ieee1275/init.c +2 -0
diff -x configure -x config.h.in -x CVS -x '*~' -x '*.mk' -urp ../grub2/kern/ieee1275/init.c ./kern/ieee1275/init.c
@@ -180,6 +180,8 @@ grub_machine_init (void)
   char args[256];
   int actual;
 
+  grub_ieee1275_init ();
+
   grub_console_init ();
 #ifdef __i386__
   grub_keyboard_controller_init ();
kern/powerpc/ieee1275/crt0.S renamed from grub2/kern/powerpc/ieee1275/crt0.S +1 -1
diff -x configure -x config.h.in -x CVS -x '*~' -x '*.mk' -urp ../grub2/kern/powerpc/ieee1275/crt0.S ./kern/powerpc/ieee1275/crt0.S
@@ -42,5 +42,5 @@ _start:	
 	lis	9, grub_ieee1275_entry_fn@ha
 	stw	5, grub_ieee1275_entry_fn@l(9)
 
-	bl	cmain
+	bl	grub_main
 1:	b	1b
#7 lore raw

kern/i386/ieee1275/startup.S line 42

Quoted 2 lines · Robert Millan
kern/i386/ieee1275/startup.S renamed from grub2/kern/i386/ieee1275/startup.S +1 -1
-	jmp EXT_C(cmain)
+	jmp EXT_C(grub_main)

Or maybe that should be a "call" instead..

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 /.)
#8 lore raw
Quoted 5 lines · Robert Millan

kern/i386/ieee1275/startup.S line 42

Quoted 2 lines · Robert Millan
kern/i386/ieee1275/startup.S renamed from grub2/kern/i386/ieee1275/startup.S +1 -1
-	jmp EXT_C(cmain)
+	jmp EXT_C(grub_main)

Or maybe that should be a "call" instead..

Only if there is any chance of return, and we can handle it.

"exit" in open firmware based systems would probably mean returning control to open firmware.

By the way, it would be nice to have "exit" in both grub for PC and grub-emu, and not only in the rescue mode. "exit" on PC means returning control to BIOS so it can try another boot device. "exit" in grub-emu means exit, of course. I have to use "reboot" now (which should not exit, but rather restart grub-emu, if it belongs to grub-emu at all) or "rescue" followed by "exit".

Signature · 2 lines
Regards,
Pavel Roskin
#9 lore raw
Quoted 9 lines · Pavel Roskin
Quoted 5 lines · Robert Millan

kern/i386/ieee1275/startup.S line 42

Quoted 2 lines · Robert Millan
kern/i386/ieee1275/startup.S renamed from grub2/kern/i386/ieee1275/startup.S +1 -1
-	jmp EXT_C(cmain)
+	jmp EXT_C(grub_main)

Or maybe that should be a "call" instead..

Only if there is any chance of return, and we can handle it.

Well, Lubomir suggested that you need that return address for gdb. Is that correct?

Quoted 2 lines

"exit" in open firmware based systems would probably mean returning control to open firmware.

Note that our ieee1275 exit is implemented via interrupt, which I believe has equivalent effect. We don't need the return address for that, in this case.

Quoted 6 lines

By the way, it would be nice to have "exit" in both grub for PC and grub-emu, and not only in the rescue mode. "exit" on PC means returning control to BIOS so it can try another boot device. "exit" in grub-emu means exit, of course. I have to use "reboot" now (which should not exit, but rather restart grub-emu, if it belongs to grub-emu at all) or "rescue" followed by "exit".

Agreed.

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 /.)
#10 lore raw
In reply to Pavel Roskin
Quoted 14 lines · Pavel Roskin
Quoted 5 lines · Robert Millan

kern/i386/ieee1275/startup.S line 42

Quoted 2 lines · Robert Millan
kern/i386/ieee1275/startup.S renamed from grub2/kern/i386/ieee1275/startup.S +1 -1
-   jmp EXT_C(cmain)
+   jmp EXT_C(grub_main)

Or maybe that should be a "call" instead..

Only if there is any chance of return, and we can handle it.

"exit" in open firmware based systems would probably mean returning control to open firmware.

Quoted 6 lines

By the way, it would be nice to have "exit" in both grub for PC and grub-emu, and not only in the rescue mode. "exit" on PC means returning control to BIOS so it can try another boot device. "exit" in grub-emu means exit, of course. I have to use "reboot" now (which should not exit, but rather restart grub-emu, if it belongs to grub-emu at all) or "rescue" followed by "exit".

Hi,

The would be nice, but we don't know if grub has overwritten the memory occupied by the original boot environment, so exit might have unexpected result.

Signature · 1 line
Bean
#11 lore raw
Quoted 11 lines · Bean
Quoted 10 lines

"exit" in open firmware based systems would probably mean returning control to open firmware.

By the way, it would be nice to have "exit" in both grub for PC and grub-emu, and not only in the rescue mode. "exit" on PC means returning control to BIOS so it can try another boot device. "exit" in grub-emu means exit, of course. I have to use "reboot" now (which should not exit, but rather restart grub-emu, if it belongs to grub-emu at all) or "rescue" followed by "exit".

Quoted 5 lines

Hi,

The would be nice, but we don't know if grub has overwritten the memory occupied by the original boot environment, so exit might have unexpected result.

In which case is this not granted? On pc/bios we use the memory map, on ieee1275 we use /memory/available, etc.

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 /.)
#12 lore raw
In reply to Robert Millan
Quoted 16 lines · Robert Millan
Quoted 15 lines · Pavel Roskin

Quoting Robert Millan <[email protected]>:

Quoted 13 lines

Now that I think of it, is there any reason for doing all the cmain() cruft before grub_main() ? Getting to grub_main() only requires a working stack, which we have, and grub_main() inmediately returns control back to architecture-specific code in grub_machine_init().

So why not move from:

_start -> cmain -> grub_main

to:

_start -> grub_main -> grub_machine_init -> cmain

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

?

I think it would be great. We should try to make the code for different architectures more uniform to simplify maintenance.

Here. Unfortunately I can't test it as I have no ieee1275 at hand currently.

Anyone?

I have an OLPC XO machine borrowed now. I will test that this evening.

Signature · 1 line
Lubomir Kundrak (Red Hat Security Response Team)
#13 lore raw
In reply to Robert Millan

Bean confirmed it works. Committed without changing jmp to call, since it wasn't clear whether we want it or not.

Quoted 106 lines · Robert Millan
Quoted 11 lines · Pavel Roskin

Quoting Robert Millan <[email protected]>:

Quoted 9 lines

Now that I think of it, is there any reason for doing all the cmain() cruft before grub_main() ? Getting to grub_main() only requires a working stack, which we have, and grub_main() inmediately returns control back to architecture-specific code in grub_machine_init().

So why not move from:

_start -> cmain -> grub_main

Quoted 8 lines
Quoted 5 lines

to:

_start -> grub_main -> grub_machine_init -> cmain

?

I think it would be great. We should try to make the code for different architectures more uniform to simplify maintenance.

Here. Unfortunately I can't test it as I have no ieee1275 at hand currently.

Anyone?

Signature · 7 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 /.)

diff -x configure -x config.h.in -x CVS -x '*~' -x '*.mk' -urp ../grub2/include/grub/ieee1275/ieee1275.h ./include/grub/ieee1275/ieee1275.h
include/grub/ieee1275/ieee1275.h renamed from grub2/include/grub/ieee1275/ieee1275.h +1 -0
@@ -97,6 +97,7 @@ extern void EXPORT_FUNC(grub_ieee1275_se
 
 
 
+void EXPORT_FUNC(grub_ieee1275_init) (void);
 int EXPORT_FUNC(grub_ieee1275_finddevice) (char *name,
 					   grub_ieee1275_phandle_t *phandlep);
 int EXPORT_FUNC(grub_ieee1275_get_property) (grub_ieee1275_phandle_t phandle,

diff -x configure -x config.h.in -x CVS -x '*~' -x '*.mk' -urp ../grub2/kern/i386/ieee1275/startup.S ./kern/i386/ieee1275/startup.S

kern/i386/ieee1275/startup.S renamed from grub2/kern/i386/ieee1275/startup.S +1 -1
@@ -39,7 +39,7 @@
 start:
 _start:
 	movl %eax, EXT_C(grub_ieee1275_entry_fn)
-	jmp EXT_C(cmain)
+	jmp EXT_C(grub_main)
 
 /*
  *  This call is special...  it never returns...  in fact it should simply

diff -x configure -x config.h.in -x CVS -x '*~' -x '*.mk' -urp ../grub2/kern/ieee1275/cmain.c ./kern/ieee1275/cmain.c

kern/ieee1275/cmain.c renamed from grub2/kern/ieee1275/cmain.c +1 -7
@@ -151,9 +151,8 @@ grub_ieee1275_find_options (void)
 #undef SF
 #undef OHW
 
-void cmain (void);
 void
-cmain (void)
+grub_ieee1275_init (void)
 {
   grub_ieee1275_finddevice ("/chosen", &grub_ieee1275_chosen);
 
@@ -162,9 +161,4 @@ cmain (void)
     grub_ieee1275_mmu = 0;
 
   grub_ieee1275_find_options ();
-
-  /* Now invoke the main function.  */
-  grub_main ();
-
-  /* Never reached.  */
 }

diff -x configure -x config.h.in -x CVS -x '*~' -x '*.mk' -urp ../grub2/kern/ieee1275/init.c ./kern/ieee1275/init.c

kern/ieee1275/init.c renamed from grub2/kern/ieee1275/init.c +2 -0
@@ -180,6 +180,8 @@ grub_machine_init (void)
   char args[256];
   int actual;
 
+  grub_ieee1275_init ();
+
   grub_console_init ();
 #ifdef __i386__
   grub_keyboard_controller_init ();

diff -x configure -x config.h.in -x CVS -x '*~' -x '*.mk' -urp ../grub2/kern/powerpc/ieee1275/crt0.S ./kern/powerpc/ieee1275/crt0.S

kern/powerpc/ieee1275/crt0.S renamed from grub2/kern/powerpc/ieee1275/crt0.S +1 -1
@@ -42,5 +42,5 @@ _start:	
 	lis	9, grub_ieee1275_entry_fn@ha
 	stw	5, grub_ieee1275_entry_fn@l(9)
 
-	bl	cmain
+	bl	grub_main
 1:	b	1b

_______________________________________________
Grub-devel mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/grub-devel

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 /.)
#14 lore raw
In reply to Lubomir Kundrak
Quoted 11 lines · Lubomir Kundrak

These are pretty self-explainable. I think I tried to sumbit these changes with the serial debugging patch year and some months ago and I was told to sign the copyright assignment. I have already done that now. :)

2008-04-15 Lubomir Kundrak <[email protected]>

* kern/main.c (grub_main) [!GRUB_UTIL]: Alias grub_main to main, so that gdb is able to find stack top.
* genmk.rb: replace strip with objcopy, so unstripped files are left untouched for debugging purposes.

I'll appreciate if you don't mix two completely different changes together.

Signature · 2 lines
Regards,
Pavel Roskin
#15 lore raw
Quoted 8 lines · Pavel Roskin
Quoted 7 lines · Lubomir Kundrak

These are pretty self-explainable. I think I tried to sumbit these changes with the serial debugging patch year and some months ago and I was told to sign the copyright assignment. I have already done that now. :)

2008-04-15 Lubomir Kundrak <[email protected]>

Quoted 7 lines
Quoted 4 lines

* kern/main.c (grub_main) [!GRUB_UTIL]: Alias grub_main to main, so that gdb is able to find stack top.
* genmk.rb: replace strip with objcopy, so unstripped files are left untouched for debugging purposes.

I'll appreciate if you don't mix two completely different changes together.

Yup, sorry for that.

The first change was discussed below. How about the second one, does it look sane?

Signature · 1 line
Lubomir Kundrak (Red Hat Security Response Team)