modules without a .toc section not loading

modules without a .toc section not loading

[Robin H. Johnson]

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

Hi,

I recently got myself a quad G5, and I'm working on a few things, and
I've run into what is either a ppc-specific GCC bug, or a module loader
bug. While the bug shows up in my custom module, it also shows up in two
other places in the current tree.

This was mentioned on the list in February, and Paulus asked for some
off-list input from the reporter, but I find nothing else.

Output in dmesg after insmod:
windfarm_pid: doesn't contain .toc or .stubs.

- My current tree is Paulus's powerpc.git tree.
- Tested with gcc 3.4.4 and 3.4.6 - both show the problem.
- in-tree modules that show it: windfarm_pid, ide-generic

Checking:
# objdump -h windfarm_pid.ko | grep toc
(not found)
# objdump -h windfarm_pid.o | grep toc
(not found)

Similarities in affected code:
All of the affected code seems to be very simple, and does not reference
very many external symbols, or any significant external symbols.

If I add some dud code with an external symbol (I added kalloc/kfree to
windfarm_pid), a .toc section does get generated correctly.

Hypothesis:
GCC isn't generating a .toc section because the source file doesn't
need one by it's consideration. 

Ideas for Solutions:
1. Either the kernel needs to realize that it does not need a .toc
section (I'm uncertain about this, being new to PPC).
2. GCC is at fault, and it should be generating a .toc.

-- 
Robin Hugh Johnson
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85

[-- Attachment #2: Type: application/pgp-signature, Size: 241 bytes --]

Re: modules without a .toc section not loading

[Robin H. Johnson]

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

On Thu, Apr 20, 2006 at 09:58:30AM +0930, Alan Modra wrote:
> > Ideas for Solutions:
> > 1. Either the kernel needs to realize that it does not need a .toc
> > section (I'm uncertain about this, being new to PPC).
> > 2. GCC is at fault, and it should be generating a .toc.
> The former.
Ok, so in module_64.c, under what cases is .toc NOT needed?

I dug at the code, and the R_PPC_REL24 blocks need the contents of the
.toc (via stub_for_addr -> create_stub -> my_r2), and I'm not certain
about it.

The R_PPC64_TOC/R_PPC64_TOC16/R_PPC64_TOC16_DS blocks shouldn't be hit
when there isn't a .toc* from what I figure.

If the R_PPC_REL24 block won't be hit when there isn't a .toc, the check
in module_frob_arch_sections, lines 194-197 can just be shortened as a
fix to the problem.

-- 
Robin Hugh Johnson
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85

[-- Attachment #2: Type: application/pgp-signature, Size: 241 bytes --]

Re: modules without a .toc section not loading

[Alan Modra]

On Wed, Apr 19, 2006 at 05:54:45PM -0700, Robin H. Johnson wrote:
> On Thu, Apr 20, 2006 at 09:58:30AM +0930, Alan Modra wrote:
> > > Ideas for Solutions:
> > > 1. Either the kernel needs to realize that it does not need a .toc
> > > section (I'm uncertain about this, being new to PPC).
> > > 2. GCC is at fault, and it should be generating a .toc.
> > The former.
> Ok, so in module_64.c, under what cases is .toc NOT needed?
> 
> I dug at the code, and the R_PPC_REL24 blocks need the contents of the
> .toc (via stub_for_addr -> create_stub -> my_r2), and I'm not certain
> about it.

Yes, the stubs need some reasonable r2 value.

> The R_PPC64_TOC/R_PPC64_TOC16/R_PPC64_TOC16_DS blocks shouldn't be hit
> when there isn't a .toc* from what I figure.

Yes, and finding such a reloc should trigger an error.

> If the R_PPC_REL24 block won't be hit when there isn't a .toc, the check
> in module_frob_arch_sections, lines 194-197 can just be shortened as a
> fix to the problem.

You could have an R_PPC_REL24 in a module (a function call into another
module or the kernel) without a .toc.  A module without a .toc section
won't itself use r2 for anything, but the any stubs added do need r2.

I'm surprised no one has fixed this.  Here's a totally untested patch
that ought to do the trick (or if it doesn't, should put you on the
right path).

--- linux-2.6.16/arch/powerpc/kernel/module_64.c~	2006-01-18 14:17:29.000000000 +1030
+++ linux-2.6.16/arch/powerpc/kernel/module_64.c	2006-04-20 18:22:42.000000000 +0930
@@ -191,8 +191,15 @@ int module_frob_arch_sections(Elf64_Ehdr
 				 (void *)hdr
 				 + sechdrs[sechdrs[i].sh_link].sh_offset);
 	}
-	if (!me->arch.stubs_section || !me->arch.toc_section) {
-		printk("%s: doesn't contain .toc or .stubs.\n", me->name);
+
+	/* If we don't have a .toc, just use .stubs.  Nothing in the
+	   module itself should use r2, but we need to set r2 in case
+	   the module calls out to other functions. */
+	if (!me->arch.toc_section)
+		me->arch.toc_section = me->arch.stubs_section;
+
+	if (!me->arch.stubs_section) {
+		printk("%s: doesn't contain .stubs.\n", me->name);
 		return -ENOEXEC;
 	}
 
@@ -338,11 +345,21 @@ int apply_relocate_add(Elf64_Shdr *sechd
 			break;
 
 		case R_PPC64_TOC:
+			if (me->arch.toc_section == me->arch.stubs_section) {
+				printk("%s: TOC reloc without .toc\n",
+				       me->name);
+				return -ENOEXEC;
+			}
 			*(unsigned long *)location = my_r2(sechdrs, me);
 			break;
 
 		case R_PPC64_TOC16:
-			/* Subtact TOC pointer */
+			if (me->arch.toc_section == me->arch.stubs_section) {
+				printk("%s: TOC16 reloc without .toc\n",
+				       me->name);
+				return -ENOEXEC;
+			}
+			/* Subtract TOC pointer */
 			value -= my_r2(sechdrs, me);
 			if (value + 0x8000 > 0xffff) {
 				printk("%s: bad TOC16 relocation (%lu)\n",
@@ -355,7 +372,12 @@ int apply_relocate_add(Elf64_Shdr *sechd
 			break;
 
 		case R_PPC64_TOC16_DS:
-			/* Subtact TOC pointer */
+			if (me->arch.toc_section == me->arch.stubs_section) {
+				printk("%s: TOC16_DS reloc without .toc\n",
+				       me->name);
+				return -ENOEXEC;
+			}
+			/* Subtract TOC pointer */
 			value -= my_r2(sechdrs, me);
 			if ((value & 3) != 0 || value + 0x8000 > 0xffff) {
 				printk("%s: bad TOC16_DS relocation (%lu)\n",

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: modules without a .toc section not loading

[Alan Modra]

On Sat, Apr 22, 2006 at 08:56:14PM -0700, Robin H. Johnson wrote:
> On Thu, Apr 20, 2006 at 06:24:01PM +0930, Alan Modra wrote:
> > > I dug at the code, and the R_PPC_REL24 blocks need the contents of the
> > > .toc (via stub_for_addr -> create_stub -> my_r2), and I'm not certain
> > > about it.
> > Yes, the stubs need some reasonable r2 value.
> Using the .stubs values for the .toc seems to work, but I should
> construct a better test case for it, to double check.
> 
> > > The R_PPC64_TOC/R_PPC64_TOC16/R_PPC64_TOC16_DS blocks shouldn't be hit
> > > when there isn't a .toc* from what I figure.
> > Yes, and finding such a reloc should trigger an error.
> Ok, there is a case where R_PPC64_TOC exists and there isn't a .toc, and
> I suspect you're just the right person to ask...

Oops.  R_PPC64_TOC without a toc isn't an error, despite it looking
self-contradictory.

[snip about R_PPC64_TOC in .opd]
> It seems that these R_PPC64_TOC entries are safe to completely ignore -
> is this correct? 

They are the toc pointer value used by functions defined in the module.
Since we are using .stubs+0x8000, that should be the value of these
relocs.  There may be situations where the opd entry escapes out of
the module as a function pointer (eg. via an initialised global function
pointer variable), so we'd better set them to the correct value.

In fact, it probably isn't worth doing error checking of any of the
R_PPC64_TOC* relocs.  If I hadn't tried to be fancy with error checks,
it seems my original patch would have been good.  Oh well, revised patch
follows.

[snip]
> I have searched Google, but not actually found any documentation on what
> the format of the .opd section is supposed to be. LSB-3.1 just says it
> has to exist, without saying what it's format is.

First word is the function entry point, second word is the toc pointer
used by the function, third word is the static chain pointer (unused by
C, but used in languages like Pascal).  Some Redhat compilers omit the
third word entirely, so opd entries may be either 16 bytes or 24 bytes
apart.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

--- linux-2.6.16/arch/powerpc/kernel/module_64.c~	2006-01-18 14:17:29.000000000 +1030
+++ linux-2.6.16/arch/powerpc/kernel/module_64.c	2006-04-24 22:01:19.000000000 +0930
@@ -191,11 +191,19 @@ int module_frob_arch_sections(Elf64_Ehdr
 				 (void *)hdr
 				 + sechdrs[sechdrs[i].sh_link].sh_offset);
 	}
-	if (!me->arch.stubs_section || !me->arch.toc_section) {
-		printk("%s: doesn't contain .toc or .stubs.\n", me->name);
+
+	if (!me->arch.stubs_section) {
+		printk("%s: doesn't contain .stubs.\n", me->name);
 		return -ENOEXEC;
 	}
 
+	/* If we don't have a .toc, just use .stubs.  We need to set r2
+	   to some reasonable value in case the module calls out to
+	   other functions via a stub, or if a function pointer escapes
+	   the module by some means.  */
+	if (!me->arch.toc_section)
+		me->arch.toc_section = me->arch.stubs_section;
+
 	/* Override the stubs size */
 	sechdrs[me->arch.stubs_section].sh_size = get_stubs_size(hdr, sechdrs);
 	return 0;
@@ -342,7 +350,7 @@ int apply_relocate_add(Elf64_Shdr *sechd
 			break;
 
 		case R_PPC64_TOC16:
-			/* Subtact TOC pointer */
+			/* Subtract TOC pointer */
 			value -= my_r2(sechdrs, me);
 			if (value + 0x8000 > 0xffff) {
 				printk("%s: bad TOC16 relocation (%lu)\n",
@@ -355,7 +363,7 @@ int apply_relocate_add(Elf64_Shdr *sechd
 			break;
 
 		case R_PPC64_TOC16_DS:
-			/* Subtact TOC pointer */
+			/* Subtract TOC pointer */
 			value -= my_r2(sechdrs, me);
 			if ((value & 3) != 0 || value + 0x8000 > 0xffff) {
 				printk("%s: bad TOC16_DS relocation (%lu)\n",