Skip to contentSkip to conversation

fix weak symbol handling

module-init-tools Richard Henderson 3 participants 8 messages Jan 13, 2003 – Jan 17, 2003

lore
Share & tools
lore
#0 lore raw

The pair to the kernel patch posted a moment ago.

r~

depmod.c +2 -2
diff -rup module-init-tools-0.9.8/depmod.c module-init-tools-0.9.8-new/depmod.c
@@ -98,7 +98,7 @@ void add_symbol(const char *name, struct
 
 static int print_unknown;
 
-struct module *find_symbol(const char *name, const char *modname)
+struct module *find_symbol(const char *name, const char *modname, int weak)
 {
 	struct symbol *s;
 
@@ -108,7 +108,7 @@ struct module *find_symbol(const char *n
 	}
 
 	/* Ignore __start_* and __stop_* like kernel might. */
-	if (print_unknown
+	if (print_unknown && !weak
 	    && (strncmp(name, "__start_", strlen("__start_")) != 0
 		|| strncmp(name, "__stop_", strlen("__stop_")) != 0))
 		warn("%s needs unknown symbol %s\n", modname, name);
depmod.h +1 -1
diff -rup module-init-tools-0.9.8/depmod.h module-init-tools-0.9.8-new/depmod.h
@@ -12,7 +12,7 @@ void *do_nofail(void *ptr, const char *f
 #define NOFAIL(ptr)	do_nofail((ptr), __FILE__, __LINE__, #ptr)
 
 void add_symbol(const char *name, struct module *owner);
-struct module *find_symbol(const char *name, const char *modname);
+struct module *find_symbol(const char *name, const char *modname, int weak);
 void add_dep(struct module *mod, struct module *depends_on);
 
 struct module
moduleops.c +6 -2
diff -rup module-init-tools-0.9.8/moduleops.c module-init-tools-0.9.8-new/moduleops.c
@@ -10,10 +10,14 @@
 #include "tables.h"
 
 #define PERBIT(x) x##32
-#define ELFPERBIT(x) Elf32_##x
+#define ElfPERBIT(x) Elf32_##x
+#define ELFPERBIT(x) ELF32_##x
 #include "moduleops_core.c"
+
 #undef PERBIT
+#undef ElfPERBIT
 #undef ELFPERBIT
 #define PERBIT(x) x##64
-#define ELFPERBIT(x) Elf64_##x
+#define ElfPERBIT(x) Elf64_##x
+#define ELFPERBIT(x) ELF64_##x
 #include "moduleops_core.c"
moduleops_core.c +10 -11
diff -rup module-init-tools-0.9.8/moduleops_core.c module-init-tools-0.9.8-new/moduleops_core.c
@@ -1,9 +1,9 @@
 /* Load the given section: NULL on error. */
-static void *PERBIT(load_section)(ELFPERBIT(Ehdr) *hdr,
+static void *PERBIT(load_section)(ElfPERBIT(Ehdr) *hdr,
 			    const char *secname,
 			    unsigned long *size)
 {
-	ELFPERBIT(Shdr) *sechdrs;
+	ElfPERBIT(Shdr) *sechdrs;
 	unsigned int i;
 	char *secnames;
 
@@ -64,7 +64,7 @@ static void PERBIT(calculate_deps)(struc
 	unsigned int i;
 	unsigned long size;
 	char *strings;
-	ELFPERBIT(Sym) *syms;
+	ElfPERBIT(Sym) *syms;
 
 	strings = PERBIT(load_section)(module->mmap, ".strtab", &size);
 	syms = PERBIT(load_section)(module->mmap, ".symtab", &size);
@@ -77,16 +77,15 @@ static void PERBIT(calculate_deps)(struc
 
 	module->num_deps = 0;
 	module->deps = NULL;
-	for (i = 0; i < size / sizeof(syms[0]); i++) {
+	for (i = 1; i < size / sizeof(syms[0]); i++) {
 		if (syms[i].st_shndx == SHN_UNDEF) {
 			/* Look for symbol */
 			const char *name = strings + syms[i].st_name;
 			struct module *owner;
+			int weak;
 
-			if (strcmp(name, "") == 0)
-				continue;
-
-			owner = find_symbol(name, module->pathname);
+			weak = ELFPERBIT(ST_BIND)(syms[i].st_info) == STB_WEAK;
+			owner = find_symbol(name, module->pathname, weak);
 			if (owner) {
 				if (verbose)
 					printf("%s needs \"%s\": %s\n",
@@ -98,13 +97,13 @@ static void PERBIT(calculate_deps)(struc
 	}
 }
 
-static void *PERBIT(deref_sym)(ELFPERBIT(Ehdr) *hdr, const char *name)
+static void *PERBIT(deref_sym)(ElfPERBIT(Ehdr) *hdr, const char *name)
 {
 	unsigned int i;
 	unsigned long size;
 	char *strings;
-	ELFPERBIT(Sym) *syms;
-	ELFPERBIT(Shdr) *sechdrs;
+	ElfPERBIT(Sym) *syms;
+	ElfPERBIT(Shdr) *sechdrs;
 
 	sechdrs = (void *)hdr + hdr->e_shoff;
 	strings = PERBIT(load_section)(hdr, ".strtab", &size);
#1 lore raw

In message <[email protected]> you write:

Quoted 1 line

The pair to the kernel patch posted a moment ago.

So the semantics you want are that if A declares a weak symbol S, and B exports a (presumably non-weak) symbol S, then A depends on B?

I think that's right, given that that is what would happen if A and B were builtin.

Now, what does Dave think about using weak symbols? Or is this Alpha-specific?

Applied,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

#2 lore raw
Quoted 2 lines · Rusty Russell

So the semantics you want are that if A declares a weak symbol S, and B exports a (presumably non-weak) symbol S, then A depends on B?

No. The semantics I need is if A references a weak symbol S and *no one* implements it, then S resolves to NULL.

r~

#3 lore raw

In message <[email protected]> you write:

Quoted 6 lines · Rusty Russell
Quoted 2 lines

So the semantics you want are that if A declares a weak symbol S, and B exports a (presumably non-weak) symbol S, then A depends on B?

No. The semantics I need is if A references a weak symbol S and *no one* implements it, then S resolves to NULL.

Sorry, I was unclear. I want to know the dependency semantics:

If B exports S, should depmod believe A needs B, or not? Your patch leaves that semantic (all it does is suppress the errors).

I'm not sure what semantics are "right", since I don't know what you're trying to do, or what is wrong with get_symbol().

Hope that clarifies?
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

#4 lore raw
Quoted 7 lines · Rusty Russell
Quoted 2 lines

No. The semantics I need is if A references a weak symbol S and *no one* implements it, then S resolves to NULL.

Sorry, I was unclear. I want to know the dependency semantics:

If B exports S, should depmod believe A needs B, or not? Your patch leaves that semantic (all it does is suppress the errors).

Well, that depends on whether A defines S or not. If A does define S, then I don't care. I'd say "no", A does not depend on B. If A does not define S, then most definitely "yes", as with any other definition.

Quoted 2 lines

I'm not sure what semantics are "right", since I don't know what you're trying to do, or what is wrong with get_symbol().

I just told you. Quoted again above. Perhaps the following dummy module will make things even clearer.

---
#include <linux/module.h>
#include <linux/init.h>

extern int not_defined __attribute__((weak));

static int init(void)
{
return &not_defined ? -EINVAL : 0;
}

static void fini(void)
{
}

module_init(init);
module_exit(fini);
---

You should be able to load this module.

r~

#5 lore raw
Quoted 11 lines · [email protected] said:
Quoted 4 lines
Quoted 2 lines

No. The semantics I need is if A references a weak symbol S and *no one* implements it, then S resolves to NULL.

Sorry, I was unclear. I want to know the dependency semantics:

If B exports S, should depmod believe A needs B, or not? Your patch leaves that semantic (all it does is suppress the errors). Well, that depends on whether A defines S or not. If A does define S, then I don't care. I'd say "no", A does not depend on B. If A does not define S, then most definitely "yes", as with any other definition.

As long as doing so doesn't make modprobe fail to load A when B isn't present or refuses to load. Otherwise what was the point in making it weak?

--
dwmw2

#6 lore raw

In message <[email protected]> you write:

Quoted 9 lines
Quoted 4 lines · [email protected] said:

Well, that depends on whether A defines S or not. If A does define S, then I don't care. I'd say "no", A does not depend on B. If A does not define S, then most definitely "yes", as with any other definition.

As long as doing so doesn't make modprobe fail to load A when B isn't present or refuses to load. Otherwise what was the point in making it weak?

If A depends on B, then modprobe will give a warning if "modprobe A" fails to load B for some reason. If B doesn't exist, then modprobe wouldn't know anything about it (presumably).

Hope that clarifies,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

#7 lore raw
Quoted 3 lines · [email protected] said:

If A depends on B, then modprobe will give a warning if "modprobe A"
fails to load B for some reason. If B doesn't exist, then modprobe wouldn't know anything about it (presumably).

A warning is fine. If A _weakly_ depends on B, a failure to load A even though B decided when asked that it didn't really want to initialise itself is not fine :)

--
dwmw2