Re: [PATCH] Handle C symbols with leading underscore (HAVE_ASM_USCORE)

[Christian Franke <Christian.Franke@t-online.de>]

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

Robert Millan wrote:
> On Sat, Nov 10, 2007 at 08:03:57PM +0100, Christian Franke wrote:
>   
>> This patch fixes this by ignoring the difference. It actually works to 
>> load modules compiled on Linux by a kernel compiled on Cygwin (with 
>> underscores) and vice versa.
>>
>> [...]
>> +
>> +  /* Ignore leading underscore used for C symbols.
>> +     Done at runtime to allow loading modules compiled on other OS.  */
>> +  if (*name == '_')
>> +    name++;
>>     
>
> Is that something we really want?  We never made any efforts at maintaining
> ABI between kernel and modules, or in allowing this kind of combinations.  If
> we go this path, it can mean more work later (e.g. support for building kernel
> and modules with different versions of GCC, etc).
>
>   

Agree.

Should the kernel probably check whether a module is build for the same 
target? This would avoid interesting behaviour if a foreign module is 
accidentally load for some reason. It could e.g. be done by checking a 
symbol derived from $target.


> Could you give an example of a situation in which this would be useful?
>
>   

There probably is none, therefore the (older) patch for the other 
variant is attached :-)

Christian

2007-11-11  Christian Franke  <franke@computer.org>

	* genkernsyms.sh.in: Handle HAVE_ASM_USCORE case.

	* genmk.rb: Handle HAVE_ASM_USCORE case in strip command.
	Add EXEEXT to CLEANFILES.

	* gensymlist.sh.in: Handle HAVE_ASM_USCORE case.

	* kern/dl.c (grub_dl_resolve_symbols): Add check for grub_mod_init
	and grub_mod_fini for symbols without a type. Handle HAVE_ASM_USCORE
	case for these symbols.
	(grub_dl_resolve_dependencies): Add check for trailing nullbytes
	in symbol table. This fixes an infinite loop if table is zero filled.



[-- Attachment #2: grub2-asm_uscore-2.patch --]
[-- Type: text/x-patch, Size: 5191 bytes --]

diff -rup grub2.orig/genkernsyms.sh.in grub2/genkernsyms.sh.in
--- grub2.orig/genkernsyms.sh.in	2006-07-12 22:42:52.000000000 +0200
+++ grub2/genkernsyms.sh.in	2007-10-06 12:59:29.000000000 +0200
@@ -16,9 +16,12 @@
 srcdir=@srcdir@
 CC="@CC@"
 
+u=
+grep "^#define HAVE_ASM_USCORE" config.h >/dev/null 2>&1 && u="_"
+
 $CC -DGRUB_SYMBOL_GENERATOR=1 -E -I. -Iinclude -I$srcdir/include $* \
   | grep -v '^#' \
   | sed -n \
-        -e '/EXPORT_FUNC *([a-zA-Z0-9_]*)/{s/.*EXPORT_FUNC *(\([a-zA-Z0-9_]*\)).*/\1 kernel/;p;}' \
-        -e '/EXPORT_VAR *([a-zA-Z0-9_]*)/{s/.*EXPORT_VAR *(\([a-zA-Z0-9_]*\)).*/\1 kernel/;p;}' \
+        -e '/EXPORT_FUNC *([a-zA-Z0-9_]*)/{s/.*EXPORT_FUNC *(\([a-zA-Z0-9_]*\)).*/'"$u"'\1 kernel/;p;}' \
+        -e '/EXPORT_VAR *([a-zA-Z0-9_]*)/{s/.*EXPORT_VAR *(\([a-zA-Z0-9_]*\)).*/'"$u"'\1 kernel/;p;}' \
   | sort -u
diff -rup grub2.orig/genmk.rb grub2/genmk.rb
--- grub2.orig/genmk.rb	2007-10-20 22:49:38.968750000 +0200
+++ grub2/genmk.rb	2007-11-10 19:37:17.562500000 +0100
@@ -115,7 +115,7 @@ UNDSYMFILES += #{undsym}
 #{@name}: #{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 $@
+	$(STRIP) --strip-unneeded -K grub_mod_init -K grub_mod_fini -K _grub_mod_init -K _grub_mod_fini -R .note -R .comment $@
 
 #{pre_obj}: $(#{prefix}_DEPENDENCIES) #{objs_str}
 	-rm -f $@
@@ -187,7 +187,7 @@ class Utility
     deps = objs.collect {|obj| obj.suffix('d')}
     deps_str = deps.join(' ');
 
-    "CLEANFILES += #{@name} #{objs_str}
+    "CLEANFILES += #{@name}$(EXEEXT) #{objs_str}
 MOSTLYCLEANFILES += #{deps_str}
 
 #{@name}: $(#{prefix}_DEPENDENCIES) #{objs_str}
diff -rup grub2.orig/gensymlist.sh.in grub2/gensymlist.sh.in
--- grub2.orig/gensymlist.sh.in	2007-07-22 01:32:18.000000000 +0200
+++ grub2/gensymlist.sh.in	2007-10-07 17:47:37.000000000 +0200
@@ -16,6 +16,8 @@
 srcdir=@srcdir@
 CC=@CC@
 
+u=
+grep "^#define HAVE_ASM_USCORE" config.h >/dev/null 2>&1 && u="_"
 
 cat <<EOF
 /* This file is automatically generated by gensymlist.sh. DO NOT EDIT! */
@@ -60,8 +62,8 @@ EOF
 $CC -DGRUB_SYMBOL_GENERATOR=1 -E -I. -Iinclude -I$srcdir/include $* \
   | grep -v '^#' \
   | sed -n \
-        -e '/EXPORT_FUNC *([a-zA-Z0-9_]*)/{s/.*EXPORT_FUNC *(\([a-zA-Z0-9_]*\)).*/      {"\1", \1},/;p;}' \
-        -e '/EXPORT_VAR *([a-zA-Z0-9_]*)/{s/.*EXPORT_VAR *(\([a-zA-Z0-9_]*\)).*/      {"\1", \&\1},/;p;}' \
+        -e '/EXPORT_FUNC *([a-zA-Z0-9_]*)/{s/.*EXPORT_FUNC *(\([a-zA-Z0-9_]*\)).*/      {"'"$u"'\1", \1},/;p;}' \
+        -e '/EXPORT_VAR *([a-zA-Z0-9_]*)/{s/.*EXPORT_VAR *(\([a-zA-Z0-9_]*\)).*/      {"'"$u"'\1", \&\1},/;p;}' \
   | sort -u
 
 cat <<EOF
diff -rup grub2.orig/kern/dl.c grub2/kern/dl.c
--- grub2.orig/kern/dl.c	2007-07-22 01:32:26.000000000 +0200
+++ grub2/kern/dl.c	2007-11-11 18:01:50.578125000 +0100
@@ -53,6 +53,12 @@ typedef Elf64_Sym Elf_Sym;
 
 #endif
 
+#ifdef HAVE_ASM_USCORE
+# define SYM_USCORE "_"
+#else
+# define SYM_USCORE ""
+#endif
+
 \f
 
 struct grub_dl_list
@@ -347,6 +353,7 @@ grub_dl_resolve_symbols (grub_dl_t mod, 
       unsigned char type = ELF_ST_TYPE (sym->st_info);
       unsigned char bind = ELF_ST_BIND (sym->st_info);
       const char *name = str + sym->st_name;
+      int check_mod_func = 0;
       
       switch (type)
 	{
@@ -359,6 +366,12 @@ grub_dl_resolve_symbols (grub_dl_t mod, 
 		return grub_error (GRUB_ERR_BAD_MODULE,
 				   "the symbol `%s' not found", name);
 	    }
+	  else if (sym->st_name != 0 && bind == STB_LOCAL)
+	    { /* Static functions have no type if initial format was not ELF.  */
+	      sym->st_value += (Elf_Addr) grub_dl_get_section_addr (mod,
+								    sym->st_shndx);
+	      check_mod_func = 1;
+	    }
 	  else
 	    sym->st_value = 0;
 	  break;
@@ -374,14 +387,11 @@ grub_dl_resolve_symbols (grub_dl_t mod, 
 	case STT_FUNC:
 	  sym->st_value += (Elf_Addr) grub_dl_get_section_addr (mod,
 								sym->st_shndx);
-	  if (bind != STB_LOCAL)
+	  if (bind == STB_LOCAL)
+	    check_mod_func = 1;
+	  else	
 	    if (grub_dl_register_symbol (name, (void *) sym->st_value, mod))
 	      return grub_errno;
-	  
-	  if (grub_strcmp (name, "grub_mod_init") == 0)
-	    mod->init = (void (*) (grub_dl_t)) sym->st_value;
-	  else if (grub_strcmp (name, "grub_mod_fini") == 0)
-	    mod->fini = (void (*) (void)) sym->st_value;
 	  break;
 
 	case STT_SECTION:
@@ -397,6 +407,13 @@ grub_dl_resolve_symbols (grub_dl_t mod, 
 	  return grub_error (GRUB_ERR_BAD_MODULE,
 			     "unknown symbol type `%d'", (int) type);
 	}
+      if (check_mod_func)
+        {
+	  if (grub_strcmp (name, SYM_USCORE "grub_mod_init") == 0)
+	    mod->init = (void (*) (grub_dl_t)) sym->st_value;
+	  else if (grub_strcmp (name, SYM_USCORE "grub_mod_fini") == 0)
+	    mod->fini = (void (*) (void)) sym->st_value;
+        }
     }
 
   return GRUB_ERR_NONE;
@@ -454,7 +471,7 @@ grub_dl_resolve_dependencies (grub_dl_t 
 	const char *name = (char *) e + s->sh_offset;
 	const char *max = name + s->sh_size;
 
-	while (name < max)
+	while (name < max && *name) /* Segment may contain trailing 0.  */
 	  {
 	    grub_dl_t m;
 	    grub_dl_dep_t dep;