2012-08-07 09:44:06

by Jonas Gorski

[permalink] [raw]
Subject: [PATCH] MIPS: fix module.c build for 32 bit

Fixes the following build failure introduced by "Make most arch
asm/module.h files use asm-generic/module.h".

CC arch/mips/kernel/module.o
arch/mips/kernel/module.c:250:14: error: 'reloc_handlers_rela' defined but not used [-Werror=unused-variable]
cc1: all warnings being treated as errors

make[6]: *** [arch/mips/kernel/module.o] Error 1

Signed-off-by: Jonas Gorski <[email protected]>

---
I don't mind this patch being squashed into the original patch. The
patch isn't in any stable git yet, so I assume any git id would be
outdated soon anyway.

Linus, I CC'd you because there already is a pending pull request for
this patch.

David, it would have been nice if the mentioned patch had made it to
linux-mips. I just caught this more or less by accident by building
linux-next.

arch/mips/kernel/module.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c
index 1500c80..afbd717 100644
--- a/arch/mips/kernel/module.c
+++ b/arch/mips/kernel/module.c
@@ -65,12 +65,14 @@ static int apply_r_mips_32_rel(struct module *me, u32 *location, Elf_Addr v)
return 0;
}

+#ifdef CONFIG_MODULES_USE_ELF_RELA
static int apply_r_mips_32_rela(struct module *me, u32 *location, Elf_Addr v)
{
*location = v;

return 0;
}
+#endif

static int apply_r_mips_26_rel(struct module *me, u32 *location, Elf_Addr v)
{
@@ -93,6 +95,7 @@ static int apply_r_mips_26_rel(struct module *me, u32 *location, Elf_Addr v)
return 0;
}

+#ifdef CONFIG_MODULES_USE_ELF_RELA
static int apply_r_mips_26_rela(struct module *me, u32 *location, Elf_Addr v)
{
if (v % 4) {
@@ -112,6 +115,7 @@ static int apply_r_mips_26_rela(struct module *me, u32 *location, Elf_Addr v)

return 0;
}
+#endif

static int apply_r_mips_hi16_rel(struct module *me, u32 *location, Elf_Addr v)
{
@@ -134,6 +138,7 @@ static int apply_r_mips_hi16_rel(struct module *me, u32 *location, Elf_Addr v)
return 0;
}

+#ifdef CONFIG_MODULES_USE_ELF_RELA
static int apply_r_mips_hi16_rela(struct module *me, u32 *location, Elf_Addr v)
{
*location = (*location & 0xffff0000) |
@@ -141,6 +146,7 @@ static int apply_r_mips_hi16_rela(struct module *me, u32 *location, Elf_Addr v)

return 0;
}
+#endif

static int apply_r_mips_lo16_rel(struct module *me, u32 *location, Elf_Addr v)
{
@@ -206,6 +212,7 @@ out_danger:
return -ENOEXEC;
}

+#ifdef CONFIG_MODULES_USE_ELF_RELA
static int apply_r_mips_lo16_rela(struct module *me, u32 *location, Elf_Addr v)
{
*location = (*location & 0xffff0000) | (v & 0xffff);
@@ -237,6 +244,7 @@ static int apply_r_mips_highest_rela(struct module *me, u32 *location,

return 0;
}
+#endif

static int (*reloc_handlers_rel[]) (struct module *me, u32 *location,
Elf_Addr v) = {
@@ -247,6 +255,7 @@ static int (*reloc_handlers_rel[]) (struct module *me, u32 *location,
[R_MIPS_LO16] = apply_r_mips_lo16_rel
};

+#ifdef CONFIG_MODULES_USE_ELF_RELA
static int (*reloc_handlers_rela[]) (struct module *me, u32 *location,
Elf_Addr v) = {
[R_MIPS_NONE] = apply_r_mips_none,
@@ -258,6 +267,7 @@ static int (*reloc_handlers_rela[]) (struct module *me, u32 *location,
[R_MIPS_HIGHER] = apply_r_mips_higher_rela,
[R_MIPS_HIGHEST] = apply_r_mips_highest_rela
};
+#endif

int apply_relocate(Elf_Shdr *sechdrs, const char *strtab,
unsigned int symindex, unsigned int relsec,
--
1.7.2.5


2012-08-13 15:40:00

by David Howells

[permalink] [raw]
Subject: Re: [PATCH] MIPS: fix module.c build for 32 bit


Hi Rusty,

I've fixed up my patch for ARM and pulled Jonas's patch on top of that. Do
you want me to merge them together?

David

2012-08-14 00:23:07

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] MIPS: fix module.c build for 32 bit

On Mon, 13 Aug 2012 16:39:42 +0100, David Howells <[email protected]> wrote:
>
> Hi Rusty,
>
> I've fixed up my patch for ARM and pulled Jonas's patch on top of that. Do
> you want me to merge them together?

Yep, thanks. And might as well sent them straight to Linus; since
linux-next didn't catch this, there's little point baking them there if
we have some acks.

If he misses it, I'll grab them.

Thanks,
Rusty.

2012-08-14 14:08:29

by David Howells

[permalink] [raw]
Subject: Re: [PATCH] MIPS: fix module.c build for 32 bit

Rusty Russell <[email protected]> wrote:

> Yep, thanks. And might as well sent them straight to Linus; since
> linux-next didn't catch this, there's little point baking them there if
> we have some acks.
>
> If he misses it, I'll grab them.

It might have to wait for the next merge window.

David

2012-08-14 15:14:06

by Ralf Baechle

[permalink] [raw]
Subject: [PATCH] MIPS: Fix module.c build for 32 bit

Fixes build failure introduced by "Make most arch asm/module.h files use
asm-generic/module.h" by moving all the RELA processing code to a
separate file to be used only for RELA processing on 64-bit kernels.

CC arch/mips/kernel/module.o
arch/mips/kernel/module.c:250:14: error: 'reloc_handlers_rela' defined but not
used [-Werror=unused-variable]
cc1: all warnings being treated as errors

make[6]: *** [arch/mips/kernel/module.o] Error 1

Signed-off-by: Ralf Baechle <[email protected]>

---
This is bigger than Jonas' suggested patch but far less #ifdefy.
A minimal fix would be to add __maybe_unused to the definition of
reloc_handlers_rela but imho __maybe_unused should be avoided where
possible.

I tested this on top of today's -next but ideally to keep bisectability
it should be applied early in the series before CONFIG_MODULES_USE_ELF_RELA
is introduced which would require trivial tweaking arch/mips/kernel/Makefile
tweaking.

arch/mips/kernel/Makefile | 1 +
arch/mips/kernel/module-rela.c | 145 +++++++++++++++++++++++++++++++++++++++++
arch/mips/kernel/module.c | 123 +---------------------------------
3 files changed, 147 insertions(+), 122 deletions(-)

diff --git a/arch/mips/kernel/Makefile b/arch/mips/kernel/Makefile
index fdaf65e..cd1e6c2 100644
--- a/arch/mips/kernel/Makefile
+++ b/arch/mips/kernel/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_SYNC_R4K) += sync-r4k.o

obj-$(CONFIG_STACKTRACE) += stacktrace.o
obj-$(CONFIG_MODULES) += mips_ksyms.o module.o
+obj-$(CONFIG_MODULES_USE_ELF_RELA) += module-rela.o

obj-$(CONFIG_FUNCTION_TRACER) += mcount.o ftrace.o

diff --git a/arch/mips/kernel/module-rela.c b/arch/mips/kernel/module-rela.c
new file mode 100644
index 0000000..61d6002
--- /dev/null
+++ b/arch/mips/kernel/module-rela.c
@@ -0,0 +1,145 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Copyright (C) 2001 Rusty Russell.
+ * Copyright (C) 2003, 2004 Ralf Baechle ([email protected])
+ * Copyright (C) 2005 Thiemo Seufer
+ */
+
+#include <linux/elf.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/moduleloader.h>
+
+extern int apply_r_mips_none(struct module *me, u32 *location, Elf_Addr v);
+
+static int apply_r_mips_32_rela(struct module *me, u32 *location, Elf_Addr v)
+{
+ *location = v;
+
+ return 0;
+}
+
+static int apply_r_mips_26_rela(struct module *me, u32 *location, Elf_Addr v)
+{
+ if (v % 4) {
+ pr_err("module %s: dangerous R_MIPS_26 RELArelocation\n",
+ me->name);
+ return -ENOEXEC;
+ }
+
+ if ((v & 0xf0000000) != (((unsigned long)location + 4) & 0xf0000000)) {
+ printk(KERN_ERR
+ "module %s: relocation overflow\n",
+ me->name);
+ return -ENOEXEC;
+ }
+
+ *location = (*location & ~0x03ffffff) | ((v >> 2) & 0x03ffffff);
+
+ return 0;
+}
+
+static int apply_r_mips_hi16_rela(struct module *me, u32 *location, Elf_Addr v)
+{
+ *location = (*location & 0xffff0000) |
+ ((((long long) v + 0x8000LL) >> 16) & 0xffff);
+
+ return 0;
+}
+
+static int apply_r_mips_lo16_rela(struct module *me, u32 *location, Elf_Addr v)
+{
+ *location = (*location & 0xffff0000) | (v & 0xffff);
+
+ return 0;
+}
+
+static int apply_r_mips_64_rela(struct module *me, u32 *location, Elf_Addr v)
+{
+ *(Elf_Addr *)location = v;
+
+ return 0;
+}
+
+static int apply_r_mips_higher_rela(struct module *me, u32 *location,
+ Elf_Addr v)
+{
+ *location = (*location & 0xffff0000) |
+ ((((long long) v + 0x80008000LL) >> 32) & 0xffff);
+
+ return 0;
+}
+
+static int apply_r_mips_highest_rela(struct module *me, u32 *location,
+ Elf_Addr v)
+{
+ *location = (*location & 0xffff0000) |
+ ((((long long) v + 0x800080008000LL) >> 48) & 0xffff);
+
+ return 0;
+}
+
+static int (*reloc_handlers_rela[]) (struct module *me, u32 *location,
+ Elf_Addr v) = {
+ [R_MIPS_NONE] = apply_r_mips_none,
+ [R_MIPS_32] = apply_r_mips_32_rela,
+ [R_MIPS_26] = apply_r_mips_26_rela,
+ [R_MIPS_HI16] = apply_r_mips_hi16_rela,
+ [R_MIPS_LO16] = apply_r_mips_lo16_rela,
+ [R_MIPS_64] = apply_r_mips_64_rela,
+ [R_MIPS_HIGHER] = apply_r_mips_higher_rela,
+ [R_MIPS_HIGHEST] = apply_r_mips_highest_rela
+};
+
+int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
+ unsigned int symindex, unsigned int relsec,
+ struct module *me)
+{
+ Elf_Mips_Rela *rel = (void *) sechdrs[relsec].sh_addr;
+ Elf_Sym *sym;
+ u32 *location;
+ unsigned int i;
+ Elf_Addr v;
+ int res;
+
+ pr_debug("Applying relocate section %u to %u\n", relsec,
+ sechdrs[relsec].sh_info);
+
+ for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) {
+ /* This is where to make the change */
+ location = (void *)sechdrs[sechdrs[relsec].sh_info].sh_addr
+ + rel[i].r_offset;
+ /* This is the symbol it is referring to */
+ sym = (Elf_Sym *)sechdrs[symindex].sh_addr
+ + ELF_MIPS_R_SYM(rel[i]);
+ if (IS_ERR_VALUE(sym->st_value)) {
+ /* Ignore unresolved weak symbol */
+ if (ELF_ST_BIND(sym->st_info) == STB_WEAK)
+ continue;
+ printk(KERN_WARNING "%s: Unknown symbol %s\n",
+ me->name, strtab + sym->st_name);
+ return -ENOENT;
+ }
+
+ v = sym->st_value + rel[i].r_addend;
+
+ res = reloc_handlers_rela[ELF_MIPS_R_TYPE(rel[i])](me, location, v);
+ if (res)
+ return res;
+ }
+
+ return 0;
+}
diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c
index 1500c80..da6dcd9 100644
--- a/arch/mips/kernel/module.c
+++ b/arch/mips/kernel/module.c
@@ -53,7 +53,7 @@ void *module_alloc(unsigned long size)
}
#endif

-static int apply_r_mips_none(struct module *me, u32 *location, Elf_Addr v)
+int apply_r_mips_none(struct module *me, u32 *location, Elf_Addr v)
{
return 0;
}
@@ -65,13 +65,6 @@ static int apply_r_mips_32_rel(struct module *me, u32 *location, Elf_Addr v)
return 0;
}

-static int apply_r_mips_32_rela(struct module *me, u32 *location, Elf_Addr v)
-{
- *location = v;
-
- return 0;
-}
-
static int apply_r_mips_26_rel(struct module *me, u32 *location, Elf_Addr v)
{
if (v % 4) {
@@ -93,26 +86,6 @@ static int apply_r_mips_26_rel(struct module *me, u32 *location, Elf_Addr v)
return 0;
}

-static int apply_r_mips_26_rela(struct module *me, u32 *location, Elf_Addr v)
-{
- if (v % 4) {
- pr_err("module %s: dangerous R_MIPS_26 RELArelocation\n",
- me->name);
- return -ENOEXEC;
- }
-
- if ((v & 0xf0000000) != (((unsigned long)location + 4) & 0xf0000000)) {
- printk(KERN_ERR
- "module %s: relocation overflow\n",
- me->name);
- return -ENOEXEC;
- }
-
- *location = (*location & ~0x03ffffff) | ((v >> 2) & 0x03ffffff);
-
- return 0;
-}
-
static int apply_r_mips_hi16_rel(struct module *me, u32 *location, Elf_Addr v)
{
struct mips_hi16 *n;
@@ -134,14 +107,6 @@ static int apply_r_mips_hi16_rel(struct module *me, u32 *location, Elf_Addr v)
return 0;
}

-static int apply_r_mips_hi16_rela(struct module *me, u32 *location, Elf_Addr v)
-{
- *location = (*location & 0xffff0000) |
- ((((long long) v + 0x8000LL) >> 16) & 0xffff);
-
- return 0;
-}
-
static int apply_r_mips_lo16_rel(struct module *me, u32 *location, Elf_Addr v)
{
unsigned long insnlo = *location;
@@ -206,38 +171,6 @@ out_danger:
return -ENOEXEC;
}

-static int apply_r_mips_lo16_rela(struct module *me, u32 *location, Elf_Addr v)
-{
- *location = (*location & 0xffff0000) | (v & 0xffff);
-
- return 0;
-}
-
-static int apply_r_mips_64_rela(struct module *me, u32 *location, Elf_Addr v)
-{
- *(Elf_Addr *)location = v;
-
- return 0;
-}
-
-static int apply_r_mips_higher_rela(struct module *me, u32 *location,
- Elf_Addr v)
-{
- *location = (*location & 0xffff0000) |
- ((((long long) v + 0x80008000LL) >> 32) & 0xffff);
-
- return 0;
-}
-
-static int apply_r_mips_highest_rela(struct module *me, u32 *location,
- Elf_Addr v)
-{
- *location = (*location & 0xffff0000) |
- ((((long long) v + 0x800080008000LL) >> 48) & 0xffff);
-
- return 0;
-}
-
static int (*reloc_handlers_rel[]) (struct module *me, u32 *location,
Elf_Addr v) = {
[R_MIPS_NONE] = apply_r_mips_none,
@@ -247,18 +180,6 @@ static int (*reloc_handlers_rel[]) (struct module *me, u32 *location,
[R_MIPS_LO16] = apply_r_mips_lo16_rel
};

-static int (*reloc_handlers_rela[]) (struct module *me, u32 *location,
- Elf_Addr v) = {
- [R_MIPS_NONE] = apply_r_mips_none,
- [R_MIPS_32] = apply_r_mips_32_rela,
- [R_MIPS_26] = apply_r_mips_26_rela,
- [R_MIPS_HI16] = apply_r_mips_hi16_rela,
- [R_MIPS_LO16] = apply_r_mips_lo16_rela,
- [R_MIPS_64] = apply_r_mips_64_rela,
- [R_MIPS_HIGHER] = apply_r_mips_higher_rela,
- [R_MIPS_HIGHEST] = apply_r_mips_highest_rela
-};
-
int apply_relocate(Elf_Shdr *sechdrs, const char *strtab,
unsigned int symindex, unsigned int relsec,
struct module *me)
@@ -299,48 +220,6 @@ int apply_relocate(Elf_Shdr *sechdrs, const char *strtab,
return 0;
}

-#ifdef CONFIG_MODULES_USE_ELF_RELA
-int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
- unsigned int symindex, unsigned int relsec,
- struct module *me)
-{
- Elf_Mips_Rela *rel = (void *) sechdrs[relsec].sh_addr;
- Elf_Sym *sym;
- u32 *location;
- unsigned int i;
- Elf_Addr v;
- int res;
-
- pr_debug("Applying relocate section %u to %u\n", relsec,
- sechdrs[relsec].sh_info);
-
- for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) {
- /* This is where to make the change */
- location = (void *)sechdrs[sechdrs[relsec].sh_info].sh_addr
- + rel[i].r_offset;
- /* This is the symbol it is referring to */
- sym = (Elf_Sym *)sechdrs[symindex].sh_addr
- + ELF_MIPS_R_SYM(rel[i]);
- if (IS_ERR_VALUE(sym->st_value)) {
- /* Ignore unresolved weak symbol */
- if (ELF_ST_BIND(sym->st_info) == STB_WEAK)
- continue;
- printk(KERN_WARNING "%s: Unknown symbol %s\n",
- me->name, strtab + sym->st_name);
- return -ENOENT;
- }
-
- v = sym->st_value + rel[i].r_addend;
-
- res = reloc_handlers_rela[ELF_MIPS_R_TYPE(rel[i])](me, location, v);
- if (res)
- return res;
- }
-
- return 0;
-}
-#endif
-
/* Given an address, look for it in the module exception tables. */
const struct exception_table_entry *search_module_dbetables(unsigned long addr)
{

2012-08-14 15:47:55

by David Howells

[permalink] [raw]
Subject: Re: [PATCH] MIPS: Fix module.c build for 32 bit

Ralf Baechle <[email protected]> wrote:

> +extern int apply_r_mips_none(struct module *me, u32 *location, Elf_Addr v);

This needs to be in a header file, not a .c file.

David

2012-08-15 03:52:05

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] MIPS: fix module.c build for 32 bit

On Tue, 14 Aug 2012 15:08:10 +0100, David Howells <[email protected]> wrote:
> Rusty Russell <[email protected]> wrote:
>
> > Yep, thanks. And might as well sent them straight to Linus; since
> > linux-next didn't catch this, there's little point baking them there if
> > we have some acks.
> >
> > If he misses it, I'll grab them.
>
> It might have to wait for the next merge window.

For a build fix????

Confused,
Rusty.

2012-08-15 08:35:18

by David Howells

[permalink] [raw]
Subject: Re: [PATCH] MIPS: fix module.c build for 32 bit

Rusty Russell <[email protected]> wrote:

> For a build fix????

Linux hasn't pulled the asm-generic cleanup patch yet - you missed the merge
window, I think. Jonas detected the problem in linux-next.

David

2012-08-16 10:48:47

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] MIPS: fix module.c build for 32 bit

On Wed, 15 Aug 2012 09:34:58 +0100, David Howells <[email protected]> wrote:
> Rusty Russell <[email protected]> wrote:
>
> > For a build fix????
>
> Linux hasn't pulled the asm-generic cleanup patch yet - you missed the merge
> window, I think. Jonas detected the problem in linux-next.

Yes, I missed the window but asked Linus to pull anyway. Was delaying
for the other module patch, left it too long.

This cleanup won't benefit from more stewing in linux-next, since it sat
there for quite a while already. Feel free to add my Acked-by and push
it straight to Linus, otherwise I'll roll them together for next window.

Thanks,
Rusty.

2012-08-20 05:01:25

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] MIPS: Fix module.c build for 32 bit

On Tue, 14 Aug 2012 17:13:45 +0200, Ralf Baechle <[email protected]> wrote:
> Fixes build failure introduced by "Make most arch asm/module.h files use
> asm-generic/module.h" by moving all the RELA processing code to a
> separate file to be used only for RELA processing on 64-bit kernels.
>
> CC arch/mips/kernel/module.o
> arch/mips/kernel/module.c:250:14: error: 'reloc_handlers_rela' defined but not
> used [-Werror=unused-variable]
> cc1: all warnings being treated as errors
>
> make[6]: *** [arch/mips/kernel/module.o] Error 1
>
> Signed-off-by: Ralf Baechle <[email protected]>
>
> ---
> This is bigger than Jonas' suggested patch but far less #ifdefy.
> A minimal fix would be to add __maybe_unused to the definition of
> reloc_handlers_rela but imho __maybe_unused should be avoided where
> possible.
>
> I tested this on top of today's -next but ideally to keep bisectability
> it should be applied early in the series before CONFIG_MODULES_USE_ELF_RELA
> is introduced which would require trivial tweaking arch/mips/kernel/Makefile
> tweaking.

OK, as suggested, applied as a separate patch (with a bit of tweaking),
and then modified David's patch to flip the condition in the Makefile.

This will sit in linux-next another cycle.

Cheers,
Rusty.