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

[Christian Franke]

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  <address@hidden>

        * 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.


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 @@
 address@hidden@
 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}
 address@hidden: #{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 += address@hidden #{objs_str}
+    "CLEANFILES += address@hidden(EXEEXT) #{objs_str}
 MOSTLYCLEANFILES += #{deps_str}
 
 address@hidden: $(#{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 @@
 address@hidden@
 address@hidden@
 
+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
+
 
 
 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;