backtrace support

backtrace support

[Vincent Guffens]

[-- Attachment #1: Type: text/plain, Size: 1452 bytes --]

Here is a new version of the backtrace support. I have written the 
ChangeLog and modified the code according to your advices.

To compile with the backtrace support use ./configure --with-debug and 
load grub2 with the kern_debug module. The kern module will be unloaded 
automatically after having registered all the kernel symbols in a linked 
list that also contains the module symbols. Any files that includes 
<grub/backtrace.h> can now call grub_backtrace().

The functions that handle the linked list with the debug symbols are 
generic and are put in kern/backtrace.c. The function grub_backtrace() 
however is architecture dependant and is put in kern/i386/pc/backtrace.c.

This is a problem because I would say that it breaks compilation for 
other architectures. I think that the other architectures should have a 
function grub_backtrace() that does nothing, or even better that really 
does print a backtrace(). However, I don't know how to do it and I can't 
test it anyway.

I have also slightly modified the way genmk.rb generates the rule to 
create the modules. I hope this slight change does not do anything silly 
  that I missed.

I hope this backtrace will be usefull, but not too much !


-- 
				Vincent Guffens
				PhD Student UCL/CESAME
				tel:   +32 10 47 80 30
Value your freedom, or you will lose it, teaches history.
"Don't bother us with politics," respond those who don't want to learn.
	      	-- Richard M. Stallman

[-- Attachment #2: grub2-backtrace.diff --]
[-- Type: text/plain, Size: 14955 bytes --]

diff -ru -N -b -B grub2/ChangeLog grub2-backtrace/ChangeLog
--- grub2/ChangeLog	2005-08-20 10:25:51.000000000 +0200
+++ grub2-backtrace/ChangeLog	2005-08-22 22:52:47.934165416 +0200
@@ -1,3 +1,45 @@
+2005-08-22	Vincent Guffens	<guffens@inma.ucl.ac.be>
+
+	* conf/i386-pc.rmk : Added kern/backtrace.c, kern/i386/pc/backtrace.c 
+	in the kernel dependancy list and backtrace.h in the header list. Added a
+	new module kern_debug.mod.
+
+	* grub2/configure.ac : Added a configure switch : --with-debug. 
+	Turn the gcc optimization flag to -O0 and define a variable GRUB_DEBUG=1 
+	in Makefile.in if this switch is used. 
+
+	* gendebugkern.sh : New file : shell script which generates the debug
+	symbols for the kernel.
+
+	* genmk.rb : Do not strip the uneeded symbols from the objects if 
+	GRUB_DEBUG=1. Add a variable $(#{prefix}_OTHERDEP) in the dependancy list 
+	of the modules. This variable can be set to something useful in i386-pc.rmk.
+	Do not compile the module with all the dependances, just use the sources.
+
+	* include/grub/backtrace.h : New file
+
+	* kern/backtrace.c : New file 
+	(grub_register_debug_sym): New function
+	(grub_unregister_debug_sym): Likewise
+	(grub_print_debug_sym) : Likewise
+
+	* kern/dl.c : (Un)Register the debug symbols when a module is (un)loaded.
+
+	* kern/err.c (grub_fatal) : Call grub_backtrace()
+
+	* kern/i386/pc/backtrace.c : New file
+	(get_fp) : New function
+	(grub_backtrace) : likewise
+
+	* kern/kern_debug.c : New file : This is the source for the new module
+	kern_debug. It registers the kernel symbol during its initialisation and
+	does nothing else. It is automatically removed  later.
+
+	* kern/main.c  (grub_load_modules) : Try to unregister the kern_debug module.
+
+	* Makefile.in : Add a new variable GRUB_DEBUG which is set by configure. Add 
+	the rule to create the kernel symbol list in kern/grub_debug_kern.sym.
+	
 2005-08-20  Yoshinori K. Okuji  <okuji@enbug.org>
 
 	* loader/powerpc/ieee1275/linux.c (grub_rescue_cmd_linux): Specify
diff -ru -N -b -B grub2/conf/i386-pc.rmk grub2-backtrace/conf/i386-pc.rmk
--- grub2/conf/i386-pc.rmk	2005-08-20 09:49:01.000000000 +0200
+++ grub2-backtrace/conf/i386-pc.rmk	2005-08-22 20:54:51.000000000 +0200
@@ -28,13 +28,13 @@
 	kern/i386/dl.c kern/i386/pc/init.c kern/partition.c \
 	kern/env.c disk/i386/pc/biosdisk.c \
 	term/i386/pc/console.c \
-	symlist.c
+	symlist.c kern/backtrace.c kern/i386/pc/backtrace.c
 kernel_img_HEADERS = arg.h boot.h device.h disk.h dl.h elf.h env.h err.h \
 	file.h fs.h kernel.h loader.h misc.h mm.h net.h partition.h \
 	pc_partition.h rescue.h symbol.h term.h types.h \
 	machine/biosdisk.h machine/boot.h machine/console.h machine/init.h \
 	machine/memory.h machine/loader.h machine/time.h machine/vga.h \
-	machine/vbe.h
+	machine/vbe.h backtrace.h
 kernel_img_CFLAGS = $(COMMON_CFLAGS)
 kernel_img_ASFLAGS = $(COMMON_ASFLAGS)
 kernel_img_LDFLAGS = -nostdlib -Wl,-N,-Ttext,8200
@@ -93,7 +93,7 @@
 	partmap/amiga.c	partmap/apple.c partmap/pc.c partmap/sun.c	\
 	util/console.c util/grub-emu.c util/misc.c			\
 	util/i386/pc/biosdisk.c util/i386/pc/getroot.c			\
-	util/i386/pc/misc.c
+	util/i386/pc/misc.c kern/backtrace.c kern/i386/pc/backtrace.c
 
 grub_emu_LDFLAGS = $(LIBCURSES)
 
@@ -113,7 +113,7 @@
 	terminal.mod fshelp.mod chain.mod multiboot.mod amiga.mod	\
 	apple.mod pc.mod sun.mod loopback.mod reboot.mod halt.mod	\
 	help.mod default.mod timeout.mod configfile.mod vbe.mod		\
-	vesafb.mod vbetest.mod vbeinfo.mod search.mod
+	vesafb.mod vbetest.mod vbeinfo.mod search.mod kern_debug.mod
 
 # For _chain.mod.
 _chain_mod_SOURCES = loader/i386/pc/chainloader.c
@@ -274,3 +274,8 @@
 # For search.mod.
 search_mod_SOURCES = commands/search.c
 search_mod_CFLAGS = $(COMMON_CFLAGS)
+
+# For kern_debug.mod
+kern_debug_mod_OTHERDEP = kern/grub_debug_kern.sym
+kern_debug_mod_SOURCES = kern/kern_debug.c
+kern_debug_mod_CFLAGS = $(COMMON_CFLAGS) 
diff -ru -N -b -B grub2/config.h.in grub2-backtrace/config.h.in
--- grub2/config.h.in	2005-08-09 01:15:21.000000000 +0200
+++ grub2-backtrace/config.h.in	2005-08-20 18:02:14.000000000 +0200
@@ -16,6 +16,9 @@
 /* Define it to either end or _end */
 #undef END_SYMBOL
 
+/* enable some debug mechanisms */
+#undef GRUB_DEBUG
+
 /* Define if C symbols get an underscore after compilation */
 #undef HAVE_ASM_USCORE
 
diff -ru -N -b -B grub2/configure.ac grub2-backtrace/configure.ac
--- grub2/configure.ac	2005-08-09 01:15:21.000000000 +0200
+++ grub2-backtrace/configure.ac	2005-08-22 18:11:06.000000000 +0200
@@ -57,10 +57,20 @@
     AC_TRY_COMPILE(, , size_flag=yes, size_flag=no)
   ])
   if test "x$size_flag" = xyes; then
+		if test "$with_debug" == "yes"
+		then
+    	tmp_CFLAGS="$tmp_CFLAGS -O0"
+		else
     tmp_CFLAGS="$tmp_CFLAGS -Os"
+		fi
+  else
+		if test "$with_debug" == "yes"
+		then	
+    	tmp_CFLAGS="$tmp_CFLAGS -O0 -fno-strength-reduce -fno-unroll-loops"
   else
     tmp_CFLAGS="$tmp_CFLAGS -O2 -fno-strength-reduce -fno-unroll-loops"
   fi
+	fi
 
   # Force no alignment to save space on i386.
   if test "x$host_cpu" = xi386; then
@@ -108,6 +118,16 @@
 # This is not a "must".
 AC_PATH_PROG(RUBY, ruby)
 
+# take care of the configure arguments
+
+AC_ARG_WITH(debug, [  --with-debug            enable some debug mechanisms])
+if test "$with_debug" == "yes"; then
+	AC_DEFINE([GRUB_DEBUG], [], [enable some debug mechanisms]) 
+	AC_SUBST(GRUB_DEBUG,1)
+else
+	AC_SUBST(GRUB_DEBUG,0)
+fi
+
 # For cross-compiling.
 if test "x$build" != "x$host"; then
   AC_CHECK_PROGS(BUILD_CC, [gcc egcs cc],
diff -ru -N -b -B grub2/gendebugkern.sh grub2-backtrace/gendebugkern.sh
--- grub2/gendebugkern.sh	1970-01-01 01:00:00.000000000 +0100
+++ grub2-backtrace/gendebugkern.sh	2005-08-22 18:47:48.000000000 +0200
@@ -0,0 +1,14 @@
+#!/bin/bash
+# Generates the list of all the symbols
+# defined in the grub kernel. This is used
+# to print a stack trace in case of trouble
+
+
+OBJECT=$1
+
+if (test -e ${OBJECT}) ; then
+	objdump -t ${OBJECT} | grep -v 00000000 | awk '/.text/ {print "{\""$6"\",","0x"$1",","0x"$5 "},"}'  ;
+else
+	echo [0 ... MAX_BTSYMBOLS-2 ] = {"symbol", 0, 0},
+fi
+				
diff -ru -N -b -B grub2/genmk.rb grub2-backtrace/genmk.rb
--- grub2/genmk.rb	2005-08-07 19:12:51.000000000 +0200
+++ grub2-backtrace/genmk.rb	2005-08-22 21:46:23.000000000 +0200
@@ -119,14 +119,18 @@
 #{@name}: #{pre_obj} #{mod_obj}
 	-rm -f $@
 	$(LD) -r -d -o $@ $^
+ifeq ($(GRUB_DEBUG),1)
+	$(STRIP) --strip-debug -K grub_mod_init -K grub_mod_fini -R .note -R .comment $@
+else
 	$(STRIP) --strip-unneeded -K grub_mod_init -K grub_mod_fini -R .note -R .comment $@
+endif
 
 #{pre_obj}: #{objs_str}
 	-rm -f $@
 	$(LD) -r -d -o $@ $^
 
-#{mod_obj}: #{mod_src}
-	$(CC) $(CPPFLAGS) $(CFLAGS) $(#{prefix}_CFLAGS) -c -o $@ $<
+#{mod_obj}: $(#{prefix}_OTHERDEP) #{mod_src} 
+	$(CC) $(CPPFLAGS) $(CFLAGS) $(#{prefix}_CFLAGS) -c -o $@ #{mod_src}
 
 #{mod_src}: moddep.lst genmodsrc.sh
 	sh $(srcdir)/genmodsrc.sh '#{mod_name}' $< > $@ || (rm -f $@; exit 1)
@@ -147,8 +151,8 @@
       flag = if /\.c$/ =~ src then 'CFLAGS' else 'ASFLAGS' end
       dir = File.dirname(src)
 
-      "#{obj}: #{src}
-	$(CC) -I#{dir} -I$(srcdir)/#{dir} $(CPPFLAGS) $(#{flag}) $(#{prefix}_#{flag}) -c -o $@ $<
+      "#{obj}: $(#{prefix}_OTHERDEP) #{src}
+	$(CC) -I#{dir} -I$(srcdir)/#{dir} $(CPPFLAGS) $(#{flag}) $(#{prefix}_#{flag}) -c -o $@ #{src}
 
 #{dep}: #{src}
 	set -e; \
Les fichiers binaires grub2/grub_disk et grub2-backtrace/grub_disk sont différents.
diff -ru -N -b -B grub2/include/grub/backtrace.h grub2-backtrace/include/grub/backtrace.h
--- grub2/include/grub/backtrace.h	1970-01-01 01:00:00.000000000 +0100
+++ grub2-backtrace/include/grub/backtrace.h	2005-08-20 18:46:33.000000000 +0200
@@ -0,0 +1,12 @@
+#ifndef GRUB_BACKTRACE_H
+#define GRUB_BACKTRACE_H
+
+#include <grub/types.h>
+#include <grub/symbol.h>
+
+void EXPORT_FUNC(grub_register_debug_sym) (const char*, void*, grub_size_t);
+void EXPORT_FUNC(grub_unregister_debug_sym) (void *);
+int EXPORT_FUNC(grub_print_debug_sym) (grub_addr_t);
+void EXPORT_FUNC(grub_backtrace) (void);
+
+#endif
diff -ru -N -b -B grub2/kern/backtrace.c grub2-backtrace/kern/backtrace.c
--- grub2/kern/backtrace.c	1970-01-01 01:00:00.000000000 +0100
+++ grub2-backtrace/kern/backtrace.c	2005-08-20 18:42:11.000000000 +0200
@@ -0,0 +1,87 @@
+#include <grub/backtrace.h>
+#include <grub/misc.h>
+#include <grub/types.h>
+#include <grub/mm.h>
+#include <config.h>
+
+#ifdef GRUB_DEBUG
+
+static struct grub_debug_sym {
+	struct grub_debug_sym * next;
+	char * name;
+	grub_addr_t addr;
+	grub_size_t size;
+} * grub_debug_sym = 0;
+
+int grub_print_debug_sym (grub_addr_t a) {
+	struct grub_debug_sym * entry = grub_debug_sym;
+
+	while( entry ) {
+		if ((entry->addr <= a) && 
+				(entry->addr + entry->size > a)) {
+			grub_printf("0x%lx+%lx : %s \n",
+											(long int)entry->addr,
+											(long int)(a-entry->addr), 
+											entry->name);
+			return 1;
+		}
+		entry = entry->next;
+	}
+	return 0;
+}
+
+void grub_register_debug_sym (const char* name ,void * addr,grub_size_t sz) {
+
+	struct grub_debug_sym * new_entry;
+
+	if (addr) {
+		new_entry = (struct grub_debug_sym *) grub_malloc(sizeof(*new_entry));
+		if (! new_entry)
+			return;
+		new_entry->name = grub_strdup(name);
+		new_entry->addr = (grub_addr_t) addr;
+		new_entry->size = sz;
+
+		new_entry->next = grub_debug_sym;
+		grub_debug_sym = new_entry;
+	}
+}
+
+void grub_unregister_debug_sym (void * address) {
+	struct grub_debug_sym * next_entry, * entry = grub_debug_sym;
+	grub_addr_t addr = (grub_addr_t) address;
+	
+	if (! entry)
+		return;
+	
+	if (entry->addr == addr ) {
+		grub_debug_sym = entry->next;
+		grub_free(entry->name);
+		grub_free(entry);
+		return;
+	}
+	
+	next_entry = entry->next;
+	
+	while (next_entry) {
+		if (next_entry->addr == addr ) {
+			entry->next=next_entry->next;
+			grub_free(next_entry->name);
+			grub_free(next_entry);
+			return;
+		}
+		entry = next_entry;
+		next_entry=next_entry->next;
+	}
+	
+}
+#else
+
+#define NOOP {do {} while (0);}
+inline void grub_register_debug_sym (const char* name  __attribute__ ((unused)),
+		                          void* ad  __attribute__ ((unused)),
+														 	grub_size_t s __attribute__ ((unused))) NOOP;
+inline void grub_unregister_debug_sym (void * addr __attribute__((unused))) NOOP;
+inline int grub_print_debug_sym (grub_addr_t a __attribute__((unused))) {return 0;};
+
+#endif
diff -ru -N -b -B grub2/kern/dl.c grub2-backtrace/kern/dl.c
--- grub2/kern/dl.c	2005-02-14 19:41:33.000000000 +0100
+++ grub2-backtrace/kern/dl.c	2005-08-20 18:48:30.000000000 +0200
@@ -29,6 +29,7 @@
 #include <grub/file.h>
 #include <grub/env.h>
 #include <grub/cache.h>
+#include <grub/backtrace.h>
 
 #if GRUB_HOST_SIZEOF_VOID_P == 4
 
@@ -216,6 +217,7 @@
 	  if (sym->mod == mod)
 	    {
 	      *p = q;
+				grub_unregister_debug_sym (sym->addr);
 	      grub_free ((void *) sym->name);
 	      grub_free (sym);
 	    }
@@ -375,6 +377,7 @@
 	case STT_FUNC:
 	  sym->st_value += (Elf_Addr) grub_dl_get_section_addr (mod,
 								sym->st_shndx);
+		grub_register_debug_sym (name,(void *) sym->st_value, sym->st_size);
 	  if (bind != STB_LOCAL)
 	    if (grub_dl_register_symbol (name, (void *) sym->st_value, mod))
 	      return grub_errno;
diff -ru -N -b -B grub2/kern/err.c grub2-backtrace/kern/err.c
--- grub2/kern/err.c	2004-04-04 15:46:01.000000000 +0200
+++ grub2-backtrace/kern/err.c	2005-08-20 18:35:16.000000000 +0200
@@ -20,6 +20,7 @@
 
 #include <grub/err.h>
 #include <grub/misc.h>
+#include <grub/backtrace.h>
 #include <stdarg.h>
 
 #define GRUB_MAX_ERRMSG	256
@@ -50,6 +51,9 @@
   grub_vprintf (fmt, ap);
   va_end (ap);
 
+#ifndef GRUB_UTIL
+	grub_backtrace();
+#endif
   grub_stop ();
 }
 
diff -ru -N -b -B grub2/kern/i386/pc/backtrace.c grub2-backtrace/kern/i386/pc/backtrace.c
--- grub2/kern/i386/pc/backtrace.c	1970-01-01 01:00:00.000000000 +0100
+++ grub2-backtrace/kern/i386/pc/backtrace.c	2005-08-20 18:42:19.000000000 +0200
@@ -0,0 +1,35 @@
+#include <grub/backtrace.h>
+#include <grub/types.h>
+#include <grub/misc.h> 
+
+#ifdef GRUB_DEBUG
+
+#define BTSYM_DISPLAY 12
+
+static grub_addr_t * get_fp(void) {
+  grub_addr_t * a;
+  asm ("movl %%ebp, %0;"
+      :"=r"(a)        /* output */
+      );
+  return a;
+}
+
+void grub_backtrace (void) {
+  grub_addr_t * a;
+  int i =1;
+
+  grub_printf("\nBacktrace:\n");
+  a = get_fp();
+
+  while ( (i <= BTSYM_DISPLAY) && grub_print_debug_sym (*(a+1)) ) {
+    a= (grub_addr_t *)(*a);
+    i++;
+  }
+}
+
+
+#else
+
+inline void grub_backtrace (void)  {do {} while (0);};
+
+#endif
diff -ru -N -b -B grub2/kern/kern_debug.c grub2-backtrace/kern/kern_debug.c
--- grub2/kern/kern_debug.c	1970-01-01 01:00:00.000000000 +0100
+++ grub2-backtrace/kern/kern_debug.c	2005-08-22 22:01:15.000000000 +0200
@@ -0,0 +1,47 @@
+#include <grub/symbol.h>
+#include <grub/types.h>
+#include <grub/backtrace.h>
+#include <grub/dl.h>
+#include <config.h>
+
+#ifdef GRUB_DEBUG
+
+#define MAX_SYMNAME_L 80
+#define MAX_BTSYMBOLS 512
+
+
+static struct kern_debug_sym {
+  char name[MAX_SYMNAME_L+1];
+  grub_addr_t addr;
+  grub_size_t size;
+} kern_debug_sym[MAX_BTSYMBOLS] = {
+#include "grub_debug_kern.sym"
+};
+
+
+GRUB_MOD_INIT
+{
+  (void)mod;			/* To stop warning. */
+	int i=0;
+
+	while (kern_debug_sym[i].addr) {
+		grub_register_debug_sym(kern_debug_sym[i].name,
+													  (void *)kern_debug_sym[i].addr,
+														kern_debug_sym[i].size);
+		i++;
+	}
+	/* This module is not needed anymore */
+
+}
+
+
+#else
+
+GRUB_MOD_INIT
+{
+  (void)mod;
+	/* Nothing to do */
+}
+
+
+#endif
diff -ru -N -b -B grub2/kern/main.c grub2-backtrace/kern/main.c
--- grub2/kern/main.c	2005-08-12 21:53:32.000000000 +0200
+++ grub2-backtrace/kern/main.c	2005-08-22 22:08:16.000000000 +0200
@@ -35,6 +35,7 @@
 {
   struct grub_module_info *modinfo;
   struct grub_module_header *header;
+	grub_dl_t mod;
   grub_addr_t modbase;
 
   modbase = grub_arch_modules_addr ();
@@ -55,6 +56,13 @@
 
   /* Add the region where modules reside into dynamic memory.  */
   grub_mm_init_region ((void *) modinfo, modinfo->size);
+	
+	/* remove the kern_debug module */
+	mod = grub_dl_get("kern_debug");
+	if (mod && ( grub_dl_unref (mod) <= 0  ))
+		grub_dl_unload (mod);
+	
+
 }
 
 /* Write hook for the environment variables of root. Remove surrounding
diff -ru -N -b -B grub2/Makefile.in grub2-backtrace/Makefile.in
--- grub2/Makefile.in	2005-08-09 01:15:21.000000000 +0200
+++ grub2-backtrace/Makefile.in	2005-08-22 21:40:21.000000000 +0200
@@ -69,6 +69,8 @@
 LIBCURSES = @LIBCURSES@
 LIBLZO = @LIBLZO@
 
+### Debug variable
+GRUB_DEBUG = @GRUB_DEBUG@
 ### General variables.
 
 RMKFILES = $(addprefix conf/,i386-pc.rmk powerpc-ieee1275.rmk)
@@ -239,5 +240,8 @@
 .PHONY: all install install-strip uninstall clean mostlyclean distclean
 .PHONY: maintainer-clean info dvi dist check
 
+kern/grub_debug_kern.sym: kernel.exec
+	./gendebugkern.sh kernel.exec > $@
+	
 # Prevent an overflow.
 .NOEXPORT:

Re: backtrace support

[Vincent Pelletier]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Vincent Guffens wrote:
> This is a problem because I would say that it breaks compilation for
> other architectures. I think that the other architectures should have a
> function grub_backtrace() that does nothing, or even better that really
> does print a backtrace(). However, I don't know how to do it and I can't
> test it anyway.

For sparc64-ieee1275, there is a ctrace command in open firmware which
does that work. grub_backtrace() could do a
grub_ieee1275_interpret("ctrace",NULL);
then
grub_ieee1275_exit();

To make ctrace work the ELF has to be unstripped - for now I hardcoded
it in conf/sparc64-ieee1275.rmk. To my experience, -O1 produces an
easier to understand ASM than -O0 (sparc64-ieee1275 allows to run in
step-by-step or breakpoint mode) because -O0 does really dumb things
that is very verbose in ASM line number. But I don't know in details
what -O1 does, and if it has drawbacks in optimisation (I think about
local functions).

> I hope this backtrace will be usefull, but not too much !

I have already found a use for it : I can't make vbetest work, it fails
with out of range pointer.

I hope it will be committed soon.

Oh, by the way, I'm trying to implement the OpenFirmware framebuffer
primitives - for now with no luck - so we can have it on ieee1275 ports.
There are lots of FCODE examples on Google, but all for PPC.
My current problem is how to allocate memory for the framebuffer itself,
and how to pass the address "to frame_buffer_adr" (the usual FCODE way
to store a value), if possible without using grub_ieee1275_interpret.

Vincent Pelletier
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFDCtRfFEQoKRQyjtURArwAAJ0QJ9cOK7WTW40UKVdf5O12vU9x5ACfcmXC
wmVVj2BJem/7hlyZ4boBMp8=
=lCgj
-----END PGP SIGNATURE-----

	

	
		
___________________________________________________________________________ 
Appel audio GRATUIT partout dans le monde avec le nouveau Yahoo! Messenger 
Téléchargez cette version sur http://fr.messenger.yahoo.com

Re: backtrace support

[Yoshinori K. Okuji]

On Monday 22 August 2005 23:23, Vincent Guffens wrote:
> +# For kern_debug.mod
> +kern_debug_mod_OTHERDEP = kern/grub_debug_kern.sym
> +kern_debug_mod_SOURCES = kern/kern_debug.c
> +kern_debug_mod_CFLAGS = $(COMMON_CFLAGS) 

I think it is a really good idea to make a variable for dependencies, but I 
would suggest a different name for this. Can this be just 
kern_debug_mod_DEPENDENCIES?

> +# take care of the configure arguments
> +
> +AC_ARG_WITH(debug, [  --with-debug            enable some debug 
mechanisms])

I think it would be better to use AC_ARG_ENABLE with --enable-debug, because 
--with is used to specify external programs in autoconf.

> diff -ru -N -b -B grub2/gendebugkern.sh grub2-backtrace/gendebugkern.sh
> --- grub2/gendebugkern.sh       1970-01-01 01:00:00.000000000 +0100
> +++ grub2-backtrace/gendebugkern.sh     2005-08-22 18:47:48.000000000 +0200
> @@ -0,0 +1,14 @@
> +#!/bin/bash

Please do not use /bin/bash for portability. It should be:

#! /bin/sh

> +void EXPORT_FUNC(grub_register_debug_sym) (const char*, void*, 
grub_size_t);
> +void EXPORT_FUNC(grub_unregister_debug_sym) (void *);
> +int EXPORT_FUNC(grub_print_debug_sym) (grub_addr_t);
> +void EXPORT_FUNC(grub_backtrace) (void);

Why do you need to export these functions? Are they used outside the kernel?

Okuji

Re: backtrace support

[Vincent Guffens]

Hi,

I will make change to the patch according to these comments.

>>+void EXPORT_FUNC(grub_register_debug_sym) (const char*, void*, 
> 
> grub_size_t);
> 
>>+void EXPORT_FUNC(grub_unregister_debug_sym) (void *);
>>+int EXPORT_FUNC(grub_print_debug_sym) (grub_addr_t);
>>+void EXPORT_FUNC(grub_backtrace) (void);
> 
> 
> Why do you need to export these functions? Are they used outside the kernel?
> 

grub_register_debug_sym is used by the module responsible for loading 
the debug symbols. grub_backtrace could be used by other modules.

-- 
				Vincent Guffens
				PhD Student UCL/CESAME
				tel:   +32 10 47 80 30
Value your freedom, or you will lose it, teaches history.
"Don't bother us with politics," respond those who don't want to learn.
	      	-- Richard M. Stallman

Re: backtrace support

[Marco Gerards]

Vincent Guffens <guffens@inma.ucl.ac.be> writes:

Hi Vincent,

> Here is a new version of the backtrace support. I have written the
> ChangeLog and modified the code according to your advices.

Thanks a lot!  Here are still some comments and questions about the
patch.

> To compile with the backtrace support use ./configure --with-debug and
> load grub2 with the kern_debug module. The kern module will be
> unloaded automatically after having registered all the kernel symbols
> in a linked list that also contains the module symbols. Any files that
> includes <grub/backtrace.h> can now call grub_backtrace().

I'm not too familiar with autoconf, but I had the idea that
--enable-debug would be more logical and that --with is to tell
configure where to find libraries.  According to the autoconf manual
it is to add external software like libraries.

I have the idea that another array for the symbols is not required.  I
think we can use one common table.  The symbol name and symbol address
are already in there.  The hard thing is would getting the size,
perhaps we could leave that open and fill it in with a function, or
can we use a ld/gcc feature to do that?  This will make the backtrace
code simpler and it saves memory.  Do you think that is somehow
possible?

> The functions that handle the linked list with the debug symbols are
> generic and are put in kern/backtrace.c. The function grub_backtrace()
> however is architecture dependant and is put in
> kern/i386/pc/backtrace.c.
>
> This is a problem because I would say that it breaks compilation for
> other architectures. I think that the other architectures should have
> a function grub_backtrace() that does nothing, or even better that
> really does print a backtrace(). However, I don't know how to do it
> and I can't test it anyway.

I can fix this for the PPC when it is applied, that's not a problem.

> I have also slightly modified the way genmk.rb generates the rule to
> create the modules. I hope this slight change does not do anything
> silly that I missed.

Hopefully Okuji can have a look at these changes.  I don't know ruby
and this code. :(

> I hope this backtrace will be usefull, but not too much !

It will be. :-)

> +	* conf/i386-pc.rmk : Added kern/backtrace.c, kern/i386/pc/backtrace.c 
> +	in the kernel dependancy list and backtrace.h in the header list. Added a
> +	new module kern_debug.mod.

Please be specific about to which variables they are added.  You can
see how we did that in the past in ChangeLog.
 +
> +	* grub2/configure.ac : Added a configure switch : --with-debug. 
> +	Turn the gcc optimization flag to -O0 and define a variable GRUB_DEBUG=1 
> +	in Makefile.in if this switch is used. 

Same here.

> +	* gendebugkern.sh : New file : shell script which generates the debug
> +	symbols for the kernel.

Just saying "New file." would be enough.  After that you can say
"Likewise." for every other new files that follows this line.  Please
make sure the ":" follows the filename directly without a space, and
start the sentence with a capital letter.

Same for the rest of the changelog and please tell which function is
changed.  Have a look at the ChangeLog for an example.



As for the sourcecode, I have seen a lot of incorrect indention, etc.
It does not yet comply with the GCS (which is the GNU Coding Style and
also describes the changelog format:
http://www.gnu.org/prep/standards/).  Can you please have a look at
this, a consistent coding style is very important for a big problem
like GRUB.  If you want me to help you or point you to some problems
in the code which you can't find after reading this, just ask me so I
can help you.

Thanks,
Marco

Re: backtrace support

[Vincent Guffens]

> I have the idea that another array for the symbols is not required.  I
> think we can use one common table.  The symbol name and symbol address
> are already in there.  The hard thing is would getting the size,
> perhaps we could leave that open and fill it in with a function, or
> can we use a ld/gcc feature to do that?  This will make the backtrace
> code simpler and it saves memory.  Do you think that is somehow
> possible?

This is true, another list of symbols is not required for this backtrace 
support but it makes the implementation a lot less intrusive. It would 
be possible to use some #ifdef here and there around the structures that 
are used already to add a field for the symbol size but it would not be 
very nice. If you turn the debug flag off, the code comes back to the 
original so I thought it would not be a problem.

However, I found using a simple example that using -O2 still allows for 
using the frame pointer. This is to be verified but if it turns to be 
usable, than maybe using the --enable-debug will be less a problem and 
more popular so it would then be wise not to have the kernel symbol list 
twice.  What do you think ?

To correct the rest, I think I will read the GCS in detail.

-- 
				Vincent Guffens
				PhD Student UCL/CESAME
				tel:   +32 10 47 80 30
Value your freedom, or you will lose it, teaches history.
"Don't bother us with politics," respond those who don't want to learn.
	      	-- Richard M. Stallman

Re: backtrace support

[Marco Gerards]

Vincent Guffens <guffens@inma.ucl.ac.be> writes:

>> I have the idea that another array for the symbols is not required.  I
>> think we can use one common table.  The symbol name and symbol address
>> are already in there.  The hard thing is would getting the size,
>> perhaps we could leave that open and fill it in with a function, or
>> can we use a ld/gcc feature to do that?  This will make the backtrace
>> code simpler and it saves memory.  Do you think that is somehow
>> possible?
>
> This is true, another list of symbols is not required for this
> backtrace support but it makes the implementation a lot less
> intrusive. It would be possible to use some #ifdef here and there
> around the structures that are used already to add a field for the
> symbol size but it would not be very nice. If you turn the debug flag
> off, the code comes back to the original so I thought it would not be
> a problem.

It won't be that many ifdefs.  I think that would be better than yet
another table.

> However, I found using a simple example that using -O2 still allows
> for using the frame pointer. This is to be verified but if it turns to
> be usable, than maybe using the --enable-debug will be less a problem
> and more popular so it would then be wise not to have the kernel
> symbol list twice.  What do you think ?

Right, I think -O2 is not a problem if you do not show line numbers.
But if you show line numbers -O2 is not always accurate AFAIK because
gcc can reorder lines.

> To correct the rest, I think I will read the GCS in detail.

Cool!

Thanks,
Marco

backtrace support

[Vincent Guffens]

[-- Attachment #1: Type: text/plain, Size: 1348 bytes --]

Hi,

After having searched for the reason of the unaligned pointer caused by 
the nested functions bug, I thought that it could be interesting to have 
a backtrace in grub. It would be triggered in grub_fatal and would print 
the last few function calls that triggered the problem.

I have implemented such a backtrace function using the stack base 
pointer. You can see how it looks like on the picture attached. This is 
a screenshot taken after the bug mentioned above and it points directly 
to the cause of the problem.

However, the price to pay for that, at least with my implementation, is 
quite high. One has to disable the optimization flag to prevent the 
-fomit-frame-pointer and the module must not be stripped. Still, during 
the developpement phase, it might be usefull to get good bug reports.

The feature is added with
./configure --with-backtrace
and grub must be compile with ./btmake instead of make which is a simple 
script which calls make twice instead of only once.

I include the patch as attachment, if not for inclusion in grub, at 
least for the potential interrested reader.


-- 
				Vincent Guffens
				PhD Student UCL/CESAME
				tel:   +32 10 47 80 30
Value your freedom, or you will lose it, teaches history.
"Don't bother us with politics," respond those who don't want to learn.
	      	-- Richard M. Stallman

[-- Attachment #2: backtrace.jpg --]
[-- Type: image/jpeg, Size: 28474 bytes --]

[-- Attachment #3: grub2-backtrace.diff --]
[-- Type: text/plain, Size: 11831 bytes --]

diff -ru -N -b -B grub2/btmake grub2-backtrace/btmake
--- grub2/btmake	1970-01-01 01:00:00.000000000 +0100
+++ grub2-backtrace/btmake	2005-08-18 18:48:36.000000000 +0200
@@ -0,0 +1,9 @@
+#!/bin/sh
+# Create a grub kernel image with backtrace support
+# configure must have been run with : --with-backtrace
+make
+./genbtsym.sh kernel.exec > kern/i386/pc/grub_btsym_list.txt
+rm kernel_img-kern_i386_pc_backtrace.d
+rm kernel_img-kern_i386_pc_backtrace.o
+make
+
diff -ru -N -b -B grub2/conf/i386-pc.rmk grub2-backtrace/conf/i386-pc.rmk
--- grub2/conf/i386-pc.rmk	2005-08-12 21:53:32.000000000 +0200
+++ grub2-backtrace/conf/i386-pc.rmk	2005-08-15 16:22:03.000000000 +0200
@@ -25,7 +25,7 @@
 kernel_img_SOURCES = kern/i386/pc/startup.S kern/main.c kern/device.c \
 	kern/disk.c kern/dl.c kern/file.c kern/fs.c kern/err.c \
 	kern/misc.c kern/mm.c kern/loader.c kern/rescue.c kern/term.c \
-	kern/i386/dl.c kern/i386/pc/init.c kern/partition.c \
+	kern/i386/dl.c kern/i386/pc/init.c kern/i386/pc/backtrace.c kern/partition.c \
 	kern/env.c disk/i386/pc/biosdisk.c \
 	term/i386/pc/console.c \
 	symlist.c
@@ -92,7 +92,7 @@
 	partmap/amiga.c	partmap/apple.c partmap/pc.c partmap/sun.c	\
 	util/console.c util/grub-emu.c util/misc.c			\
 	util/i386/pc/biosdisk.c util/i386/pc/getroot.c			\
-	util/i386/pc/misc.c
+	util/i386/pc/misc.c kern/i386/pc/backtrace.c
 
 grub_emu_LDFLAGS = $(LIBCURSES)
 
diff -ru -N -b -B grub2/config.h.in grub2-backtrace/config.h.in
--- grub2/config.h.in	2005-08-09 01:15:21.000000000 +0200
+++ grub2-backtrace/config.h.in	2005-08-13 18:32:23.000000000 +0200
@@ -16,6 +16,9 @@
 /* Define it to either end or _end */
 #undef END_SYMBOL
 
+/* enable backtrace support */
+#undef GRUB_BACKTRACE
+
 /* Define if C symbols get an underscore after compilation */
 #undef HAVE_ASM_USCORE
 
diff -ru -N -b -B grub2/configure.ac grub2-backtrace/configure.ac
--- grub2/configure.ac	2005-08-09 01:15:21.000000000 +0200
+++ grub2-backtrace/configure.ac	2005-08-18 17:36:26.000000000 +0200
@@ -57,9 +57,19 @@
     AC_TRY_COMPILE(, , size_flag=yes, size_flag=no)
   ])
   if test "x$size_flag" = xyes; then
+		if test "$with_backtrace" == "yes"
+		then
+			tmp_CFLAGS="$tmp_CFLAGS -O0"
+		else
     tmp_CFLAGS="$tmp_CFLAGS -Os"
+		fi
+  else
+		if test "$with_backtrace" == "yes"
+		then
+    	tmp_CFLAGS="$tmp_CFLAGS -O0 -fno-strength-reduce -fno-unroll-loops"
   else
-    tmp_CFLAGS="$tmp_CFLAGS -O2 -fno-strength-reduce -fno-unroll-loops"
+    	tmp_CFLAGS="$tmp_CFLAGS -Os -fno-strength-reduce -fno-unroll-loops"
+		fi
   fi
 
   # Force no alignment to save space on i386.
@@ -108,6 +118,16 @@
 # This is not a "must".
 AC_PATH_PROG(RUBY, ruby)
 
+# Include the stack trace support ?
+AC_ARG_WITH(backtrace, [  --with-backtrace            enable stack trace support for i386])
+if test "$with_backtrace" == "yes"
+then
+	case "$host_cpu" in
+  	i[[3456]]86) AC_DEFINE([GRUB_BACKTRACE], [], [enable backtrace support])  ;;
+  	*) AC_MSG_NOTICE([backtrace support available for i386 only]) ;;
+	esac
+fi
+
 # For cross-compiling.
 if test "x$build" != "x$host"; then
   AC_CHECK_PROGS(BUILD_CC, [gcc egcs cc],
diff -ru -N -b -B grub2/genbtsym.sh grub2-backtrace/genbtsym.sh
--- grub2/genbtsym.sh	1970-01-01 01:00:00.000000000 +0100
+++ grub2-backtrace/genbtsym.sh	2005-08-18 18:34:12.000000000 +0200
@@ -0,0 +1,14 @@
+#!/bin/bash
+# Generates the list of all the symbols
+# defined in the grub kernel. This is used
+# to print a stack trace in case of trouble
+
+
+OBJECT=$1
+
+if (test -e ${OBJECT}) ; then
+	objdump -t ${OBJECT} | grep -v 00000000 | awk '/.text/ {print "{\""$6"\",","0x"$1",","0x"$5 "},"}'  ;
+else
+	echo [0 ... MAX_BTSYMBOLS-2 ] = {"symbol", 0, 0},
+fi
+				
diff -ru -N -b -B grub2/genmk.rb grub2-backtrace/genmk.rb
--- grub2/genmk.rb	2005-08-07 19:12:51.000000000 +0200
+++ grub2-backtrace/genmk.rb	2005-08-18 18:43:05.000000000 +0200
@@ -119,7 +119,9 @@
 #{@name}: #{pre_obj} #{mod_obj}
 	-rm -f $@
 	$(LD) -r -d -o $@ $^
-	$(STRIP) --strip-unneeded -K grub_mod_init -K grub_mod_fini -R .note -R .comment $@
+	 cat config.h | grep define | grep BACKTRACE \
+	 && $(STRIP) --strip-debug -R .note -R .comment $@ \
+	 || $(STRIP) --strip-unneeded -K grub_mod_init -K grub_mod_fini -R .note -R .comment $@ 
 
 #{pre_obj}: #{objs_str}
 	-rm -f $@
diff -ru -N -b -B grub2/include/grub/kernel.h grub2-backtrace/include/grub/kernel.h
--- grub2/include/grub/kernel.h	2005-02-15 01:07:01.000000000 +0100
+++ grub2-backtrace/include/grub/kernel.h	2005-08-18 20:49:57.000000000 +0200
@@ -21,6 +21,7 @@
 #define GRUB_KERNEL_HEADER	1
 
 #include <grub/types.h>
+#include <grub/symbol.h>
 
 /* The module header.  */
 struct grub_module_header
@@ -58,4 +59,11 @@
 /* Register all the exported symbols. This is automatically generated.  */
 void grub_register_exported_symbols (void);
 
+/* backtrace support for i386 */
+void EXPORT_FUNC(grub_backtrace) (void);
+void EXPORT_FUNC(grub_btregister_symbol) (const char*,void*,grub_size_t);
+void EXPORT_FUNC(grub_btunregister_symbol) (void *);
+void EXPORT_FUNC(grub_init_btsym) (void);
+
+
 #endif /* ! GRUB_KERNEL_HEADER */
diff -ru -N -b -B grub2/kern/dl.c grub2-backtrace/kern/dl.c
--- grub2/kern/dl.c	2005-02-14 19:41:33.000000000 +0100
+++ grub2-backtrace/kern/dl.c	2005-08-18 20:51:51.000000000 +0200
@@ -29,6 +29,7 @@
 #include <grub/file.h>
 #include <grub/env.h>
 #include <grub/cache.h>
+#include <grub/kernel.h>
 
 #if GRUB_HOST_SIZEOF_VOID_P == 4
 
@@ -216,6 +217,7 @@
 	  if (sym->mod == mod)
 	    {
 	      *p = q;
+				grub_btunregister_symbol(sym->addr);
 	      grub_free ((void *) sym->name);
 	      grub_free (sym);
 	    }
@@ -375,6 +377,7 @@
 	case STT_FUNC:
 	  sym->st_value += (Elf_Addr) grub_dl_get_section_addr (mod,
 								sym->st_shndx);
+		grub_btregister_symbol(name,(void *) sym->st_value, sym->st_size);
 	  if (bind != STB_LOCAL)
 	    if (grub_dl_register_symbol (name, (void *) sym->st_value, mod))
 	      return grub_errno;
@@ -654,6 +657,7 @@
   grub_dl_remove (mod);
   grub_dl_unregister_symbols (mod);
   
+  
   for (dep = mod->dep; dep; dep = depn)
     {
       depn = dep->next;
diff -ru -N -b -B grub2/kern/err.c grub2-backtrace/kern/err.c
--- grub2/kern/err.c	2004-04-04 15:46:01.000000000 +0200
+++ grub2-backtrace/kern/err.c	2005-08-15 16:22:37.000000000 +0200
@@ -20,6 +20,7 @@
 
 #include <grub/err.h>
 #include <grub/misc.h>
+#include <grub/kernel.h>
 #include <stdarg.h>
 
 #define GRUB_MAX_ERRMSG	256
@@ -50,6 +51,9 @@
   grub_vprintf (fmt, ap);
   va_end (ap);
 
+#ifndef GRUB_UTIL
+	grub_backtrace();
+#endif	
   grub_stop ();
 }
 
diff -ru -N -b -B grub2/kern/i386/pc/backtrace.c grub2-backtrace/kern/i386/pc/backtrace.c
--- grub2/kern/i386/pc/backtrace.c	1970-01-01 01:00:00.000000000 +0100
+++ grub2-backtrace/kern/i386/pc/backtrace.c	2005-08-18 21:04:31.000000000 +0200
@@ -0,0 +1,150 @@
+#include <grub/kernel.h>
+#include <grub/misc.h>
+#include <grub/types.h>
+#include <grub/mm.h>
+#include <config.h>
+
+#ifdef GRUB_BACKTRACE
+
+#define MAX_BTSYMBOLS 512
+#define BTSYM_DISPLAY 10
+#define MAX_SYMNAME_L 80
+
+static struct btsym {
+	char name[MAX_SYMNAME_L+1];
+	grub_addr_t address;
+	grub_size_t size;	
+} btsym_list[MAX_BTSYMBOLS] = {
+#include "grub_btsym_list.txt"
+};
+
+static struct btsym_l {
+	struct btsym_l * next;
+	struct btsym * entry;
+} * btsym_l = 0;
+
+static int get_btsymbol(grub_addr_t a) {
+	struct btsym_l * it = btsym_l;
+	struct btsym * entry;
+
+	while( it ) {
+		entry = it->entry;
+		if ((entry->address <= a) && 
+										(entry->address + 
+										 entry->size > a)) {
+			grub_printf("0x%lx+%lx : %s \n",
+											(long int)entry->address,
+											(long int)(a-entry->address), 
+											entry->name);
+			return 1;
+		}
+		it = it->next;
+	}
+	return 0;
+}
+/*
+static void print_btsymbol(void) {
+	struct btsym_l * it = btsym_l;
+	struct btsym * entry;
+	int i=1;
+
+	while( it  ) {
+		entry = it->entry;
+		if (grub_strcmp(entry->name,"grub_cmd_hello") == 0)
+		grub_printf("0x%lx sz=0x%lx: %s \n",
+									(long int)entry->address,(long int)entry->size,
+									entry->name);
+		it = it->next;
+		i++;
+	}
+}
+*/
+static grub_addr_t * get_fp(void) {
+	grub_addr_t * a;
+	asm ("movl %%ebp, %0;"
+			:"=r"(a)        /* output */
+			);
+	return a;
+}
+
+
+void grub_backtrace (void) {
+	grub_addr_t * a;
+	int i =1;
+
+	grub_printf("\nBacktrace:\n");
+	a = get_fp();
+	
+	while ( (i <= BTSYM_DISPLAY) && get_btsymbol(*(a+1)) ) {
+		a= (grub_addr_t *)(*a);
+		i++;
+  }
+}
+
+
+void grub_btregister_symbol (const char* name ,void * addr,grub_size_t sz) {
+
+	struct btsym_l * new_entry;
+	struct btsym * new_symbol;
+
+	if (addr) {
+		new_symbol = (struct btsym *) grub_malloc(sizeof(*new_symbol));
+		grub_strncpy(new_symbol->name,name,MAX_SYMNAME_L);
+		new_symbol->name[MAX_SYMNAME_L] = '\0';
+		new_symbol->address = (grub_addr_t) addr;
+		new_symbol->size = sz;
+
+		new_entry = (struct btsym_l *) grub_malloc(sizeof(*new_entry));
+		new_entry->entry = new_symbol;
+		new_entry->next = btsym_l;
+		btsym_l = new_entry;
+	}
+}
+// Don't unregister a kernel symbol !
+void grub_btunregister_symbol (void * address) {
+	struct btsym_l * next_entry, * entry = btsym_l;
+	grub_addr_t addr = (grub_addr_t) address;
+	
+	if (! entry)
+		return;
+	
+	if (entry->entry->address == addr ) {
+		btsym_l = entry->next;
+		grub_free(entry->entry);
+		grub_free(entry);
+		return;
+	}
+	
+	next_entry = entry->next;
+	
+	while (next_entry) {
+		if (next_entry->entry->address == addr ) {
+			entry->next=next_entry->next;
+			grub_free(next_entry->entry);
+			grub_free(next_entry);
+			return;
+		}
+		entry = next_entry;
+		next_entry=next_entry->next;
+	}
+	
+}
+
+void grub_init_btsym (void) {
+	
+	struct btsym_l * new_entry;
+	int i=0;
+	
+	if (btsym_l)
+		return;
+
+	while (btsym_list[i].address) {
+		new_entry = (struct btsym_l *) grub_malloc(sizeof(*new_entry));
+		new_entry->entry = &(btsym_list[i]);
+		new_entry->next = btsym_l;
+		btsym_l = new_entry;
+		i++;	
+	}
+}
+
+#endif
diff -ru -N -b -B grub2/kern/main.c grub2-backtrace/kern/main.c
--- grub2/kern/main.c	2005-08-12 21:53:32.000000000 +0200
+++ grub2-backtrace/kern/main.c	2005-08-18 21:31:30.089464136 +0200
@@ -28,6 +28,19 @@
 #include <grub/file.h>
 #include <grub/device.h>
 #include <grub/env.h>
+#include <config.h>
+
+#ifndef GRUB_BACKTRACE
+/* if GRUB_BACKTRACE is defined the backtrace functions
+ * are defined in kern/i386/pc/backtrace.c */
+#define NOOP {do {} while (0);}
+void grub_backtrace(void) NOOP;
+void grub_btregister_symbol(const char* name  __attribute__ ((unused)),
+														void* ad  __attribute__ ((unused)),
+														grub_size_t s __attribute__ ((unused))) NOOP;
+void grub_init_btsym (void) NOOP;
+void grub_btunregister_symbol (void * addr __attribute__((unused))) NOOP;
+#endif
 
 /* Load all modules in core.  */
 static void
@@ -112,6 +125,7 @@
 {
   /* First of all, initialize the machine.  */
   grub_machine_init ();
+	grub_init_btsym ();
 
   /* Hello.  */
   grub_setcolorstate (GRUB_TERM_COLOR_HIGHLIGHT);
diff -ru -N -b -B grub2/Makefile.in grub2-backtrace/Makefile.in
--- grub2/Makefile.in	2005-08-09 01:15:21.000000000 +0200
+++ grub2-backtrace/Makefile.in	2005-08-18 21:48:25.675071656 +0200
@@ -82,7 +82,7 @@
 CLEANFILES =
 MOSTLYCLEANFILES = 
 DISTCLEANFILES = config.status config.cache config.log config.h \
-	Makefile stamp-h include/grub/cpu include/grub/machine
+	Makefile stamp-h include/grub/cpu include/grub/machine tags
 MAINTAINER_CLEANFILES = $(srcdir)/configure $(addprefix $(srcdir)/,$(MKFILES))
 
 # The default target.
@@ -212,6 +212,10 @@
 	@echo "$(distdir).tar.gz is ready for distribution" | \
 	  sed 'h;s/./=/g;p;x;p;x'
 
+tags:
+	  CTAGSF=`ctags --version | grep -i exuberant >/dev/null || echo "-T -w -d"`
+		  find . -name \*.[hcS] | xargs ctags -a --typedefs  $(CTAGSF)
+
 check:
 
 .SUFFIX:

Re: backtrace support

[Marco Gerards]

Vincent Guffens <guffens@inma.ucl.ac.be> writes:

Hi Vincent,

> After having searched for the reason of the unaligned pointer caused
> by the nested functions bug, I thought that it could be interesting to
> have a backtrace in grub. It would be triggered in grub_fatal and
> would print the last few function calls that triggered the problem.

Neat!  I would like to have this in GRUB2 for debugging.  If Okuji
likes it too and you can send in a Changelog entry as well, I will
proofread the patch so it can be applied ASAP.

Thanks,
Marco

Re: backtrace support

[Vincent Guffens]

Marco Gerards wrote:
> Vincent Guffens <guffens@inma.ucl.ac.be> writes:
> 
> Hi Vincent,
> 
> 
>>After having searched for the reason of the unaligned pointer caused
>>by the nested functions bug, I thought that it could be interesting to
>>have a backtrace in grub. It would be triggered in grub_fatal and
>>would print the last few function calls that triggered the problem.
> 
> 
> Neat!  I would like to have this in GRUB2 for debugging.  If Okuji
> likes it too and you can send in a Changelog entry as well, I will
> proofread the patch so it can be applied ASAP.

cool ! I will wait a little bit for your comments and those from Okuji 
as there are a few changes I made that I'm not sure if they are very 
clean, especially those in genmk.rb. I used some "cat config.h" and 
maybe there are some better ways. Also, I did not mention that the 
debuging symbol list is somehow redundant with the already existing 
symbol list but that way, it does not modify anything if the backtrace 
support is not compiled in.

Should the Changelog entries be included in the patch or is it generated 
with the cvs log and it should then be sent in the mail body ?

Re: backtrace support

[Yoshinori K. Okuji]

On Thursday 18 August 2005 23:19, Vincent Guffens wrote:
> cool ! I will wait a little bit for your comments and those from Okuji
> as there are a few changes I made that I'm not sure if they are very
> clean, especially those in genmk.rb.

Good job, Vincent.

> I used some "cat config.h" and 
> maybe there are some better ways. 

Possibly you can substitute a variable in Makefile.in and use the variable.

> Should the Changelog entries be included in the patch or is it generated
> with the cvs log and it should then be sent in the mail body ?

ChangeLog must be handwritten. As long as it is written well, I don't care 
about how you send it. Using the Change Log mode in Emacs would help you a 
lot.

It is still necessary to clean up the patch. Running make twice does not sound 
good. One easy way is to build a separate backtrace module outside the 
kernel, and load it as soon as possible, like the normal module.

Also, I think it would be better to rename --enable-backtrace to 
--enable-debug, so that we can enable all debugging features by one option 
(when we get more debugging features, this could be useful).

Okuji