This is the second take of the patch series.
Changes to previous version:
- new CONFIG_HAVE_MODULE_SECTION_STUBS config option
- put stub entries of a code section in front of the section
____________
The parisc port (esp. the 32bit kernel) currently lacks the ability to
load large kernel modules like xfs or ipv6. This is a long outstanding
bug and has already been reported a few times, e.g.:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=350482,
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=401439,
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=508489
The symptom is like this:
# modprobe xfs
FATAL: Error inserting xfs
(/lib/modules/2.6.26-1-parisc/kernel/fs/xfs/xfs.ko): Invalid module
format
In dmesg:
module xfs relocation of symbol xfs_btree_read_bufs is out of range
(0x3ffefffe in 17 bits)
The reason for the failure is, that the architecture only provides the
R_PARISC_PCREL17F (for 32bit kernels) and R_PARISC_PCREL22F (for PA2.0
and 64bit kernels) relocations, which sometimes can't reach the target
address of the stub entry if the kernel module is too large. Currently
parisc (like other architectures) creates one big PLT section for all
stubs at the beginning of the init and core sections.
The following two patches changes the parisc module loader to put stubs
for the code sections in front of each section, so that the distance to
the stubs more easily fits into the available 17/22 bits.
The first patch touches the generic module loader and adds a call to the
new module_additional_section_size() function to get_offset() if
CONFIG_HAVE_MODULE_SECTION_STUBS is defined. On parisc this
function returns the additional bytes for the stub area of a given section.
The second patch implements the parisc-specific changes.
Tested with 32- and 64bit parisc kernels.
Helge
[PATCH 1/2] module.c: fix module loading failure of large modules (take 2)
When creating the final layout of a kernel module in memory, allow the
module loader to reserve some additional memory in front of a given section.
This is currently only needed for the parisc port which needs to put the
stub entries there to fulfill the 17/22bit PCREL relocations with large
kernel modules like xfs.
Signed-off-by: Helge Deller <[email protected]>
diff --git a/include/linux/moduleloader.h b/include/linux/moduleloader.h
index eb10339..65fb34c 100644
--- a/include/linux/moduleloader.h
+++ b/include/linux/moduleloader.h
@@ -13,6 +13,9 @@ int module_frob_arch_sections(Elf_Ehdr *hdr,
char *secstrings,
struct module *mod);
+unsigned long module_additional_section_size(struct module *mod,
+ unsigned int section);
+
/* Allocator used for allocating struct module, core sections and init
sections. Returns NULL on failure. */
void *module_alloc(unsigned long size);
diff --git a/init/Kconfig b/init/Kconfig
index f763762..5383815 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -908,6 +908,9 @@ config MODULE_SRCVERSION_ALL
the version). With this option, such a "srcversion" field
will be created for all modules. If unsure, say N.
+config HAVE_MODULE_SECTION_STUBS
+ bool
+
config KMOD
def_bool y
help
diff --git a/kernel/module.c b/kernel/module.c
index 1f4cc00..4b86308 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1579,10 +1579,15 @@ static int simplify_symbols(Elf_Shdr *sechdrs,
}
/* Update size with this section: return offset. */
-static long get_offset(unsigned int *size, Elf_Shdr *sechdr)
+static long get_offset(struct module *mod, unsigned int *size,
+ Elf_Shdr *sechdr, unsigned int section)
{
long ret;
+#ifdef CONFIG_HAVE_MODULE_SECTION_STUBS
+ /* allocate some memory for stubs in front of each section */
+ *size += module_additional_section_size(mod, section);
+#endif
ret = ALIGN(*size, sechdr->sh_addralign ?: 1);
*size = ret + sechdr->sh_size;
return ret;
@@ -1622,7 +1627,7 @@ static void layout_sections(struct module *mod,
|| strncmp(secstrings + s->sh_name,
".init", 5) == 0)
continue;
- s->sh_entsize = get_offset(&mod->core_size, s);
+ s->sh_entsize = get_offset(mod, &mod->core_size, s, i);
DEBUGP("\t%s\n", secstrings + s->sh_name);
}
if (m == 0)
@@ -1640,7 +1645,7 @@ static void layout_sections(struct module *mod,
|| strncmp(secstrings + s->sh_name,
".init", 5) != 0)
continue;
- s->sh_entsize = (get_offset(&mod->init_size, s)
+ s->sh_entsize = (get_offset(mod, &mod->init_size, s, i)
| INIT_OFFSET_MASK);
DEBUGP("\t%s\n", secstrings + s->sh_name);
}
[PATCH 2/2] parisc: fix module loading failure of large modules (take 2)
On 32bit (and sometimes 64bit) and with big kernel modules like xfs or
ipv6 the relocation types R_PARISC_PCREL17F and R_PARISC_PCREL22F may
fail to reach their PLT stub if we only create one big stub array for
all sections at the beginning of the core or init section.
With this patch we now instead add individual PLT stub entries
directly in front of the code sections where the stubs are actually
called. This reduces the distance between the PCREL location and the
stub entry so that the relocations can be fulfilled.
The kernel module loader will call module_additional_section_size() and
request us to return the amount of additional memory we need for the
stubs of each section. This memory will then be allocated in front
of the code code segment when the kernel layouts the final addresses
of all sections.
Tested with 32- and 64bit kernels.
Signed-off-by: Helge Deller <[email protected]>
diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
index 644a70b..f975e31 100644
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -9,6 +9,7 @@ config PARISC
def_bool y
select HAVE_IDE
select HAVE_OPROFILE
+ select HAVE_MODULE_SECTION_STUBS
select RTC_CLASS
select RTC_DRV_PARISC
help
diff --git a/arch/parisc/include/asm/module.h b/arch/parisc/include/asm/module.h
index c2cb49e..1f41234 100644
--- a/arch/parisc/include/asm/module.h
+++ b/arch/parisc/include/asm/module.h
@@ -23,8 +23,10 @@ struct mod_arch_specific
{
unsigned long got_offset, got_count, got_max;
unsigned long fdesc_offset, fdesc_count, fdesc_max;
- unsigned long stub_offset, stub_count, stub_max;
- unsigned long init_stub_offset, init_stub_count, init_stub_max;
+ struct {
+ unsigned long stub_offset;
+ unsigned int stub_entries;
+ } *section;
int unwind_section;
struct unwind_table *unwind;
};
diff --git a/arch/parisc/kernel/module.c b/arch/parisc/kernel/module.c
index 44138c3..f8129ce 100644
--- a/arch/parisc/kernel/module.c
+++ b/arch/parisc/kernel/module.c
@@ -6,6 +6,7 @@
*
* Linux/PA-RISC Project (http://www.parisc-linux.org/)
* Copyright (C) 2003 Randolph Chung <tausq at debian . org>
+ * Copyright (C) 2008 Helge Deller <[email protected]>
*
*
* This program is free software; you can redistribute it and/or modify
@@ -24,6 +25,20 @@
*
*
* Notes:
+ * - PLT stub handling
+ * On 32bit (and sometimes 64bit) and with big kernel modules like xfs or
+ * ipv6 the relocation types R_PARISC_PCREL17F and R_PARISC_PCREL22F may
+ * fail to reach its PLT stub if we only create one big stub array for
+ * all sections at the beginning of the core or init section.
+ * Instead we now insert individual PLT stub entries directly in front of
+ * of the code sections where the stubs are actually called.
+ * This reduces the distance between the PCREL location and the stub entry
+ * so that the relocations can be fulfilled.
+ * The kernel module loader will call module_additional_section_size()
+ * and request us to return the amount of additional memory we need for
+ * the stubs of each section. This memory will then be allocated in front
+ * of the code code segment.
+ *
* - SEGREL32 handling
* We are not doing SEGREL32 handling correctly. According to the ABI, we
* should do a value offset, like this:
@@ -58,9 +73,13 @@
#define DEBUGP(fmt...)
#endif
+#define RELOC_REACHABLE(val, bits) \
+ (( ( !((val) & (1<<((bits)-1))) && ((val)>>(bits)) != 0 ) || \
+ ( ((val) & (1<<((bits)-1))) && ((val)>>(bits)) != (((__typeof__(val))(~0))>>((bits)+2)))) ? \
+ 0 : 1)
+
#define CHECK_RELOC(val, bits) \
- if ( ( !((val) & (1<<((bits)-1))) && ((val)>>(bits)) != 0 ) || \
- ( ((val) & (1<<((bits)-1))) && ((val)>>(bits)) != (((__typeof__(val))(~0))>>((bits)+2)))) { \
+ if (!RELOC_REACHABLE(val, bits)) { \
printk(KERN_ERR "module %s relocation of symbol %s is out of range (0x%lx in %d bits)\n", \
me->name, strtab + sym->st_name, (unsigned long)val, bits); \
return -ENOEXEC; \
@@ -92,13 +111,6 @@ static inline int in_local(struct module *me, void *loc)
return in_init(me, loc) || in_core(me, loc);
}
-static inline int in_local_section(struct module *me, void *loc, void *dot)
-{
- return (in_init(me, loc) && in_init(me, dot)) ||
- (in_core(me, loc) && in_core(me, dot));
-}
-
-
#ifndef CONFIG_64BIT
struct got_entry {
Elf32_Addr addr;
@@ -258,23 +270,42 @@ static inline unsigned long count_stubs(const Elf_Rela *rela, unsigned long n)
/* Free memory returned from module_alloc */
void module_free(struct module *mod, void *module_region)
{
+ kfree(mod->arch.section);
+ mod->arch.section = NULL;
+
vfree(module_region);
/* FIXME: If module_region == mod->init_region, trim exception
table entries. */
}
+/* return number of additional bytes to reserve for a section */
+unsigned long module_additional_section_size(struct module *mod,
+ unsigned int section)
+{
+ /* size needed for all stubs of this section (including
+ * one additional for correct alignment of the stubs) */
+ return (mod->arch.section[section].stub_entries + 1)
+ * sizeof(struct stub_entry);
+}
+
#define CONST
int module_frob_arch_sections(CONST Elf_Ehdr *hdr,
CONST Elf_Shdr *sechdrs,
CONST char *secstrings,
struct module *me)
{
- unsigned long gots = 0, fdescs = 0, stubs = 0, init_stubs = 0;
+ unsigned long gots = 0, fdescs = 0, len;
unsigned int i;
+ len = hdr->e_shnum * sizeof(me->arch.section[0]);
+ me->arch.section = kzalloc(len, GFP_KERNEL);
+ if (!me->arch.section)
+ return -ENOMEM;
+
for (i = 1; i < hdr->e_shnum; i++) {
- const Elf_Rela *rels = (void *)hdr + sechdrs[i].sh_offset;
+ const Elf_Rela *rels = (void *)sechdrs[i].sh_addr;
unsigned long nrels = sechdrs[i].sh_size / sizeof(*rels);
+ unsigned int count, s;
if (strncmp(secstrings + sechdrs[i].sh_name,
".PARISC.unwind", 14) == 0)
@@ -290,11 +321,23 @@ int module_frob_arch_sections(CONST Elf_Ehdr *hdr,
*/
gots += count_gots(rels, nrels);
fdescs += count_fdescs(rels, nrels);
- if(strncmp(secstrings + sechdrs[i].sh_name,
- ".rela.init", 10) == 0)
- init_stubs += count_stubs(rels, nrels);
- else
- stubs += count_stubs(rels, nrels);
+
+ /* XXX: By sorting the relocs and finding duplicate entries
+ * we could reduce the number of necessary stubs and save
+ * some memory. */
+ count = count_stubs(rels, nrels);
+ if (!count)
+ continue;
+
+ /* so we need relocation stubs. reserve necessary memory. */
+ /* sh_info gives the section for which we need to add stubs. */
+ s = sechdrs[i].sh_info;
+
+ /* each code section should only have one relocation section */
+ WARN_ON(me->arch.section[s].stub_entries);
+
+ /* store number of stubs we need for this section */
+ me->arch.section[s].stub_entries += count;
}
/* align things a bit */
@@ -306,18 +349,8 @@ int module_frob_arch_sections(CONST Elf_Ehdr *hdr,
me->arch.fdesc_offset = me->core_size;
me->core_size += fdescs * sizeof(Elf_Fdesc);
- me->core_size = ALIGN(me->core_size, 16);
- me->arch.stub_offset = me->core_size;
- me->core_size += stubs * sizeof(struct stub_entry);
-
- me->init_size = ALIGN(me->init_size, 16);
- me->arch.init_stub_offset = me->init_size;
- me->init_size += init_stubs * sizeof(struct stub_entry);
-
me->arch.got_max = gots;
me->arch.fdesc_max = fdescs;
- me->arch.stub_max = stubs;
- me->arch.init_stub_max = init_stubs;
return 0;
}
@@ -380,23 +413,27 @@ enum elf_stub_type {
};
static Elf_Addr get_stub(struct module *me, unsigned long value, long addend,
- enum elf_stub_type stub_type, int init_section)
+ enum elf_stub_type stub_type, Elf_Addr loc0, unsigned int targetsec)
{
- unsigned long i;
struct stub_entry *stub;
- if(init_section) {
- i = me->arch.init_stub_count++;
- BUG_ON(me->arch.init_stub_count > me->arch.init_stub_max);
- stub = me->module_init + me->arch.init_stub_offset +
- i * sizeof(struct stub_entry);
- } else {
- i = me->arch.stub_count++;
- BUG_ON(me->arch.stub_count > me->arch.stub_max);
- stub = me->module_core + me->arch.stub_offset +
- i * sizeof(struct stub_entry);
+ /* initialize stub_offset to point in front of the section */
+ if (!me->arch.section[targetsec].stub_offset) {
+ loc0 -= (me->arch.section[targetsec].stub_entries + 1) *
+ sizeof(struct stub_entry);
+ /* get correct alignment for the stubs */
+ loc0 = ALIGN(loc0, sizeof(struct stub_entry));
+ me->arch.section[targetsec].stub_offset = loc0;
}
+ /* get address of stub entry */
+ stub = (void *) me->arch.section[targetsec].stub_offset;
+ me->arch.section[targetsec].stub_offset += sizeof(struct stub_entry);
+
+ /* do not write outside available stub area */
+ BUG_ON(0 == me->arch.section[targetsec].stub_entries--);
+
+
#ifndef CONFIG_64BIT
/* for 32-bit the stub looks like this:
* ldil L'XXX,%r1
@@ -489,15 +526,19 @@ int apply_relocate_add(Elf_Shdr *sechdrs,
Elf32_Addr val;
Elf32_Sword addend;
Elf32_Addr dot;
+ Elf_Addr loc0;
+ unsigned int targetsec = sechdrs[relsec].sh_info;
//unsigned long dp = (unsigned long)$global$;
register unsigned long dp asm ("r27");
DEBUGP("Applying relocate section %u to %u\n", relsec,
- sechdrs[relsec].sh_info);
+ targetsec);
for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) {
/* This is where to make the change */
- loc = (void *)sechdrs[sechdrs[relsec].sh_info].sh_addr
+ loc = (void *)sechdrs[targetsec].sh_addr
+ rel[i].r_offset;
+ /* This is the start of the target section */
+ loc0 = sechdrs[targetsec].sh_addr;
/* This is the symbol it is referring to */
sym = (Elf32_Sym *)sechdrs[symindex].sh_addr
+ ELF32_R_SYM(rel[i].r_info);
@@ -569,19 +610,32 @@ int apply_relocate_add(Elf_Shdr *sechdrs,
break;
case R_PARISC_PCREL17F:
/* 17-bit PC relative address */
- val = get_stub(me, val, addend, ELF_STUB_GOT, in_init(me, loc));
+ /* calculate direct call offset */
+ val += addend;
val = (val - dot - 8)/4;
- CHECK_RELOC(val, 17)
+ if (!RELOC_REACHABLE(val, 17)) {
+ /* direct distance too far, create
+ * stub entry instead */
+ val = get_stub(me, sym->st_value, addend,
+ ELF_STUB_DIRECT, loc0, targetsec);
+ val = (val - dot - 8)/4;
+ CHECK_RELOC(val, 17);
+ }
*loc = (*loc & ~0x1f1ffd) | reassemble_17(val);
break;
case R_PARISC_PCREL22F:
/* 22-bit PC relative address; only defined for pa20 */
- val = get_stub(me, val, addend, ELF_STUB_GOT, in_init(me, loc));
- DEBUGP("STUB FOR %s loc %lx+%lx at %lx\n",
- strtab + sym->st_name, (unsigned long)loc, addend,
- val)
+ /* calculate direct call offset */
+ val += addend;
val = (val - dot - 8)/4;
- CHECK_RELOC(val, 22);
+ if (!RELOC_REACHABLE(val, 22)) {
+ /* direct distance too far, create
+ * stub entry instead */
+ val = get_stub(me, sym->st_value, addend,
+ ELF_STUB_DIRECT, loc0, targetsec);
+ val = (val - dot - 8)/4;
+ CHECK_RELOC(val, 22);
+ }
*loc = (*loc & ~0x3ff1ffd) | reassemble_22(val);
break;
@@ -610,13 +664,17 @@ int apply_relocate_add(Elf_Shdr *sechdrs,
Elf64_Addr val;
Elf64_Sxword addend;
Elf64_Addr dot;
+ Elf_Addr loc0;
+ unsigned int targetsec = sechdrs[relsec].sh_info;
DEBUGP("Applying relocate section %u to %u\n", relsec,
- sechdrs[relsec].sh_info);
+ targetsec);
for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) {
/* This is where to make the change */
- loc = (void *)sechdrs[sechdrs[relsec].sh_info].sh_addr
+ loc = (void *)sechdrs[targetsec].sh_addr
+ rel[i].r_offset;
+ /* This is the start of the target section */
+ loc0 = sechdrs[targetsec].sh_addr;
/* This is the symbol it is referring to */
sym = (Elf64_Sym *)sechdrs[symindex].sh_addr
+ ELF64_R_SYM(rel[i].r_info);
@@ -672,42 +730,40 @@ int apply_relocate_add(Elf_Shdr *sechdrs,
DEBUGP("PCREL22F Symbol %s loc %p val %lx\n",
strtab + sym->st_name,
loc, val);
+ val += addend;
/* can we reach it locally? */
- if(!in_local_section(me, (void *)val, (void *)dot)) {
-
- if (in_local(me, (void *)val))
- /* this is the case where the
- * symbol is local to the
- * module, but in a different
- * section, so stub the jump
- * in case it's more than 22
- * bits away */
- val = get_stub(me, val, addend, ELF_STUB_DIRECT,
- in_init(me, loc));
- else if (strncmp(strtab + sym->st_name, "$$", 2)
+ if (in_local(me, (void *)val)) {
+ /* this is the case where the symbol is local
+ * to the module, but in a different section,
+ * so stub the jump in case it's more than 22
+ * bits away */
+ val = (val - dot - 8)/4;
+ if (!RELOC_REACHABLE(val, 22)) {
+ /* direct distance too far, create
+ * stub entry instead */
+ val = get_stub(me, sym->st_value,
+ addend, ELF_STUB_DIRECT,
+ loc0, targetsec);
+ } else {
+ /* Ok, we can reach it directly. */
+ val = sym->st_value;
+ val += addend;
+ }
+ } else {
+ val = sym->st_value;
+ if (strncmp(strtab + sym->st_name, "$$", 2)
== 0)
val = get_stub(me, val, addend, ELF_STUB_MILLI,
- in_init(me, loc));
+ loc0, targetsec);
else
val = get_stub(me, val, addend, ELF_STUB_GOT,
- in_init(me, loc));
+ loc0, targetsec);
}
DEBUGP("STUB FOR %s loc %lx, val %lx+%lx at %lx\n",
strtab + sym->st_name, loc, sym->st_value,
addend, val);
- /* FIXME: local symbols work as long as the
- * core and init pieces aren't separated too
- * far. If this is ever broken, you will trip
- * the check below. The way to fix it would
- * be to generate local stubs to go between init
- * and core */
- if((Elf64_Sxword)(val - dot - 8) > 0x800000 -1 ||
- (Elf64_Sxword)(val - dot - 8) < -0x800000) {
- printk(KERN_ERR "Module %s, symbol %s is out of range for PCREL22F relocation\n",
- me->name, strtab + sym->st_name);
- return -ENOEXEC;
- }
val = (val - dot - 8)/4;
+ CHECK_RELOC(val, 22);
*loc = (*loc & ~0x3ff1ffd) | reassemble_22(val);
break;
case R_PARISC_DIR64:
@@ -794,12 +850,8 @@ int module_finalize(const Elf_Ehdr *hdr,
addr = (u32 *)entry->addr;
printk("INSNS: %x %x %x %x\n",
addr[0], addr[1], addr[2], addr[3]);
- printk("stubs used %ld, stubs max %ld\n"
- "init_stubs used %ld, init stubs max %ld\n"
- "got entries used %ld, gots max %ld\n"
+ printk("got entries used %ld, gots max %ld\n"
"fdescs used %ld, fdescs max %ld\n",
- me->arch.stub_count, me->arch.stub_max,
- me->arch.init_stub_count, me->arch.init_stub_max,
me->arch.got_count, me->arch.got_max,
me->arch.fdesc_count, me->arch.fdesc_max);
#endif
@@ -829,7 +881,10 @@ int module_finalize(const Elf_Ehdr *hdr,
me->name, me->arch.got_count, MAX_GOTS);
return -EINVAL;
}
-
+
+ kfree(me->arch.section);
+ me->arch.section = NULL;
+
/* no symbol table */
if(symhdr == NULL)
return 0;
Ugh, I fail at typing, resending for rusty's benefit.
On Tue, Dec 30, 2008 at 01:07:24PM -0500, Kyle McMartin wrote:
> [Adding rusty to CC]
>
> On Mon, Dec 29, 2008 at 09:43:12PM +0100, Helge Deller wrote:
> > +unsigned long module_additional_section_size(struct module *mod,
> > + unsigned int section);
> > +
>
> I think this would be more palatable as
>
> #ifdef HAVE_MODULE_SECTION_STUBS
> unsigned long module_additional_section_size(struct module *mod,
> unsigned int section);
> #else
> static inline unsigned long module_additional_section_size(struct module *mod,
> unsigned int section)
> {
> return 0;
> }
> #endif
>
> and removing the conditional in kernel/module.c, possibly the symbol
> should be "arch_module_a..." just to make it clear to anyone reading.
>
> Anyway, it's up to rusty.
>
> Rusty, we'd like to get this patch in, so I can merge the dependent
> parisc-specific patch.
>
> regards, Kyle
>
> > /* Allocator used for allocating struct module, core sections and init
> > sections. Returns NULL on failure. */
> > void *module_alloc(unsigned long size);
> > diff --git a/init/Kconfig b/init/Kconfig
> > index f763762..5383815 100644
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -908,6 +908,9 @@ config MODULE_SRCVERSION_ALL
> > the version). With this option, such a "srcversion" field
> > will be created for all modules. If unsure, say N.
> >
> > +config HAVE_MODULE_SECTION_STUBS
> > + bool
> > +
> > config KMOD
> > def_bool y
> > help
> > diff --git a/kernel/module.c b/kernel/module.c
> > index 1f4cc00..4b86308 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -1579,10 +1579,15 @@ static int simplify_symbols(Elf_Shdr *sechdrs,
> > }
> >
> > /* Update size with this section: return offset. */
> > -static long get_offset(unsigned int *size, Elf_Shdr *sechdr)
> > +static long get_offset(struct module *mod, unsigned int *size,
> > + Elf_Shdr *sechdr, unsigned int section)
> > {
> > long ret;
> >
> > +#ifdef CONFIG_HAVE_MODULE_SECTION_STUBS
> > + /* allocate some memory for stubs in front of each section */
> > + *size += module_additional_section_size(mod, section);
> > +#endif
> > ret = ALIGN(*size, sechdr->sh_addralign ?: 1);
> > *size = ret + sechdr->sh_size;
> > return ret;
> > @@ -1622,7 +1627,7 @@ static void layout_sections(struct module *mod,
> > || strncmp(secstrings + s->sh_name,
> > ".init", 5) == 0)
> > continue;
> > - s->sh_entsize = get_offset(&mod->core_size, s);
> > + s->sh_entsize = get_offset(mod, &mod->core_size, s, i);
> > DEBUGP("\t%s\n", secstrings + s->sh_name);
> > }
> > if (m == 0)
> > @@ -1640,7 +1645,7 @@ static void layout_sections(struct module *mod,
> > || strncmp(secstrings + s->sh_name,
> > ".init", 5) != 0)
> > continue;
> > - s->sh_entsize = (get_offset(&mod->init_size, s)
> > + s->sh_entsize = (get_offset(mod, &mod->init_size, s, i)
> > | INIT_OFFSET_MASK);
> > DEBUGP("\t%s\n", secstrings + s->sh_name);
> > }
> >
>
Kyle McMartin wrote:
> Ugh, I fail at typing, resending for rusty's benefit.
>
> On Tue, Dec 30, 2008 at 01:07:24PM -0500, Kyle McMartin wrote:
>> [Adding rusty to CC]
>>
>> On Mon, Dec 29, 2008 at 09:43:12PM +0100, Helge Deller wrote:
>>> +unsigned long module_additional_section_size(struct module *mod,
>>> + unsigned int section);
>>> +
>> I think this would be more palatable as
>>
>> #ifdef HAVE_MODULE_SECTION_STUBS
>> unsigned long module_additional_section_size(struct module *mod,
>> unsigned int section);
>> #else
>> static inline unsigned long module_additional_section_size(struct module *mod,
>> unsigned int section)
>> {
>> return 0;
>> }
>> #endif
>>
>> and removing the conditional in kernel/module.c, possibly the symbol
>> should be "arch_module_a..." just to make it clear to anyone reading.
>>
>> Anyway, it's up to rusty.
>>
>> Rusty, we'd like to get this patch in, so I can merge the dependent
>> parisc-specific patch.
Rusty, I think I've a cleaner patch for that and will send it soon...
Helge
Helge Deller wrote:
> Kyle McMartin wrote:
>>> Anyway, it's up to rusty.
>>>
>>> Rusty, we'd like to get this patch in, so I can merge the dependent
>>> parisc-specific patch.
Rusty,
While my initial patch (http://lkml.org/lkml/2008/12/29/274) added a
new CONFIG option and an optional callback function, this version instead
reuses the ElfHdr.sh_entsize field to tell the layout function how many
bytes to reserve.
It seems that only the IA64 module loader is affected by that change,
so the trivial patch for IA64 is below as well.
Other architectures use the sh_entsize field as well, but only for
bootloaders (x86, mips) or for special purpose loaders (powerpc cell),
but not for kernel modules. So the patch below should be pretty safe.
Personally I prefer this patch, but if you think my initial patch is
better, I'd be happy as well.
arch/ia64/kernel/module.c | 4 +++-
kernel/module.c | 37 ++++++++++++++++++++-----------------
2 files changed, 23 insertions(+), 18 deletions(-)
[PATCH 1/2] module.c: fix module loading failure of large modules (take 3)
When creating the final layout of a kernel module in memory, allow the
module loader to reserve some additional memory in front of a given section.
The amount of to-be allocated memory is returned by the architecture's
module_frob_arch_sections() function in the sechdrs.sh_entsize field.
By default this field is initialized by the module loader to zero.
Additional memory in front of code sections is currently only needed
for the parisc port which needs to put the stub entries there to fulfill
the 17/22bit PCREL relocations with large kernel modules like xfs.
Signed-off-by: Helge Deller <[email protected]>
diff --git a/arch/ia64/kernel/module.c b/arch/ia64/kernel/module.c
index aaa7d90..5bd35f1 100644
--- a/arch/ia64/kernel/module.c
+++ b/arch/ia64/kernel/module.c
@@ -53,6 +53,8 @@
#define MAX_LTOFF ((uint64_t) (1 << 22)) /* max. allowable linkage-table offset */
+#define INIT_LAYOUT_MASK (1UL << (BITS_PER_LONG-1))
+
/* Define some relocation helper macros/types: */
#define FORMAT_SHIFT 0
@@ -804,7 +806,7 @@ apply_relocate_add (Elf64_Shdr *sechdrs, const char *strtab, unsigned int symind
target_sec = sechdrs + sechdrs[relsec].sh_info;
- if (target_sec->sh_entsize == ~0UL)
+ if (!(target_sec->sh_entsize & INIT_LAYOUT_MASK))
/*
* If target section wasn't allocated, we don't need to relocate it.
* Happens, e.g., for debug sections.
diff --git a/kernel/module.c b/kernel/module.c
index 1f4cc00..a09174d 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -62,8 +62,9 @@
#define ARCH_SHF_SMALL 0
#endif
-/* If this is set, the section belongs in the init part of the module */
-#define INIT_OFFSET_MASK (1UL << (BITS_PER_LONG-1))
+#define INIT_LAYOUT_MASK (1UL << (BITS_PER_LONG-1)) /* section-layout done */
+#define INIT_OFFSET_MASK (1UL << (BITS_PER_LONG-2)) /* part of init section */
+#define INIT_STRIP_MASK (INIT_OFFSET_MASK | INIT_LAYOUT_MASK)
/* List of modules, protected by module_mutex or preempt_disable
* (delete uses stop_machine/add uses RCU list operations). */
@@ -1583,6 +1584,8 @@ static long get_offset(unsigned int *size, Elf_Shdr *sechdr)
{
long ret;
+ /* architectures can request sh_entsize bytes in front of a section */
+ *size += (sechdr->sh_entsize & ~INIT_STRIP_MASK);
ret = ALIGN(*size, sechdr->sh_addralign ?: 1);
*size = ret + sechdr->sh_size;
return ret;
@@ -1590,8 +1593,8 @@ static long get_offset(unsigned int *size, Elf_Shdr *sechdr)
/* Lay out the SHF_ALLOC sections in a way not dissimilar to how ld
might -- code, read-only data, read-write data, small data. Tally
- sizes, and place the offsets into sh_entsize fields: high bit means it
- belongs in init. */
+ sizes, and place the offsets into sh_entsize fields: INIT_OFFSET_MASK
+ indicates that it belongs in init. */
static void layout_sections(struct module *mod,
const Elf_Ehdr *hdr,
Elf_Shdr *sechdrs,
@@ -1608,9 +1611,6 @@ static void layout_sections(struct module *mod,
};
unsigned int m, i;
- for (i = 0; i < hdr->e_shnum; i++)
- sechdrs[i].sh_entsize = ~0UL;
-
DEBUGP("Core section allocation order:\n");
for (m = 0; m < ARRAY_SIZE(masks); ++m) {
for (i = 0; i < hdr->e_shnum; ++i) {
@@ -1618,11 +1618,12 @@ static void layout_sections(struct module *mod,
if ((s->sh_flags & masks[m][0]) != masks[m][0]
|| (s->sh_flags & masks[m][1])
- || s->sh_entsize != ~0UL
+ || (s->sh_entsize & INIT_LAYOUT_MASK)
|| strncmp(secstrings + s->sh_name,
".init", 5) == 0)
continue;
- s->sh_entsize = get_offset(&mod->core_size, s);
+ s->sh_entsize = get_offset(&mod->core_size, s)
+ | INIT_LAYOUT_MASK;
DEBUGP("\t%s\n", secstrings + s->sh_name);
}
if (m == 0)
@@ -1636,12 +1637,12 @@ static void layout_sections(struct module *mod,
if ((s->sh_flags & masks[m][0]) != masks[m][0]
|| (s->sh_flags & masks[m][1])
- || s->sh_entsize != ~0UL
+ || (s->sh_entsize & INIT_LAYOUT_MASK)
|| strncmp(secstrings + s->sh_name,
".init", 5) != 0)
continue;
s->sh_entsize = (get_offset(&mod->init_size, s)
- | INIT_OFFSET_MASK);
+ | INIT_OFFSET_MASK | INIT_LAYOUT_MASK);
DEBUGP("\t%s\n", secstrings + s->sh_name);
}
if (m == 0)
@@ -1986,7 +1987,11 @@ static noinline struct module *load_module(void __user *umod,
mod->state = MODULE_STATE_COMING;
- /* Allow arches to frob section contents and sizes. */
+ /* Allow arches to frob section contents, sh_entsize will tell the
+ * the section layouter how much space to allocate in front of each
+ * section */
+ for (i = 0; i < hdr->e_shnum; i++)
+ sechdrs[i].sh_entsize = 0UL;
err = module_frob_arch_sections(hdr, sechdrs, secstrings, mod);
if (err < 0)
goto free_mod;
@@ -2034,11 +2039,9 @@ static noinline struct module *load_module(void __user *umod,
if (!(sechdrs[i].sh_flags & SHF_ALLOC))
continue;
- if (sechdrs[i].sh_entsize & INIT_OFFSET_MASK)
- dest = mod->module_init
- + (sechdrs[i].sh_entsize & ~INIT_OFFSET_MASK);
- else
- dest = mod->module_core + sechdrs[i].sh_entsize;
+ dest = ((sechdrs[i].sh_entsize & INIT_OFFSET_MASK) ?
+ mod->module_init : mod->module_core)
+ + (sechdrs[i].sh_entsize & ~INIT_STRIP_MASK);
if (sechdrs[i].sh_type != SHT_NOBITS)
memcpy(dest, (void *)sechdrs[i].sh_addr,
[PATCH 2/2] parisc: fix module loading failure of large modules (take 3)
On 32bit (and sometimes 64bit) and with big kernel modules like xfs or
ipv6 the relocation types R_PARISC_PCREL17F and R_PARISC_PCREL22F may
fail to reach their PLT stub if we only create one big stub array for
all sections at the beginning of the core or init section.
With this patch we now instead add individual PLT stub entries
directly in front of the code sections where the stubs are actually
called. This reduces the distance between the PCREL location and the
stub entry so that the relocations can be fulfilled.
The kernel module loader will allocate as much memory in front
of the code code segment as we return in the sechdrs.sh_entsize field.
Tested with 32- and 64bit kernels.
Signed-off-by: Helge Deller <[email protected]>
diff --git a/arch/parisc/include/asm/module.h b/arch/parisc/include/asm/module.h
index c2cb49e..1f41234 100644
--- a/arch/parisc/include/asm/module.h
+++ b/arch/parisc/include/asm/module.h
@@ -23,8 +23,10 @@ struct mod_arch_specific
{
unsigned long got_offset, got_count, got_max;
unsigned long fdesc_offset, fdesc_count, fdesc_max;
- unsigned long stub_offset, stub_count, stub_max;
- unsigned long init_stub_offset, init_stub_count, init_stub_max;
+ struct {
+ unsigned long stub_offset;
+ unsigned int stub_entries;
+ } *section;
int unwind_section;
struct unwind_table *unwind;
};
diff --git a/arch/parisc/kernel/module.c b/arch/parisc/kernel/module.c
index 44138c3..aaa0879 100644
--- a/arch/parisc/kernel/module.c
+++ b/arch/parisc/kernel/module.c
@@ -6,6 +6,7 @@
*
* Linux/PA-RISC Project (http://www.parisc-linux.org/)
* Copyright (C) 2003 Randolph Chung <tausq at debian . org>
+ * Copyright (C) 2008 Helge Deller <[email protected]>
*
*
* This program is free software; you can redistribute it and/or modify
@@ -24,6 +25,20 @@
*
*
* Notes:
+ * - PLT stub handling
+ * On 32bit (and sometimes 64bit) and with big kernel modules like xfs or
+ * ipv6 the relocation types R_PARISC_PCREL17F and R_PARISC_PCREL22F may
+ * fail to reach its PLT stub if we only create one big stub array for
+ * all sections at the beginning of the core or init section.
+ * Instead we now insert individual PLT stub entries directly in front of
+ * of the code sections where the stubs are actually called.
+ * This reduces the distance between the PCREL location and the stub entry
+ * so that the relocations can be fulfilled.
+ * In module_frob_arch_sections() we calculate how many bytes we need for
+ * the stubs of each section and return this in the sh_entsize field. The
+ * kernel module layouter will then later reserve this amount of bytes
+ * in front of the code sections for us.
+ *
* - SEGREL32 handling
* We are not doing SEGREL32 handling correctly. According to the ABI, we
* should do a value offset, like this:
@@ -58,9 +73,13 @@
#define DEBUGP(fmt...)
#endif
+#define RELOC_REACHABLE(val, bits) \
+ (( ( !((val) & (1<<((bits)-1))) && ((val)>>(bits)) != 0 ) || \
+ ( ((val) & (1<<((bits)-1))) && ((val)>>(bits)) != (((__typeof__(val))(~0))>>((bits)+2)))) ? \
+ 0 : 1)
+
#define CHECK_RELOC(val, bits) \
- if ( ( !((val) & (1<<((bits)-1))) && ((val)>>(bits)) != 0 ) || \
- ( ((val) & (1<<((bits)-1))) && ((val)>>(bits)) != (((__typeof__(val))(~0))>>((bits)+2)))) { \
+ if (!RELOC_REACHABLE(val, bits)) { \
printk(KERN_ERR "module %s relocation of symbol %s is out of range (0x%lx in %d bits)\n", \
me->name, strtab + sym->st_name, (unsigned long)val, bits); \
return -ENOEXEC; \
@@ -92,13 +111,6 @@ static inline int in_local(struct module *me, void *loc)
return in_init(me, loc) || in_core(me, loc);
}
-static inline int in_local_section(struct module *me, void *loc, void *dot)
-{
- return (in_init(me, loc) && in_init(me, dot)) ||
- (in_core(me, loc) && in_core(me, dot));
-}
-
-
#ifndef CONFIG_64BIT
struct got_entry {
Elf32_Addr addr;
@@ -258,6 +270,9 @@ static inline unsigned long count_stubs(const Elf_Rela *rela, unsigned long n)
/* Free memory returned from module_alloc */
void module_free(struct module *mod, void *module_region)
{
+ kfree(mod->arch.section);
+ mod->arch.section = NULL;
+
vfree(module_region);
/* FIXME: If module_region == mod->init_region, trim exception
table entries. */
@@ -269,12 +284,18 @@ int module_frob_arch_sections(CONST Elf_Ehdr *hdr,
CONST char *secstrings,
struct module *me)
{
- unsigned long gots = 0, fdescs = 0, stubs = 0, init_stubs = 0;
+ unsigned long gots = 0, fdescs = 0, len;
unsigned int i;
+ len = hdr->e_shnum * sizeof(me->arch.section[0]);
+ me->arch.section = kzalloc(len, GFP_KERNEL);
+ if (!me->arch.section)
+ return -ENOMEM;
+
for (i = 1; i < hdr->e_shnum; i++) {
- const Elf_Rela *rels = (void *)hdr + sechdrs[i].sh_offset;
+ const Elf_Rela *rels = (void *)sechdrs[i].sh_addr;
unsigned long nrels = sechdrs[i].sh_size / sizeof(*rels);
+ unsigned int count, s;
if (strncmp(secstrings + sechdrs[i].sh_name,
".PARISC.unwind", 14) == 0)
@@ -290,11 +311,29 @@ int module_frob_arch_sections(CONST Elf_Ehdr *hdr,
*/
gots += count_gots(rels, nrels);
fdescs += count_fdescs(rels, nrels);
- if(strncmp(secstrings + sechdrs[i].sh_name,
- ".rela.init", 10) == 0)
- init_stubs += count_stubs(rels, nrels);
- else
- stubs += count_stubs(rels, nrels);
+
+ /* XXX: By sorting the relocs and finding duplicate entries
+ * we could reduce the number of necessary stubs and save
+ * some memory. */
+ count = count_stubs(rels, nrels);
+ if (!count)
+ continue;
+
+ /* so we need relocation stubs. reserve necessary memory. */
+ /* sh_info gives the section for which we need to add stubs. */
+ s = sechdrs[i].sh_info;
+
+ /* each code section should only have one relocation section */
+ WARN_ON(me->arch.section[s].stub_entries);
+
+ /* store number of stubs we need for this section */
+ me->arch.section[s].stub_entries += count;
+
+ /* tell section layouter how much space we need for all stubs
+ * of this section (including one additional for correct
+ * alignment of the stubs) */
+ sechdrs[s].sh_entsize = sizeof(struct stub_entry) *
+ (me->arch.section[s].stub_entries + 1);
}
/* align things a bit */
@@ -306,18 +345,8 @@ int module_frob_arch_sections(CONST Elf_Ehdr *hdr,
me->arch.fdesc_offset = me->core_size;
me->core_size += fdescs * sizeof(Elf_Fdesc);
- me->core_size = ALIGN(me->core_size, 16);
- me->arch.stub_offset = me->core_size;
- me->core_size += stubs * sizeof(struct stub_entry);
-
- me->init_size = ALIGN(me->init_size, 16);
- me->arch.init_stub_offset = me->init_size;
- me->init_size += init_stubs * sizeof(struct stub_entry);
-
me->arch.got_max = gots;
me->arch.fdesc_max = fdescs;
- me->arch.stub_max = stubs;
- me->arch.init_stub_max = init_stubs;
return 0;
}
@@ -380,23 +409,27 @@ enum elf_stub_type {
};
static Elf_Addr get_stub(struct module *me, unsigned long value, long addend,
- enum elf_stub_type stub_type, int init_section)
+ enum elf_stub_type stub_type, Elf_Addr loc0, unsigned int targetsec)
{
- unsigned long i;
struct stub_entry *stub;
- if(init_section) {
- i = me->arch.init_stub_count++;
- BUG_ON(me->arch.init_stub_count > me->arch.init_stub_max);
- stub = me->module_init + me->arch.init_stub_offset +
- i * sizeof(struct stub_entry);
- } else {
- i = me->arch.stub_count++;
- BUG_ON(me->arch.stub_count > me->arch.stub_max);
- stub = me->module_core + me->arch.stub_offset +
- i * sizeof(struct stub_entry);
+ /* initialize stub_offset to point in front of the section */
+ if (!me->arch.section[targetsec].stub_offset) {
+ loc0 -= (me->arch.section[targetsec].stub_entries + 1) *
+ sizeof(struct stub_entry);
+ /* get correct alignment for the stubs */
+ loc0 = ALIGN(loc0, sizeof(struct stub_entry));
+ me->arch.section[targetsec].stub_offset = loc0;
}
+ /* get address of stub entry */
+ stub = (void *) me->arch.section[targetsec].stub_offset;
+ me->arch.section[targetsec].stub_offset += sizeof(struct stub_entry);
+
+ /* do not write outside available stub area */
+ BUG_ON(0 == me->arch.section[targetsec].stub_entries--);
+
+
#ifndef CONFIG_64BIT
/* for 32-bit the stub looks like this:
* ldil L'XXX,%r1
@@ -489,15 +522,19 @@ int apply_relocate_add(Elf_Shdr *sechdrs,
Elf32_Addr val;
Elf32_Sword addend;
Elf32_Addr dot;
+ Elf_Addr loc0;
+ unsigned int targetsec = sechdrs[relsec].sh_info;
//unsigned long dp = (unsigned long)$global$;
register unsigned long dp asm ("r27");
DEBUGP("Applying relocate section %u to %u\n", relsec,
- sechdrs[relsec].sh_info);
+ targetsec);
for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) {
/* This is where to make the change */
- loc = (void *)sechdrs[sechdrs[relsec].sh_info].sh_addr
+ loc = (void *)sechdrs[targetsec].sh_addr
+ rel[i].r_offset;
+ /* This is the start of the target section */
+ loc0 = sechdrs[targetsec].sh_addr;
/* This is the symbol it is referring to */
sym = (Elf32_Sym *)sechdrs[symindex].sh_addr
+ ELF32_R_SYM(rel[i].r_info);
@@ -569,19 +606,32 @@ int apply_relocate_add(Elf_Shdr *sechdrs,
break;
case R_PARISC_PCREL17F:
/* 17-bit PC relative address */
- val = get_stub(me, val, addend, ELF_STUB_GOT, in_init(me, loc));
+ /* calculate direct call offset */
+ val += addend;
val = (val - dot - 8)/4;
- CHECK_RELOC(val, 17)
+ if (!RELOC_REACHABLE(val, 17)) {
+ /* direct distance too far, create
+ * stub entry instead */
+ val = get_stub(me, sym->st_value, addend,
+ ELF_STUB_DIRECT, loc0, targetsec);
+ val = (val - dot - 8)/4;
+ CHECK_RELOC(val, 17);
+ }
*loc = (*loc & ~0x1f1ffd) | reassemble_17(val);
break;
case R_PARISC_PCREL22F:
/* 22-bit PC relative address; only defined for pa20 */
- val = get_stub(me, val, addend, ELF_STUB_GOT, in_init(me, loc));
- DEBUGP("STUB FOR %s loc %lx+%lx at %lx\n",
- strtab + sym->st_name, (unsigned long)loc, addend,
- val)
+ /* calculate direct call offset */
+ val += addend;
val = (val - dot - 8)/4;
- CHECK_RELOC(val, 22);
+ if (!RELOC_REACHABLE(val, 22)) {
+ /* direct distance too far, create
+ * stub entry instead */
+ val = get_stub(me, sym->st_value, addend,
+ ELF_STUB_DIRECT, loc0, targetsec);
+ val = (val - dot - 8)/4;
+ CHECK_RELOC(val, 22);
+ }
*loc = (*loc & ~0x3ff1ffd) | reassemble_22(val);
break;
@@ -610,13 +660,17 @@ int apply_relocate_add(Elf_Shdr *sechdrs,
Elf64_Addr val;
Elf64_Sxword addend;
Elf64_Addr dot;
+ Elf_Addr loc0;
+ unsigned int targetsec = sechdrs[relsec].sh_info;
DEBUGP("Applying relocate section %u to %u\n", relsec,
- sechdrs[relsec].sh_info);
+ targetsec);
for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) {
/* This is where to make the change */
- loc = (void *)sechdrs[sechdrs[relsec].sh_info].sh_addr
+ loc = (void *)sechdrs[targetsec].sh_addr
+ rel[i].r_offset;
+ /* This is the start of the target section */
+ loc0 = sechdrs[targetsec].sh_addr;
/* This is the symbol it is referring to */
sym = (Elf64_Sym *)sechdrs[symindex].sh_addr
+ ELF64_R_SYM(rel[i].r_info);
@@ -672,42 +726,40 @@ int apply_relocate_add(Elf_Shdr *sechdrs,
DEBUGP("PCREL22F Symbol %s loc %p val %lx\n",
strtab + sym->st_name,
loc, val);
+ val += addend;
/* can we reach it locally? */
- if(!in_local_section(me, (void *)val, (void *)dot)) {
-
- if (in_local(me, (void *)val))
- /* this is the case where the
- * symbol is local to the
- * module, but in a different
- * section, so stub the jump
- * in case it's more than 22
- * bits away */
- val = get_stub(me, val, addend, ELF_STUB_DIRECT,
- in_init(me, loc));
- else if (strncmp(strtab + sym->st_name, "$$", 2)
+ if (in_local(me, (void *)val)) {
+ /* this is the case where the symbol is local
+ * to the module, but in a different section,
+ * so stub the jump in case it's more than 22
+ * bits away */
+ val = (val - dot - 8)/4;
+ if (!RELOC_REACHABLE(val, 22)) {
+ /* direct distance too far, create
+ * stub entry instead */
+ val = get_stub(me, sym->st_value,
+ addend, ELF_STUB_DIRECT,
+ loc0, targetsec);
+ } else {
+ /* Ok, we can reach it directly. */
+ val = sym->st_value;
+ val += addend;
+ }
+ } else {
+ val = sym->st_value;
+ if (strncmp(strtab + sym->st_name, "$$", 2)
== 0)
val = get_stub(me, val, addend, ELF_STUB_MILLI,
- in_init(me, loc));
+ loc0, targetsec);
else
val = get_stub(me, val, addend, ELF_STUB_GOT,
- in_init(me, loc));
+ loc0, targetsec);
}
DEBUGP("STUB FOR %s loc %lx, val %lx+%lx at %lx\n",
strtab + sym->st_name, loc, sym->st_value,
addend, val);
- /* FIXME: local symbols work as long as the
- * core and init pieces aren't separated too
- * far. If this is ever broken, you will trip
- * the check below. The way to fix it would
- * be to generate local stubs to go between init
- * and core */
- if((Elf64_Sxword)(val - dot - 8) > 0x800000 -1 ||
- (Elf64_Sxword)(val - dot - 8) < -0x800000) {
- printk(KERN_ERR "Module %s, symbol %s is out of range for PCREL22F relocation\n",
- me->name, strtab + sym->st_name);
- return -ENOEXEC;
- }
val = (val - dot - 8)/4;
+ CHECK_RELOC(val, 22);
*loc = (*loc & ~0x3ff1ffd) | reassemble_22(val);
break;
case R_PARISC_DIR64:
@@ -794,12 +846,8 @@ int module_finalize(const Elf_Ehdr *hdr,
addr = (u32 *)entry->addr;
printk("INSNS: %x %x %x %x\n",
addr[0], addr[1], addr[2], addr[3]);
- printk("stubs used %ld, stubs max %ld\n"
- "init_stubs used %ld, init stubs max %ld\n"
- "got entries used %ld, gots max %ld\n"
+ printk("got entries used %ld, gots max %ld\n"
"fdescs used %ld, fdescs max %ld\n",
- me->arch.stub_count, me->arch.stub_max,
- me->arch.init_stub_count, me->arch.init_stub_max,
me->arch.got_count, me->arch.got_max,
me->arch.fdesc_count, me->arch.fdesc_max);
#endif
@@ -829,7 +877,10 @@ int module_finalize(const Elf_Ehdr *hdr,
me->name, me->arch.got_count, MAX_GOTS);
return -EINVAL;
}
-
+
+ kfree(me->arch.section);
+ me->arch.section = NULL;
+
/* no symbol table */
if(symhdr == NULL)
return 0;
On Tuesday 30 December 2008 07:04:54 Helge Deller wrote:
> This is the second take of the patch series.
> Changes to previous version:
> - new CONFIG_HAVE_MODULE_SECTION_STUBS config option
> - put stub entries of a code section in front of the section
>
> ____________
> The parisc port (esp. the 32bit kernel) currently lacks the ability to
> load large kernel modules like xfs or ipv6. This is a long outstanding
> bug and has already been reported a few times, e.g.:
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=350482,
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=401439,
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=508489
>
> The symptom is like this:
> # modprobe xfs
> FATAL: Error inserting xfs
> (/lib/modules/2.6.26-1-parisc/kernel/fs/xfs/xfs.ko): Invalid module
> format
>
> In dmesg:
> module xfs relocation of symbol xfs_btree_read_bufs is out of range
> (0x3ffefffe in 17 bits)
>
> The reason for the failure is, that the architecture only provides the
> R_PARISC_PCREL17F (for 32bit kernels) and R_PARISC_PCREL22F (for PA2.0
> and 64bit kernels) relocations, which sometimes can't reach the target
> address of the stub entry if the kernel module is too large. Currently
> parisc (like other architectures) creates one big PLT section for all
> stubs at the beginning of the init and core sections.
>
> The following two patches changes the parisc module loader to put stubs
> for the code sections in front of each section, so that the distance to
> the stubs more easily fits into the available 17/22 bits.
So now any one section has to pass 17 bits to break? How close are you with
the xfs module?
But it's kind of nasty, overloading sh_entsize further. Could we instead
do something like add a arch_module_section_size() weak fn which you can
overload? We'd use that in get_offset() so our layout and size calculations
were correct, and use sh_size everywhere else.
Cheers,
Rusty.
Rusty Russell wrote:
> On Tuesday 30 December 2008 07:04:54 Helge Deller wrote:
>> This is the second take of the patch series.
>> Changes to previous version:
>> - new CONFIG_HAVE_MODULE_SECTION_STUBS config option
>> - put stub entries of a code section in front of the section
>>
>> ____________
>> The parisc port (esp. the 32bit kernel) currently lacks the ability to
>> load large kernel modules like xfs or ipv6. This is a long outstanding
>> bug and has already been reported a few times, e.g.:
>> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=350482,
>> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=401439,
>> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=508489
>>
>> The symptom is like this:
>> # modprobe xfs
>> FATAL: Error inserting xfs
>> (/lib/modules/2.6.26-1-parisc/kernel/fs/xfs/xfs.ko): Invalid module
>> format
>>
>> In dmesg:
>> module xfs relocation of symbol xfs_btree_read_bufs is out of range
>> (0x3ffefffe in 17 bits)
>>
>> The reason for the failure is, that the architecture only provides the
>> R_PARISC_PCREL17F (for 32bit kernels) and R_PARISC_PCREL22F (for PA2.0
>> and 64bit kernels) relocations, which sometimes can't reach the target
>> address of the stub entry if the kernel module is too large. Currently
>> parisc (like other architectures) creates one big PLT section for all
>> stubs at the beginning of the init and core sections.
>>
>> The following two patches changes the parisc module loader to put stubs
>> for the code sections in front of each section, so that the distance to
>> the stubs more easily fits into the available 17/22 bits.
>
> So now any one section has to pass 17 bits to break? How close are you with
> the xfs module?
I did not tested it very much, but xfs is around 1.1M on disk, and ~750K when
loaded. I think it breaked being 1/4 through the relocations.
> But it's kind of nasty, overloading sh_entsize further. Could we instead
> do something like add a arch_module_section_size() weak fn which you can
> overload?
Sure. I'll respin the patch.
> We'd use that in get_offset() so our layout and size calculations
> were correct,
Yes, good.
> and use sh_size everywhere else.
BTW, although the comment states that arches can change section sizes
in the module_frob_arch_sections() function:
/* Allow arches to frob section contents and sizes. */
it will break horrible if you do so.
What I found was, that if you change sh_size, at least the module
references / dependency chain will break when running lsmod.
Helge
On Wednesday 31 December 2008 09:32:41 Helge Deller wrote:
> BTW, although the comment states that arches can change section sizes
> in the module_frob_arch_sections() function:
> /* Allow arches to frob section contents and sizes. */
> it will break horrible if you do so.
> What I found was, that if you change sh_size, at least the module
> references / dependency chain will break when running lsmod.
Well, you can change the size of NOBITS sections: other things are likely
to be less successful. You could probably make them smaller, though.
Cheers,
Rusty.
[PATCH 1/2] module.c: fix module loading failure of large kernel modules
When creating the final layout of a kernel module in memory, allow the
module loader to reserve some additional memory in front of a given section.
This is currently only needed for the parisc port which needs to put the
stub entries there to fulfill the 17/22bit PCREL relocations with large
kernel modules like xfs.
Differences of this patch to previous versions:
- added weak funtion arch_module_section_size()
- no kernel config options needed
- no overloading of sh_entsize
Signed-off-by: Helge Deller <[email protected]>
diffstat:
include/linux/moduleloader.h | 4 ++++
kernel/module.c | 16 +++++++++++++---
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/include/linux/moduleloader.h b/include/linux/moduleloader.h
index eb10339..f2b1b62 100644
--- a/include/linux/moduleloader.h
+++ b/include/linux/moduleloader.h
@@ -13,6 +13,10 @@ int module_frob_arch_sections(Elf_Ehdr *hdr,
char *secstrings,
struct module *mod);
+/* Additional bytes needed by arch in front of individual sections */
+unsigned int arch_module_section_size(struct module *mod,
+ unsigned int section);
+
/* Allocator used for allocating struct module, core sections and init
sections. Returns NULL on failure. */
void *module_alloc(unsigned long size);
diff --git a/kernel/module.c b/kernel/module.c
index 1f4cc00..5b91b17 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1578,11 +1578,21 @@ static int simplify_symbols(Elf_Shdr *sechdrs,
return ret;
}
+/* Additional bytes needed by arch in front of individual sections */
+unsigned int __attribute__ ((weak)) arch_module_section_size(
+ struct module *mod, unsigned int section)
+{
+ /* default implementation just returns zero */
+ return 0;
+}
+
/* Update size with this section: return offset. */
-static long get_offset(unsigned int *size, Elf_Shdr *sechdr)
+static long get_offset(struct module *mod, unsigned int *size,
+ Elf_Shdr *sechdr, unsigned int section)
{
long ret;
+ *size += arch_module_section_size(mod, section);
ret = ALIGN(*size, sechdr->sh_addralign ?: 1);
*size = ret + sechdr->sh_size;
return ret;
@@ -1622,7 +1632,7 @@ static void layout_sections(struct module *mod,
|| strncmp(secstrings + s->sh_name,
".init", 5) == 0)
continue;
- s->sh_entsize = get_offset(&mod->core_size, s);
+ s->sh_entsize = get_offset(mod, &mod->core_size, s, i);
DEBUGP("\t%s\n", secstrings + s->sh_name);
}
if (m == 0)
@@ -1640,7 +1650,7 @@ static void layout_sections(struct module *mod,
|| strncmp(secstrings + s->sh_name,
".init", 5) != 0)
continue;
- s->sh_entsize = (get_offset(&mod->init_size, s)
+ s->sh_entsize = (get_offset(mod, &mod->init_size, s, i)
| INIT_OFFSET_MASK);
DEBUGP("\t%s\n", secstrings + s->sh_name);
}
[PATCH 2/2] parisc: fix module loading failure of large modules (take 3)
On 32bit (and sometimes 64bit) and with big kernel modules like xfs or
ipv6 the relocation types R_PARISC_PCREL17F and R_PARISC_PCREL22F may
fail to reach their PLT stub if we only create one big stub array for
all sections at the beginning of the core or init section.
With this patch we now instead add individual PLT stub entries
directly in front of the code sections where the stubs are actually
called. This reduces the distance between the PCREL location and the
stub entry so that the relocations can be fulfilled.
While calculating the final layout of the kernel module in memory, the
kernel module loader calls arch_module_section_size() to request the
to be reserved amount of memory in front of each individual section.
Tested with 32- and 64bit kernels.
Signed-off-by: Helge Deller <[email protected]>
diffstat:
include/asm/module.h | 6 -
kernel/module.c | 217 +++++++++++++++++++++++++++++++--------------------
2 files changed, 140 insertions(+), 83 deletions(-)
diff --git a/arch/parisc/include/asm/module.h b/arch/parisc/include/asm/module.h
index c2cb49e..1f41234 100644
--- a/arch/parisc/include/asm/module.h
+++ b/arch/parisc/include/asm/module.h
@@ -23,8 +23,10 @@ struct mod_arch_specific
{
unsigned long got_offset, got_count, got_max;
unsigned long fdesc_offset, fdesc_count, fdesc_max;
- unsigned long stub_offset, stub_count, stub_max;
- unsigned long init_stub_offset, init_stub_count, init_stub_max;
+ struct {
+ unsigned long stub_offset;
+ unsigned int stub_entries;
+ } *section;
int unwind_section;
struct unwind_table *unwind;
};
diff --git a/arch/parisc/kernel/module.c b/arch/parisc/kernel/module.c
index 44138c3..3228007 100644
--- a/arch/parisc/kernel/module.c
+++ b/arch/parisc/kernel/module.c
@@ -6,6 +6,7 @@
*
* Linux/PA-RISC Project (http://www.parisc-linux.org/)
* Copyright (C) 2003 Randolph Chung <tausq at debian . org>
+ * Copyright (C) 2008 Helge Deller <[email protected]>
*
*
* This program is free software; you can redistribute it and/or modify
@@ -24,6 +25,19 @@
*
*
* Notes:
+ * - PLT stub handling
+ * On 32bit (and sometimes 64bit) and with big kernel modules like xfs or
+ * ipv6 the relocation types R_PARISC_PCREL17F and R_PARISC_PCREL22F may
+ * fail to reach their PLT stub if we only create one big stub array for
+ * all sections at the beginning of the core or init section.
+ * Instead we now insert individual PLT stub entries directly in front of
+ * of the code sections where the stubs are actually called.
+ * This reduces the distance between the PCREL location and the stub entry
+ * so that the relocations can be fulfilled.
+ * While calculating the final layout of the kernel module in memory, the
+ * kernel module loader calls arch_module_section_size() to request the
+ * to be reserved amount of memory in front of each individual section.
+ *
* - SEGREL32 handling
* We are not doing SEGREL32 handling correctly. According to the ABI, we
* should do a value offset, like this:
@@ -58,9 +72,13 @@
#define DEBUGP(fmt...)
#endif
+#define RELOC_REACHABLE(val, bits) \
+ (( ( !((val) & (1<<((bits)-1))) && ((val)>>(bits)) != 0 ) || \
+ ( ((val) & (1<<((bits)-1))) && ((val)>>(bits)) != (((__typeof__(val))(~0))>>((bits)+2)))) ? \
+ 0 : 1)
+
#define CHECK_RELOC(val, bits) \
- if ( ( !((val) & (1<<((bits)-1))) && ((val)>>(bits)) != 0 ) || \
- ( ((val) & (1<<((bits)-1))) && ((val)>>(bits)) != (((__typeof__(val))(~0))>>((bits)+2)))) { \
+ if (!RELOC_REACHABLE(val, bits)) { \
printk(KERN_ERR "module %s relocation of symbol %s is out of range (0x%lx in %d bits)\n", \
me->name, strtab + sym->st_name, (unsigned long)val, bits); \
return -ENOEXEC; \
@@ -92,13 +110,6 @@ static inline int in_local(struct module *me, void *loc)
return in_init(me, loc) || in_core(me, loc);
}
-static inline int in_local_section(struct module *me, void *loc, void *dot)
-{
- return (in_init(me, loc) && in_init(me, dot)) ||
- (in_core(me, loc) && in_core(me, dot));
-}
-
-
#ifndef CONFIG_64BIT
struct got_entry {
Elf32_Addr addr;
@@ -258,23 +269,43 @@ static inline unsigned long count_stubs(const Elf_Rela *rela, unsigned long n)
/* Free memory returned from module_alloc */
void module_free(struct module *mod, void *module_region)
{
+ kfree(mod->arch.section);
+ mod->arch.section = NULL;
+
vfree(module_region);
/* FIXME: If module_region == mod->init_region, trim exception
table entries. */
}
+
+/* Additional bytes needed in front of individual sections. */
+unsigned int arch_module_section_size(struct module *mod,
+ unsigned int section)
+{
+ /* size needed for all stubs of this section (including
+ * one additional for correct alignment of the stubs) */
+ return (mod->arch.section[section].stub_entries + 1)
+ * sizeof(struct stub_entry);
+}
+
#define CONST
int module_frob_arch_sections(CONST Elf_Ehdr *hdr,
CONST Elf_Shdr *sechdrs,
CONST char *secstrings,
struct module *me)
{
- unsigned long gots = 0, fdescs = 0, stubs = 0, init_stubs = 0;
+ unsigned long gots = 0, fdescs = 0, len;
unsigned int i;
+ len = hdr->e_shnum * sizeof(me->arch.section[0]);
+ me->arch.section = kzalloc(len, GFP_KERNEL);
+ if (!me->arch.section)
+ return -ENOMEM;
+
for (i = 1; i < hdr->e_shnum; i++) {
- const Elf_Rela *rels = (void *)hdr + sechdrs[i].sh_offset;
+ const Elf_Rela *rels = (void *)sechdrs[i].sh_addr;
unsigned long nrels = sechdrs[i].sh_size / sizeof(*rels);
+ unsigned int count, s;
if (strncmp(secstrings + sechdrs[i].sh_name,
".PARISC.unwind", 14) == 0)
@@ -290,11 +321,23 @@ int module_frob_arch_sections(CONST Elf_Ehdr *hdr,
*/
gots += count_gots(rels, nrels);
fdescs += count_fdescs(rels, nrels);
- if(strncmp(secstrings + sechdrs[i].sh_name,
- ".rela.init", 10) == 0)
- init_stubs += count_stubs(rels, nrels);
- else
- stubs += count_stubs(rels, nrels);
+
+ /* XXX: By sorting the relocs and finding duplicate entries
+ * we could reduce the number of necessary stubs and save
+ * some memory. */
+ count = count_stubs(rels, nrels);
+ if (!count)
+ continue;
+
+ /* so we need relocation stubs. reserve necessary memory. */
+ /* sh_info gives the section for which we need to add stubs. */
+ s = sechdrs[i].sh_info;
+
+ /* each code section should only have one relocation section */
+ WARN_ON(me->arch.section[s].stub_entries);
+
+ /* store number of stubs we need for this section */
+ me->arch.section[s].stub_entries += count;
}
/* align things a bit */
@@ -306,18 +349,8 @@ int module_frob_arch_sections(CONST Elf_Ehdr *hdr,
me->arch.fdesc_offset = me->core_size;
me->core_size += fdescs * sizeof(Elf_Fdesc);
- me->core_size = ALIGN(me->core_size, 16);
- me->arch.stub_offset = me->core_size;
- me->core_size += stubs * sizeof(struct stub_entry);
-
- me->init_size = ALIGN(me->init_size, 16);
- me->arch.init_stub_offset = me->init_size;
- me->init_size += init_stubs * sizeof(struct stub_entry);
-
me->arch.got_max = gots;
me->arch.fdesc_max = fdescs;
- me->arch.stub_max = stubs;
- me->arch.init_stub_max = init_stubs;
return 0;
}
@@ -380,23 +413,27 @@ enum elf_stub_type {
};
static Elf_Addr get_stub(struct module *me, unsigned long value, long addend,
- enum elf_stub_type stub_type, int init_section)
+ enum elf_stub_type stub_type, Elf_Addr loc0, unsigned int targetsec)
{
- unsigned long i;
struct stub_entry *stub;
- if(init_section) {
- i = me->arch.init_stub_count++;
- BUG_ON(me->arch.init_stub_count > me->arch.init_stub_max);
- stub = me->module_init + me->arch.init_stub_offset +
- i * sizeof(struct stub_entry);
- } else {
- i = me->arch.stub_count++;
- BUG_ON(me->arch.stub_count > me->arch.stub_max);
- stub = me->module_core + me->arch.stub_offset +
- i * sizeof(struct stub_entry);
+ /* initialize stub_offset to point in front of the section */
+ if (!me->arch.section[targetsec].stub_offset) {
+ loc0 -= (me->arch.section[targetsec].stub_entries + 1) *
+ sizeof(struct stub_entry);
+ /* get correct alignment for the stubs */
+ loc0 = ALIGN(loc0, sizeof(struct stub_entry));
+ me->arch.section[targetsec].stub_offset = loc0;
}
+ /* get address of stub entry */
+ stub = (void *) me->arch.section[targetsec].stub_offset;
+ me->arch.section[targetsec].stub_offset += sizeof(struct stub_entry);
+
+ /* do not write outside available stub area */
+ BUG_ON(0 == me->arch.section[targetsec].stub_entries--);
+
+
#ifndef CONFIG_64BIT
/* for 32-bit the stub looks like this:
* ldil L'XXX,%r1
@@ -489,15 +526,19 @@ int apply_relocate_add(Elf_Shdr *sechdrs,
Elf32_Addr val;
Elf32_Sword addend;
Elf32_Addr dot;
+ Elf_Addr loc0;
+ unsigned int targetsec = sechdrs[relsec].sh_info;
//unsigned long dp = (unsigned long)$global$;
register unsigned long dp asm ("r27");
DEBUGP("Applying relocate section %u to %u\n", relsec,
- sechdrs[relsec].sh_info);
+ targetsec);
for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) {
/* This is where to make the change */
- loc = (void *)sechdrs[sechdrs[relsec].sh_info].sh_addr
+ loc = (void *)sechdrs[targetsec].sh_addr
+ rel[i].r_offset;
+ /* This is the start of the target section */
+ loc0 = sechdrs[targetsec].sh_addr;
/* This is the symbol it is referring to */
sym = (Elf32_Sym *)sechdrs[symindex].sh_addr
+ ELF32_R_SYM(rel[i].r_info);
@@ -569,19 +610,32 @@ int apply_relocate_add(Elf_Shdr *sechdrs,
break;
case R_PARISC_PCREL17F:
/* 17-bit PC relative address */
- val = get_stub(me, val, addend, ELF_STUB_GOT, in_init(me, loc));
+ /* calculate direct call offset */
+ val += addend;
val = (val - dot - 8)/4;
- CHECK_RELOC(val, 17)
+ if (!RELOC_REACHABLE(val, 17)) {
+ /* direct distance too far, create
+ * stub entry instead */
+ val = get_stub(me, sym->st_value, addend,
+ ELF_STUB_DIRECT, loc0, targetsec);
+ val = (val - dot - 8)/4;
+ CHECK_RELOC(val, 17);
+ }
*loc = (*loc & ~0x1f1ffd) | reassemble_17(val);
break;
case R_PARISC_PCREL22F:
/* 22-bit PC relative address; only defined for pa20 */
- val = get_stub(me, val, addend, ELF_STUB_GOT, in_init(me, loc));
- DEBUGP("STUB FOR %s loc %lx+%lx at %lx\n",
- strtab + sym->st_name, (unsigned long)loc, addend,
- val)
+ /* calculate direct call offset */
+ val += addend;
val = (val - dot - 8)/4;
- CHECK_RELOC(val, 22);
+ if (!RELOC_REACHABLE(val, 22)) {
+ /* direct distance too far, create
+ * stub entry instead */
+ val = get_stub(me, sym->st_value, addend,
+ ELF_STUB_DIRECT, loc0, targetsec);
+ val = (val - dot - 8)/4;
+ CHECK_RELOC(val, 22);
+ }
*loc = (*loc & ~0x3ff1ffd) | reassemble_22(val);
break;
@@ -610,13 +664,17 @@ int apply_relocate_add(Elf_Shdr *sechdrs,
Elf64_Addr val;
Elf64_Sxword addend;
Elf64_Addr dot;
+ Elf_Addr loc0;
+ unsigned int targetsec = sechdrs[relsec].sh_info;
DEBUGP("Applying relocate section %u to %u\n", relsec,
- sechdrs[relsec].sh_info);
+ targetsec);
for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) {
/* This is where to make the change */
- loc = (void *)sechdrs[sechdrs[relsec].sh_info].sh_addr
+ loc = (void *)sechdrs[targetsec].sh_addr
+ rel[i].r_offset;
+ /* This is the start of the target section */
+ loc0 = sechdrs[targetsec].sh_addr;
/* This is the symbol it is referring to */
sym = (Elf64_Sym *)sechdrs[symindex].sh_addr
+ ELF64_R_SYM(rel[i].r_info);
@@ -672,42 +730,40 @@ int apply_relocate_add(Elf_Shdr *sechdrs,
DEBUGP("PCREL22F Symbol %s loc %p val %lx\n",
strtab + sym->st_name,
loc, val);
+ val += addend;
/* can we reach it locally? */
- if(!in_local_section(me, (void *)val, (void *)dot)) {
-
- if (in_local(me, (void *)val))
- /* this is the case where the
- * symbol is local to the
- * module, but in a different
- * section, so stub the jump
- * in case it's more than 22
- * bits away */
- val = get_stub(me, val, addend, ELF_STUB_DIRECT,
- in_init(me, loc));
- else if (strncmp(strtab + sym->st_name, "$$", 2)
+ if (in_local(me, (void *)val)) {
+ /* this is the case where the symbol is local
+ * to the module, but in a different section,
+ * so stub the jump in case it's more than 22
+ * bits away */
+ val = (val - dot - 8)/4;
+ if (!RELOC_REACHABLE(val, 22)) {
+ /* direct distance too far, create
+ * stub entry instead */
+ val = get_stub(me, sym->st_value,
+ addend, ELF_STUB_DIRECT,
+ loc0, targetsec);
+ } else {
+ /* Ok, we can reach it directly. */
+ val = sym->st_value;
+ val += addend;
+ }
+ } else {
+ val = sym->st_value;
+ if (strncmp(strtab + sym->st_name, "$$", 2)
== 0)
val = get_stub(me, val, addend, ELF_STUB_MILLI,
- in_init(me, loc));
+ loc0, targetsec);
else
val = get_stub(me, val, addend, ELF_STUB_GOT,
- in_init(me, loc));
+ loc0, targetsec);
}
DEBUGP("STUB FOR %s loc %lx, val %lx+%lx at %lx\n",
strtab + sym->st_name, loc, sym->st_value,
addend, val);
- /* FIXME: local symbols work as long as the
- * core and init pieces aren't separated too
- * far. If this is ever broken, you will trip
- * the check below. The way to fix it would
- * be to generate local stubs to go between init
- * and core */
- if((Elf64_Sxword)(val - dot - 8) > 0x800000 -1 ||
- (Elf64_Sxword)(val - dot - 8) < -0x800000) {
- printk(KERN_ERR "Module %s, symbol %s is out of range for PCREL22F relocation\n",
- me->name, strtab + sym->st_name);
- return -ENOEXEC;
- }
val = (val - dot - 8)/4;
+ CHECK_RELOC(val, 22);
*loc = (*loc & ~0x3ff1ffd) | reassemble_22(val);
break;
case R_PARISC_DIR64:
@@ -794,12 +850,8 @@ int module_finalize(const Elf_Ehdr *hdr,
addr = (u32 *)entry->addr;
printk("INSNS: %x %x %x %x\n",
addr[0], addr[1], addr[2], addr[3]);
- printk("stubs used %ld, stubs max %ld\n"
- "init_stubs used %ld, init stubs max %ld\n"
- "got entries used %ld, gots max %ld\n"
+ printk("got entries used %ld, gots max %ld\n"
"fdescs used %ld, fdescs max %ld\n",
- me->arch.stub_count, me->arch.stub_max,
- me->arch.init_stub_count, me->arch.init_stub_max,
me->arch.got_count, me->arch.got_max,
me->arch.fdesc_count, me->arch.fdesc_max);
#endif
@@ -829,7 +881,10 @@ int module_finalize(const Elf_Ehdr *hdr,
me->name, me->arch.got_count, MAX_GOTS);
return -EINVAL;
}
-
+
+ kfree(me->arch.section);
+ me->arch.section = NULL;
+
/* no symbol table */
if(symhdr == NULL)
return 0;
On Wednesday 31 December 2008 22:01:18 Helge Deller wrote:
> +/* Additional bytes needed by arch in front of individual sections */
> +unsigned int arch_module_section_size(struct module *mod,
> + unsigned int section);
> +
...
> +/* Additional bytes needed by arch in front of individual sections */
> +unsigned int __attribute__ ((weak)) arch_module_section_size(
> + struct module *mod, unsigned int section)
> +{
> + /* default implementation just returns zero */
> + return 0;
> +}
Not quite what I had in mind... let me show you:
/* Bytes needed for a section: default is just the section size. */
unsigned int __attribute__((weak))
arch_module_section_size(struct module *mod, Elf_Shdr *sechdrs, unsigned int sec)
{
return sechdrs[sec].sh_size;
}
Otherwise I'd have called it "arch_module_extra_size()".
Cheers,
Rusty.
Rusty Russell wrote:
> Not quite what I had in mind... let me show you:
> ...
> Otherwise I'd have called it "arch_module_extra_size()".
Hmm, this needs more thinking then.
So, in summary this would be your proposed change (?):
+/* Bytes needed for a section: default is just the section size. */
+unsigned int __attribute__((weak))
+arch_module_section_size(struct module *mod, Elf_Shdr *sechdrs, unsigned int sec)
+{
+ return sechdrs[sec].sh_size;
+}
+
/* Update size with this section: return offset. */
-static long get_offset(unsigned int *size, Elf_Shdr *sechdr)
+static long get_offset(struct module *mod, unsigned int *size,
+ Elf_Shdr *sechdr, unsigned int section)
{
long ret;
ret = ALIGN(*size, sechdr->sh_addralign ?: 1);
- *size = ret + sechdr->sh_size;
+ *size = ret + arch_module_section_size(mod, sechdr, section);
return ret;
}
This would mean that I can increase the section size in the arch-specific function
by returning a bigger value than sh_size.
This would give me space at the end of the section, but not at the beginning
(which is what I need), as sh_entsize (the offset into memory) will stay the
same as before.
Example: Having an initial value for core_size of zero, the code bits of the
very first section are still copied into the very first byte in memory, leaving
me no room for the stubs.
The important part of get_offset() is, which value is returned to the caller.
Let's try another example which would work for me:
+static long get_offset(struct module *mod, unsigned int *size,
+ Elf_Shdr *sechdr, unsigned int section)
{
- long ret;
+ long ret, sect_size;
+ sect_size = arch_module_section_size(mod, sechdr, section);
+ *size += (sect_size - sechdr->sh_size);
ret = ALIGN(*size, sechdr->sh_addralign ?: 1);
*size = ret + sechdr->sh_size;
return ret;
IMHO, this is hackish and ugly.
A last option for me would be to set core_size to the initial value
of bytes which I would need for section 1 and returning in
arch_module_section_size() when asked for the size of section 1 the
sum of sh_size[section 1] + additional_bytes_needed_for_section_2,
and so on...
Any proposals?
Helge
On Wed, 31 Dec 2008 09:47:19 -0800 (PST) Linus Torvalds <[email protected]> wrote:
>
> > (I just notice this because I saw the warning about swiotlb_alloc_boot()
> > not being __init but calling __alloc_bootmem_low and so I looked at the
> > code yesterday)
>
> Lucky us. What's nasty about this is that most developers probably have
> updated versions of gcc, and are then surprised when some odd user has
> insane behavior that doesn't match the source code - because the compiler
> did something unexpected.
Adrian claimed that it was gcc-4.1.0 and 4.1.1 only. He proposed
banning them: http://lkml.org/lkml/2008/8/5/444
> I guess I could make a sparse rule for this, but nobody seems to run or
> care about sparse anyway. Sad.
No, there are some people who regularly run sparse and fix stuff. Pending
things here include:
y:/usr/src/25> grep sparse series
input-ads7846c-sparse-lock-annotation.patch
hugetlb-fix-sparse-warnings.patch
lib-fix-sparse-shadowed-variable-warning.patch
lib-proportionsc-trivial-sparse-lock-annotation.patch
nvidia-fix-sparse-warnings.patch
viafb-fix-sparse-warnings.patch
pm3fb-fix-sparse-warning.patch
neofb-fix-sparse-warnings.patch
i810-fix-sparse-warnings.patch
intelfb-fix-sparse-warnings.patch
Which is a good way for it to be used.
>
> I guess I could make a sparse rule for this, but nobody seems to run or
> care about sparse anyway. Sad.
There is some increased janitorial effort recently.
Try to grep for [Ss]+parse in the shortlog for the last couple
of months.
Sam
> Some gcc versions will inline weak functions if they are in scope - even
> if there is a non-weak function somewhere else. So you MUST NOT have the
> weak definition in the same file (or indirectly called through some inline
> functions in a header file) as the call. Because if you do, then any user
> with the wrong version of gcc will get the weak function semantics, even
> if it was meant to be overridden by something else.
Does this mean lib/swiotlb.c is broken now? It has eg:
void * __weak swiotlb_alloc_boot(size_t size, unsigned long nslabs)
and then
void __init
swiotlb_init_with_default_size(size_t default_size)
{
...
io_tlb_start = swiotlb_alloc_boot(bytes, io_tlb_nslabs);
later on in the same file.
(I just notice this because I saw the warning about swiotlb_alloc_boot()
not being __init but calling __alloc_bootmem_low and so I looked at the
code yesterday)
- R.
Linus Torvalds wrote:
>
> On Wed, 31 Dec 2008, Helge Deller wrote:
>> [PATCH 1/2] module.c: fix module loading failure of large kernel modules
>>
>> When creating the final layout of a kernel module in memory, allow the
>> module loader to reserve some additional memory in front of a given section.
>> This is currently only needed for the parisc port which needs to put the
>> stub entries there to fulfill the 17/22bit PCREL relocations with large
>> kernel modules like xfs.
>>
>> Differences of this patch to previous versions:
>> - added weak funtion arch_module_section_size()
>
> This doesn't work.
>
> We've had this bug several times now, and one of them just very recently.
>
> Some gcc versions will inline weak functions if they are in scope - even
> if there is a non-weak function somewhere else. So you MUST NOT have the
> weak definition in the same file (or indirectly called through some inline
> functions in a header file) as the call. Because if you do, then any user
> with the wrong version of gcc will get the weak function semantics, even
> if it was meant to be overridden by something else.
Ok, that might explain why I saw some strange things in the unwind tables
after that I switched to using the weak function (hppa-crosscompiler, gcc-3.3.4).
>> +/* Additional bytes needed by arch in front of individual sections */
>> +unsigned int __attribute__ ((weak)) arch_module_section_size(
>> + struct module *mod, unsigned int section)
>
> We don't write out that whole "__attribute__" crud. We use
>
> unsigned in __weak arch_module_section_size(struct module *mod, unsigned int section)
>
> instead.
Ok.
> But as mentioned, it needs to be in another compilation unit.
Understood.
Rusty, back to you for an advise on how to continue ;-)
(I assume Rusty is just right now celebrating new-year)
Helge
On Wed, 31 Dec 2008, Andrew Morton wrote:
>
> Adrian claimed that it was gcc-4.1.0 and 4.1.1 only. He proposed
> banning them: http://lkml.org/lkml/2008/8/5/444
If it really is just those releases, then yes, considering the number of
cases we apparently have, and considering how ugly it is in some cases to
move the weak function anywhere else, maybe banning those versions is the
proper thing to do.
It probably won't hurt very many people - yeah, some people will be forced
to upgrade, but I have this memory of early 4.1 having had other bugs
anyway, so it's probably a good idea.
Linus
On Wed, 31 Dec 2008, Linus Torvalds wrote:
>
> I guess I could make a sparse rule for this, but nobody seems to run or
> care about sparse anyway. Sad.
Ho humm. Looks like a sparse rule might be in order. These are just from
my (smallish) personal config. I haven't checked them all, but I did check
the first two, and yes, sparse did get it right.
init/main.c:548:24: warning: Calling weak function 'smp_setup_processor_id' in file scope
init/main.c:674:24: warning: Calling weak function 'thread_info_cache_init' in file scope
arch/x86/kernel/e820.c:1364:38: warning: Calling weak function 'machine_specific_memory_setup' in file scope
arch/x86/kernel/e820.c:1371:20: warning: Calling weak function 'memory_setup' in file scope
arch/x86/kernel/reboot.c:386:22: warning: Calling weak function 'mach_reboot_fixups' in file scope
kernel/sched.c:7720:26: warning: Calling weak function 'arch_update_cpu_topology' in file scope
kernel/sched.c:7808:41: warning: Calling weak function 'arch_update_cpu_topology' in file scope
kernel/fork.c:229:29: warning: Calling weak function 'arch_dup_task_struct' in file scope
kernel/fork.c:1325:44: warning: Calling weak function 'idle_regs' in file scope
kernel/sched_clock.c:158:19: warning: Calling weak function 'sched_clock' in file scope
kernel/sched_clock.c:207:19: warning: Calling weak function 'sched_clock' in file scope
kernel/time/timekeeping.c:283:43: warning: Calling weak function 'read_persistent_clock' in file scope
kernel/time/timekeeping.c:318:43: warning: Calling weak function 'read_persistent_clock' in file scope
kernel/time/timekeeping.c:353:50: warning: Calling weak function 'read_persistent_clock' in file scope
mm/vmalloc.c:1645:18: warning: Calling weak function 'vmalloc_sync_all' in file scope
mm/hugetlb.c:1039:32: warning: Calling weak function 'alloc_bootmem_huge_page' in file scope
mm/sparse.c:448:29: warning: Calling weak function 'vmemmap_populate_print_last' in file scope
fs/pipe.c:1069:18: warning: Calling weak function 'sys_pipe2' in file scope
fs/proc/meminfo.c:145:21: warning: Calling weak function 'arch_report_meminfo' in file scope
drivers/acpi/pci_irq.c:676:21: warning: Calling weak function 'acpi_unregister_gsi' in file scope
drivers/char/mem.c:168:23: warning: Calling weak function 'unxlate_dev_mem_ptr' in file scope
drivers/char/mem.c:172:22: warning: Calling weak function 'unxlate_dev_mem_ptr' in file scope
drivers/char/mem.c:240:23: warning: Calling weak function 'unxlate_dev_mem_ptr' in file scope
drivers/char/mem.c:246:22: warning: Calling weak function 'unxlate_dev_mem_ptr' in file scope
drivers/char/mem.c:318:12: warning: Calling weak function 'map_devmem' in file scope
drivers/char/mem.c:324:14: warning: Calling weak function 'unmap_devmem' in file scope
drivers/char/mem.c:349:35: warning: Calling weak function 'phys_mem_access_prot_allowed' in file scope
drivers/char/mem.c:365:15: warning: Calling weak function 'unmap_devmem' in file scope
drivers/pci/probe.c:1159:36: warning: Calling weak function 'set_pci_bus_resources_arch_default' in file scope
drivers/pci/pci.c:1015:24: warning: Calling weak function 'pcibios_disable_device' in file scope
drivers/pci/pci.c:1043:37: warning: Calling weak function 'pcibios_set_pcie_reset_state' in file scope
drivers/pci/pci-sysfs.c:897:39: warning: Calling weak function 'pcibios_add_platform_entries' in file scope
drivers/pci/msi.c:49:27: warning: Calling weak function 'arch_setup_msi_irq' in file scope
drivers/pci/msi.c:69:25: warning: Calling weak function 'arch_teardown_msi_irq' in file scope
drivers/pci/msi.c:421:27: warning: Calling weak function 'arch_setup_msi_irqs' in file scope
drivers/pci/msi.c:492:27: warning: Calling weak function 'arch_setup_msi_irqs' in file scope
drivers/pci/msi.c:562:29: warning: Calling weak function 'arch_msi_check_device' in file scope
drivers/pci/msi.c:654:24: warning: Calling weak function 'arch_teardown_msi_irqs' in file scope
lib/swiotlb.c:141:28: warning: Calling weak function 'swiotlb_phys_to_bus' in file scope
lib/swiotlb.c:146:41: warning: Calling weak function 'swiotlb_bus_to_phys' in file scope
lib/swiotlb.c:156:28: warning: Calling weak function 'swiotlb_phys_to_bus' in file scope
lib/swiotlb.c:167:30: warning: Calling weak function 'swiotlb_phys_to_bus' in file scope
lib/swiotlb.c:168:28: warning: Calling weak function 'swiotlb_phys_to_bus' in file scope
lib/swiotlb.c:204:35: warning: Calling weak function 'swiotlb_alloc_boot' in file scope
lib/swiotlb.c:260:31: warning: Calling weak function 'swiotlb_alloc' in file scope
lib/swiotlb.c:336:58: warning: Calling weak function 'swiotlb_arch_range_needs_mapping' in file scope
lib/swiotlb.c:336:58: warning: Calling weak function 'swiotlb_arch_range_needs_mapping' in file scope
I bet there are others that I don't see just because I don't compile the
code.
The appended trial sparse diff is against sparse -git as of a couple of
days ago if anybody wants to run it themselves.
The _logical_ fix would be to add "noinline" to the definition of
"__weak", but that's reported not to help. I think the bug may be in the
assembler that pre-links the call if it sees it in file scope. Or maybe
gcc doesn't honor noinline for empty functions. Whatever.
Regardless, it's a damn pain.
Linus
---
evaluate.c | 7 +++++++
parse.c | 7 ++++---
symbol.h | 3 ++-
3 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/evaluate.c b/evaluate.c
index f976645..0ae6c93 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -2744,6 +2744,13 @@ static int evaluate_symbol_call(struct expression *expr)
if (ctype->op && ctype->op->evaluate)
return ctype->op->evaluate(expr);
+ if (ctype->ctype.modifiers & MOD_WEAK) {
+ struct symbol *fn = ctype->ctype.base_type;
+
+ if (fn->stmt || fn->inline_stmt)
+ warning(expr->pos, "Calling weak function '%s' in file scope", show_ident(ctype->ident));
+ }
+
if (ctype->ctype.modifiers & MOD_INLINE) {
int ret;
struct symbol *curr = current_fn;
diff --git a/parse.c b/parse.c
index eb31871..1ea497f 100644
--- a/parse.c
+++ b/parse.c
@@ -289,6 +289,9 @@ static struct init_keyword {
{ "word", NS_KEYWORD, MOD_LONG, .op = &mode_spec_op },
{ "__word__", NS_KEYWORD, MOD_LONG, .op = &mode_spec_op },
+ { "weak", NS_KEYWORD, MOD_WEAK, .op = &attr_mod_op },
+ { "__weak__", NS_KEYWORD, MOD_WEAK, .op = &attr_mod_op },
+
/* Ignored attributes */
{ "nothrow", NS_KEYWORD, .op = &ignore_attr_op },
{ "__nothrow", NS_KEYWORD, .op = &ignore_attr_op },
@@ -317,8 +320,6 @@ static struct init_keyword {
{ "__sentinel__", NS_KEYWORD, .op = &ignore_attr_op },
{ "regparm", NS_KEYWORD, .op = &ignore_attr_op },
{ "__regparm__", NS_KEYWORD, .op = &ignore_attr_op },
- { "weak", NS_KEYWORD, .op = &ignore_attr_op },
- { "__weak__", NS_KEYWORD, .op = &ignore_attr_op },
{ "alias", NS_KEYWORD, .op = &ignore_attr_op },
{ "__alias__", NS_KEYWORD, .op = &ignore_attr_op },
{ "pure", NS_KEYWORD, .op = &ignore_attr_op },
@@ -1582,7 +1583,7 @@ static struct statement *start_function(struct symbol *sym)
start_function_scope();
ret = alloc_symbol(sym->pos, SYM_NODE);
ret->ctype = sym->ctype.base_type->ctype;
- ret->ctype.modifiers &= ~(MOD_STORAGE | MOD_CONST | MOD_VOLATILE | MOD_INLINE | MOD_ADDRESSABLE | MOD_NOCAST | MOD_NODEREF | MOD_ACCESSED | MOD_TOPLEVEL);
+ ret->ctype.modifiers &= ~(MOD_STORAGE | MOD_CONST | MOD_VOLATILE | MOD_INLINE | MOD_ADDRESSABLE | MOD_NOCAST | MOD_NODEREF | MOD_ACCESSED | MOD_TOPLEVEL | MOD_WEAK);
ret->ctype.modifiers |= (MOD_AUTO | MOD_REGISTER);
bind_symbol(ret, &return_ident, NS_ITERATOR);
stmt->ret = ret;
diff --git a/symbol.h b/symbol.h
index c4d7f28..b3fcccd 100644
--- a/symbol.h
+++ b/symbol.h
@@ -185,6 +185,7 @@ struct symbol {
#define MOD_LONGLONG 0x0800
#define MOD_TYPEDEF 0x1000
+#define MOD_WEAK 0x2000
#define MOD_INLINE 0x40000
#define MOD_ADDRESSABLE 0x80000
@@ -205,7 +206,7 @@ struct symbol {
#define MOD_BITWISE 0x80000000
#define MOD_NONLOCAL (MOD_EXTERN | MOD_TOPLEVEL)
-#define MOD_STORAGE (MOD_AUTO | MOD_REGISTER | MOD_STATIC | MOD_EXTERN | MOD_INLINE | MOD_TOPLEVEL | MOD_FORCE)
+#define MOD_STORAGE (MOD_AUTO | MOD_REGISTER | MOD_STATIC | MOD_EXTERN | MOD_INLINE | MOD_TOPLEVEL | MOD_FORCE | MOD_WEAK)
#define MOD_SIGNEDNESS (MOD_SIGNED | MOD_UNSIGNED | MOD_EXPLICITLY_SIGNED)
#define MOD_SPECIFIER (MOD_CHAR | MOD_SHORT | MOD_LONG | MOD_LONGLONG | MOD_SIGNEDNESS)
#define MOD_SIZE (MOD_CHAR | MOD_SHORT | MOD_LONG | MOD_LONGLONG)
On Thursday 01 January 2009 03:59:36 Linus Torvalds wrote:
>
> On Wed, 31 Dec 2008, Helge Deller wrote:
> >
> > [PATCH 1/2] module.c: fix module loading failure of large kernel modules
> >
> > When creating the final layout of a kernel module in memory, allow the
> > module loader to reserve some additional memory in front of a given section.
> > This is currently only needed for the parisc port which needs to put the
> > stub entries there to fulfill the 17/22bit PCREL relocations with large
> > kernel modules like xfs.
> >
> > Differences of this patch to previous versions:
> > - added weak funtion arch_module_section_size()
>
> This doesn't work.
>
> We've had this bug several times now, and one of them just very recently.
>
> Some gcc versions will inline weak functions if they are in scope
Ah, someone hit this elsewhere and thought this was an arch-specific bug.
Any chance we can just kill those compiler versions and move on with our
lives? 4.1.3 definitely doesn't have the problem.
> We don't write out that whole "__attribute__" crud.
I think what you mean is "I prefer __weak". Which is fine, and not
trivially disprovable by grep.
It's a gratuitous kernelism, but it's an inoffensive one.
Rusty.
On Wed, 31 Dec 2008, Helge Deller wrote:
>
> [PATCH 1/2] module.c: fix module loading failure of large kernel modules
>
> When creating the final layout of a kernel module in memory, allow the
> module loader to reserve some additional memory in front of a given section.
> This is currently only needed for the parisc port which needs to put the
> stub entries there to fulfill the 17/22bit PCREL relocations with large
> kernel modules like xfs.
>
> Differences of this patch to previous versions:
> - added weak funtion arch_module_section_size()
This doesn't work.
We've had this bug several times now, and one of them just very recently.
Some gcc versions will inline weak functions if they are in scope - even
if there is a non-weak function somewhere else. So you MUST NOT have the
weak definition in the same file (or indirectly called through some inline
functions in a header file) as the call. Because if you do, then any user
with the wrong version of gcc will get the weak function semantics, even
if it was meant to be overridden by something else.
> +/* Additional bytes needed by arch in front of individual sections */
> +unsigned int __attribute__ ((weak)) arch_module_section_size(
> + struct module *mod, unsigned int section)
We don't write out that whole "__attribute__" crud. We use
unsigned in __weak arch_module_section_size(struct module *mod, unsigned int section)
instead. But as mentioned, it needs to be in another compilation unit.
Linus
On Wed, 31 Dec 2008, Roland Dreier wrote:
>
> Does this mean lib/swiotlb.c is broken now?
Yes it does.
> It has eg:
>
> void * __weak swiotlb_alloc_boot(size_t size, unsigned long nslabs)
>
> and then
>
> void __init
> swiotlb_init_with_default_size(size_t default_size)
> {
> ...
> io_tlb_start = swiotlb_alloc_boot(bytes, io_tlb_nslabs);
>
> later on in the same file.
Good eyes.
Ingo? This comes from commit b81ea27b2329bf44b30c427800954f845896d476, by
Ian, through Jeremy.
> (I just notice this because I saw the warning about swiotlb_alloc_boot()
> not being __init but calling __alloc_bootmem_low and so I looked at the
> code yesterday)
Lucky us. What's nasty about this is that most developers probably have
updated versions of gcc, and are then surprised when some odd user has
insane behavior that doesn't match the source code - because the compiler
did something unexpected.
I guess I could make a sparse rule for this, but nobody seems to run or
care about sparse anyway. Sad.
Linus
On Wed, 2008-12-31 at 09:29 -0800, Linus Torvalds wrote:
>
> On Wed, 31 Dec 2008, Helge Deller wrote:
> >
> > [PATCH 1/2] module.c: fix module loading failure of large kernel modules
> >
> > When creating the final layout of a kernel module in memory, allow the
> > module loader to reserve some additional memory in front of a given section.
> > This is currently only needed for the parisc port which needs to put the
> > stub entries there to fulfill the 17/22bit PCREL relocations with large
> > kernel modules like xfs.
> >
> > Differences of this patch to previous versions:
> > - added weak funtion arch_module_section_size()
>
> This doesn't work.
>
> We've had this bug several times now, and one of them just very recently.
>
> Some gcc versions will inline weak functions if they are in scope - even
> if there is a non-weak function somewhere else. So you MUST NOT have the
> weak definition in the same file (or indirectly called through some inline
> functions in a header file) as the call. Because if you do, then any user
> with the wrong version of gcc will get the weak function semantics, even
> if it was meant to be overridden by something else.
Um, but we got told to use all these weak functions instead of the
ARCH_HAS... defines. They certainly litter the x86 boot code. The
standard pattern was to put the weak function in the file where it was
used, which is what you're now saying will miscompile.
Which gcc versions are the problem? Because it's going to be a bit
painful catching and killing all the problems ... it might be better
just to patch the master kernel makefile to refuse to build with the
offending gcc versions.
James
From: Linus Torvalds <[email protected]>
Date: Wed, 31 Dec 2008 13:22:53 -0800 (PST)
> On Wed, 31 Dec 2008, Andrew Morton wrote:
> >
> > Adrian claimed that it was gcc-4.1.0 and 4.1.1 only. He proposed
> > banning them: http://lkml.org/lkml/2008/8/5/444
>
> If it really is just those releases, then yes, considering the number of
> cases we apparently have, and considering how ugly it is in some cases to
> move the weak function anywhere else, maybe banning those versions is the
> proper thing to do.
>
> It probably won't hurt very many people - yeah, some people will be forced
> to upgrade, but I have this memory of early 4.1 having had other bugs
> anyway, so it's probably a good idea.
I think this is probably the best way to handle this.
On Thursday 01 January 2009 00:43:56 Helge Deller wrote:
> Rusty Russell wrote:
> > Not quite what I had in mind... let me show you:
> > ...
> > Otherwise I'd have called it "arch_module_extra_size()".
>
> This would mean that I can increase the section size in the arch-specific function
> by returning a bigger value than sh_size.
> This would give me space at the end of the section, but not at the beginning
> (which is what I need), as sh_entsize (the offset into memory) will stay the
> same as before.
Ah, OK. I assumed that extra room at the end was sufficient.
So I've taken your previous patch and renamed the function to
arch_mod_section_prepend(). If we decide not to kill those bad gcc versions,
I'll stick it in some other random file (lib/weak.c? Blech...)
BTW, this is very late for this merge window. I would have liked to see
this a month ago :(
Subject: module: fix module loading failure of large kernel modules for parisc
Date: Wed, 31 Dec 2008 12:31:18 +0100
From: Helge Deller <[email protected]>
When creating the final layout of a kernel module in memory, allow the
module loader to reserve some additional memory in front of a given section.
This is currently only needed for the parisc port which needs to put the
stub entries there to fulfill the 17/22bit PCREL relocations with large
kernel modules like xfs.
Signed-off-by: Helge Deller <[email protected]>
Signed-off-by: Rusty Russell <[email protected]> (renamed fn)
diff --git a/include/linux/moduleloader.h b/include/linux/moduleloader.h
--- a/include/linux/moduleloader.h
+++ b/include/linux/moduleloader.h
@@ -12,6 +12,9 @@ int module_frob_arch_sections(Elf_Ehdr *
Elf_Shdr *sechdrs,
char *secstrings,
struct module *mod);
+
+/* Additional bytes needed by arch in front of individual sections */
+unsigned int arch_mod_section_prepend(struct module *mod, unsigned int section);
/* Allocator used for allocating struct module, core sections and init
sections. Returns NULL on failure. */
diff --git a/kernel/module.c b/kernel/module.c
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1588,11 +1588,21 @@ static int simplify_symbols(Elf_Shdr *se
return ret;
}
+/* Additional bytes needed by arch in front of individual sections */
+unsigned int __weak arch_mod_section_prepend(struct module *mod,
+ unsigned int section)
+{
+ /* default implementation just returns zero */
+ return 0;
+}
+
/* Update size with this section: return offset. */
-static long get_offset(unsigned int *size, Elf_Shdr *sechdr)
+static long get_offset(struct module *mod, unsigned int *size,
+ Elf_Shdr *sechdr, unsigned int section)
{
long ret;
+ *size += arch_mod_section_prepend(mod, section);
ret = ALIGN(*size, sechdr->sh_addralign ?: 1);
*size = ret + sechdr->sh_size;
return ret;
@@ -1632,7 +1642,7 @@ static void layout_sections(struct modul
|| strncmp(secstrings + s->sh_name,
".init", 5) == 0)
continue;
- s->sh_entsize = get_offset(&mod->core_size, s);
+ s->sh_entsize = get_offset(mod, &mod->core_size, s, i);
DEBUGP("\t%s\n", secstrings + s->sh_name);
}
if (m == 0)
@@ -1650,7 +1660,7 @@ static void layout_sections(struct modul
|| strncmp(secstrings + s->sh_name,
".init", 5) != 0)
continue;
- s->sh_entsize = (get_offset(&mod->init_size, s)
+ s->sh_entsize = (get_offset(mod, &mod->init_size, s, i)
| INIT_OFFSET_MASK);
DEBUGP("\t%s\n", secstrings + s->sh_name);
}
Linus Torvalds wrote:
> Some gcc versions will inline weak functions if they are in scope - even
> if there is a non-weak function somewhere else. So you MUST NOT have the
> weak definition in the same file (or indirectly called through some inline
> functions in a header file) as the call. Because if you do, then any user
> with the wrong version of gcc will get the weak function semantics, even
> if it was meant to be overridden by something else.
>
Yes. I think this behaviour is considered to be desperately broken by
the gcc developers and has been fixed in all recent gccs. There are a
couple of broken versions, and there have been patches floating around
to just refuse to use them; otherwise __weak is effectively unusable.
(Ah, I see Adrian has posted it again and everyone agrees with it.)
On the other hand, I have seen a couple of instances of "inline __weak"
which is insane, but I don't know if gcc does anything crazy with it, or
if its common enough to bother warning about.
J
Rusty Russell wrote:
> On Thursday 01 January 2009 00:43:56 Helge Deller wrote:
>> Rusty Russell wrote:
>>> Not quite what I had in mind... let me show you:
>>> ...
>>> Otherwise I'd have called it "arch_module_extra_size()".
>> This would mean that I can increase the section size in the arch-specific function
>> by returning a bigger value than sh_size.
>> This would give me space at the end of the section, but not at the beginning
>> (which is what I need), as sh_entsize (the offset into memory) will stay the
>> same as before.
>
> Ah, OK. I assumed that extra room at the end was sufficient.
>
> So I've taken your previous patch and renamed the function to
> arch_mod_section_prepend().
Thanks a lot !!!!
Patch below is ok and I'll adjust the parisc-specific patch
acordingly.
> If we decide not to kill those bad gcc versions,
> I'll stick it in some other random file (lib/weak.c? Blech...)
Andrew has pulled in the patch from Adrian in his -mm tree, so we should
be safe now.
> BTW, this is very late for this merge window. I would have liked to see
> this a month ago :(
Yes, sorry, I didn't had a patch available earlier.
I hope it's still in-time though, esp. as this will fix one of our longest
outstanding bugs on parisc and is quite important for us.
Thanks,
Helge
> Subject: module: fix module loading failure of large kernel modules for parisc
> Date: Wed, 31 Dec 2008 12:31:18 +0100
> From: Helge Deller <[email protected]>
>
> When creating the final layout of a kernel module in memory, allow the
> module loader to reserve some additional memory in front of a given section.
> This is currently only needed for the parisc port which needs to put the
> stub entries there to fulfill the 17/22bit PCREL relocations with large
> kernel modules like xfs.
>
> Signed-off-by: Helge Deller <[email protected]>
> Signed-off-by: Rusty Russell <[email protected]> (renamed fn)
>
> diff --git a/include/linux/moduleloader.h b/include/linux/moduleloader.h
> --- a/include/linux/moduleloader.h
> +++ b/include/linux/moduleloader.h
> @@ -12,6 +12,9 @@ int module_frob_arch_sections(Elf_Ehdr *
> Elf_Shdr *sechdrs,
> char *secstrings,
> struct module *mod);
> +
> +/* Additional bytes needed by arch in front of individual sections */
> +unsigned int arch_mod_section_prepend(struct module *mod, unsigned int section);
>
> /* Allocator used for allocating struct module, core sections and init
> sections. Returns NULL on failure. */
> diff --git a/kernel/module.c b/kernel/module.c
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -1588,11 +1588,21 @@ static int simplify_symbols(Elf_Shdr *se
> return ret;
> }
>
> +/* Additional bytes needed by arch in front of individual sections */
> +unsigned int __weak arch_mod_section_prepend(struct module *mod,
> + unsigned int section)
> +{
> + /* default implementation just returns zero */
> + return 0;
> +}
> +
> /* Update size with this section: return offset. */
> -static long get_offset(unsigned int *size, Elf_Shdr *sechdr)
> +static long get_offset(struct module *mod, unsigned int *size,
> + Elf_Shdr *sechdr, unsigned int section)
> {
> long ret;
>
> + *size += arch_mod_section_prepend(mod, section);
> ret = ALIGN(*size, sechdr->sh_addralign ?: 1);
> *size = ret + sechdr->sh_size;
> return ret;
> @@ -1632,7 +1642,7 @@ static void layout_sections(struct modul
> || strncmp(secstrings + s->sh_name,
> ".init", 5) == 0)
> continue;
> - s->sh_entsize = get_offset(&mod->core_size, s);
> + s->sh_entsize = get_offset(mod, &mod->core_size, s, i);
> DEBUGP("\t%s\n", secstrings + s->sh_name);
> }
> if (m == 0)
> @@ -1650,7 +1660,7 @@ static void layout_sections(struct modul
> || strncmp(secstrings + s->sh_name,
> ".init", 5) != 0)
> continue;
> - s->sh_entsize = (get_offset(&mod->init_size, s)
> + s->sh_entsize = (get_offset(mod, &mod->init_size, s, i)
> | INIT_OFFSET_MASK);
> DEBUGP("\t%s\n", secstrings + s->sh_name);
> }
>
* Linus Torvalds <[email protected]> wrote:
>
>
> On Wed, 31 Dec 2008, Andrew Morton wrote:
> >
> > Adrian claimed that it was gcc-4.1.0 and 4.1.1 only. He proposed
> > banning them: http://lkml.org/lkml/2008/8/5/444
>
> If it really is just those releases, then yes, considering the number of
> cases we apparently have, and considering how ugly it is in some cases
> to move the weak function anywhere else, maybe banning those versions is
> the proper thing to do.
>
> It probably won't hurt very many people - yeah, some people will be
> forced to upgrade, but I have this memory of early 4.1 having had other
> bugs anyway, so it's probably a good idea.
That would be _really_ nice to do IMHO: in many cases putting the __weak
definition into same-file scope with a call site is a natural approach. I
think that's how we ended up having so many instances of that bug. When
you use __weak as a 'default implementation' for some function, then it's
very natural to put it into the same file that also uses it.
It goes into separate, inactive scope only in a few special cases: such as
when it's some library function that can be overriden by the architecture.
But if it's some non-libray kernel code then the usage site is close to
the definition site.
If you look at most of the __weak fixes they IMO actually turned clean
code into less clean code: they detached some natural clustering of
definition and callsite.
And __weak is very elegant IMO, it can avoid a lot of #ifdefs and can be
used to self-document architecture interfaces - so it would be nice to
make it always work, regardless of the callsite's scope.
Ingo
On Thu, 1 Jan 2009 15:24:01 +0100 Ingo Molnar <[email protected]> wrote:
> * Linus Torvalds <[email protected]> wrote:
>
> >
> >
> > On Wed, 31 Dec 2008, Andrew Morton wrote:
> > >
> > > Adrian claimed that it was gcc-4.1.0 and 4.1.1 only. He proposed
> > > banning them: http://lkml.org/lkml/2008/8/5/444
> >
> > If it really is just those releases, then yes, considering the number of
> > cases we apparently have, and considering how ugly it is in some cases
> > to move the weak function anywhere else, maybe banning those versions is
> > the proper thing to do.
> >
> > It probably won't hurt very many people - yeah, some people will be
> > forced to upgrade, but I have this memory of early 4.1 having had other
> > bugs anyway, so it's probably a good idea.
>
> That would be _really_ nice to do IMHO
I wonder if we should do it in -stable too. Probably yes.
* Sam Ravnborg <[email protected]> wrote:
> > I guess I could make a sparse rule for this, but nobody seems to run
> > or care about sparse anyway. Sad.
>
> There is some increased janitorial effort recently. Try to grep for
> [Ss]+parse in the shortlog for the last couple of months.
in the x86 space there's a lot of Sparse activity as well: a number of
developers do systematic runs of Sparse and keep the tree Sparse-clean.
The moment this spreads to a critical mass of subsystems we can start
automating it. (as i was able to automate the checking of -Werr builds on
x86, with arbitrary random .config's)
Ingo
* David Miller <[email protected]> wrote:
> From: Linus Torvalds <[email protected]>
> Date: Wed, 31 Dec 2008 13:22:53 -0800 (PST)
>
> > On Wed, 31 Dec 2008, Andrew Morton wrote:
> > >
> > > Adrian claimed that it was gcc-4.1.0 and 4.1.1 only. He proposed
> > > banning them: http://lkml.org/lkml/2008/8/5/444
> >
> > If it really is just those releases, then yes, considering the number
> > of cases we apparently have, and considering how ugly it is in some
> > cases to move the weak function anywhere else, maybe banning those
> > versions is the proper thing to do.
> >
> > It probably won't hurt very many people - yeah, some people will be
> > forced to upgrade, but I have this memory of early 4.1 having had
> > other bugs anyway, so it's probably a good idea.
>
> I think this is probably the best way to handle this.
okay - to move this matter from the discussion-space to the
solution-space, how about the patch below? (tested on x86 with a
non-affected compiler version.)
Ingo
-------------->
>From e6346e5ab54dcf12888a79dfd5402f5de09b3fad Mon Sep 17 00:00:00 2001
From: Ingo Molnar <[email protected]>
Date: Fri, 2 Jan 2009 12:46:22 +0100
Subject: [PATCH] kbuild: Disallow GCC 4.1.0 / 4.1.1
Impact: fix crashes that can trigger if built with GCC 4.1.0 or 4.1.1
GCC 4.1.0 and 4.1.1 has a bug that can miscompile __weak symbols,
by inlining __weak functions into same-file call sites - breaking the
kernel if the __weak symbol is overriden later on.
In the past we tried to work around this problem by artificially
isolating call site and definition site - but these bugs tend to
pop up regularly:
43a2563: sparseirq: move __weak symbols into separate compilation unit
13a0c3c: sparseirq: work around compiler optimizing away __weak functions
And Linus has extended Sparse to report same-file callsites for __weak
symbols - which gave two dozen hits.
We have not found a clean way to work around this bug on the source
code level (noinline and explicit barrier()s are ignored by GCC), so we do
not allow these compilers (which are quite rare these days, have other bugs
and are superceded by the 4.1.2 bugfix release anyway).
Kernel builds under gcc 410 or 411 will now fail with this error message:
Sorry, your compiler is too old, too buggy or not recognized.
Signed-off-by: Ingo Molnar <[email protected]>
---
include/linux/compiler.h | 17 +++++++++++++++--
1 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index ea7c6be..dd558ce 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -36,12 +36,25 @@ extern void __chk_io_ptr(const volatile void __iomem *);
#ifdef __KERNEL__
-#if __GNUC__ >= 4
+/*
+ * GCC 4.1.0 and 4.1.1 has a bug that can miscompile __weak symbols,
+ * by inlining __weak functions into same-file call sites - breaking the
+ * kernel if the __weak symbol is overriden later on.
+ *
+ * We have not found a clean way to work around this bug on the source
+ * code level, so we do not allow these compilers (which are quite
+ * rare these days, have other bugs and are superceded by the 4.1.2
+ * bugfix release anyway):
+ */
+#define gcc41_inlining_bug \
+ (__GNUC__ == 4 && __GNUC_MINOR__ == 1 && __GNUC_PATCHLEVEL__ <= 1)
+
+#if __GNUC__ >= 4 && !gcc41_inlining_bug
# include <linux/compiler-gcc4.h>
#elif __GNUC__ == 3 && __GNUC_MINOR__ >= 2
# include <linux/compiler-gcc3.h>
#else
-# error Sorry, your compiler is too old/not recognized.
+# error Sorry, your compiler is too old, too buggy or not recognized.
#endif
#define notrace __attribute__((no_instrument_function))
On Friday 02 January 2009, Ingo Molnar wrote:
>
> * David Miller <[email protected]> wrote:
>
> > From: Linus Torvalds <[email protected]>
> > Date: Wed, 31 Dec 2008 13:22:53 -0800 (PST)
> >
> > > On Wed, 31 Dec 2008, Andrew Morton wrote:
> > > >
> > > > Adrian claimed that it was gcc-4.1.0 and 4.1.1 only. He proposed
> > > > banning them: http://lkml.org/lkml/2008/8/5/444
> > >
> > > If it really is just those releases, then yes, considering the number
> > > of cases we apparently have, and considering how ugly it is in some
> > > cases to move the weak function anywhere else, maybe banning those
> > > versions is the proper thing to do.
> > >
> > > It probably won't hurt very many people - yeah, some people will be
> > > forced to upgrade, but I have this memory of early 4.1 having had
> > > other bugs anyway, so it's probably a good idea.
> >
> > I think this is probably the best way to handle this.
>
> okay - to move this matter from the discussion-space to the
> solution-space, how about the patch below? (tested on x86 with a
> non-affected compiler version.)
...or we can just merge Adrian's patch from June 2008 which also fixes
the issue nicely.
OTOH your patch has an advantage of addressing the problem in the more
appropriate place (include/linux/compiler.h) and from what I see allows
us to remove previous gcc 4.1.0 check from init/main.c?
> Ingo
>
> -------------->
> From e6346e5ab54dcf12888a79dfd5402f5de09b3fad Mon Sep 17 00:00:00 2001
> From: Ingo Molnar <[email protected]>
> Date: Fri, 2 Jan 2009 12:46:22 +0100
> Subject: [PATCH] kbuild: Disallow GCC 4.1.0 / 4.1.1
>
> Impact: fix crashes that can trigger if built with GCC 4.1.0 or 4.1.1
>
> GCC 4.1.0 and 4.1.1 has a bug that can miscompile __weak symbols,
> by inlining __weak functions into same-file call sites - breaking the
> kernel if the __weak symbol is overriden later on.
>
> In the past we tried to work around this problem by artificially
> isolating call site and definition site - but these bugs tend to
> pop up regularly:
>
> 43a2563: sparseirq: move __weak symbols into separate compilation unit
> 13a0c3c: sparseirq: work around compiler optimizing away __weak functions
>
> And Linus has extended Sparse to report same-file callsites for __weak
> symbols - which gave two dozen hits.
>
> We have not found a clean way to work around this bug on the source
> code level (noinline and explicit barrier()s are ignored by GCC), so we do
> not allow these compilers (which are quite rare these days, have other bugs
> and are superceded by the 4.1.2 bugfix release anyway).
>
> Kernel builds under gcc 410 or 411 will now fail with this error message:
>
> Sorry, your compiler is too old, too buggy or not recognized.
>
> Signed-off-by: Ingo Molnar <[email protected]>
> ---
> include/linux/compiler.h | 17 +++++++++++++++--
> 1 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index ea7c6be..dd558ce 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -36,12 +36,25 @@ extern void __chk_io_ptr(const volatile void __iomem *);
>
> #ifdef __KERNEL__
>
> -#if __GNUC__ >= 4
> +/*
> + * GCC 4.1.0 and 4.1.1 has a bug that can miscompile __weak symbols,
> + * by inlining __weak functions into same-file call sites - breaking the
> + * kernel if the __weak symbol is overriden later on.
> + *
> + * We have not found a clean way to work around this bug on the source
> + * code level, so we do not allow these compilers (which are quite
> + * rare these days, have other bugs and are superceded by the 4.1.2
> + * bugfix release anyway):
> + */
> +#define gcc41_inlining_bug \
> + (__GNUC__ == 4 && __GNUC_MINOR__ == 1 && __GNUC_PATCHLEVEL__ <= 1)
> +
> +#if __GNUC__ >= 4 && !gcc41_inlining_bug
> # include <linux/compiler-gcc4.h>
> #elif __GNUC__ == 3 && __GNUC_MINOR__ >= 2
> # include <linux/compiler-gcc3.h>
> #else
> -# error Sorry, your compiler is too old/not recognized.
> +# error Sorry, your compiler is too old, too buggy or not recognized.
> #endif
>
> #define notrace __attribute__((no_instrument_function))
* Bartlomiej Zolnierkiewicz <[email protected]> wrote:
> On Friday 02 January 2009, Ingo Molnar wrote:
> >
> > * David Miller <[email protected]> wrote:
> >
> > > From: Linus Torvalds <[email protected]>
> > > Date: Wed, 31 Dec 2008 13:22:53 -0800 (PST)
> > >
> > > > On Wed, 31 Dec 2008, Andrew Morton wrote:
> > > > >
> > > > > Adrian claimed that it was gcc-4.1.0 and 4.1.1 only. He proposed
> > > > > banning them: http://lkml.org/lkml/2008/8/5/444
> > > >
> > > > If it really is just those releases, then yes, considering the number
> > > > of cases we apparently have, and considering how ugly it is in some
> > > > cases to move the weak function anywhere else, maybe banning those
> > > > versions is the proper thing to do.
> > > >
> > > > It probably won't hurt very many people - yeah, some people will be
> > > > forced to upgrade, but I have this memory of early 4.1 having had
> > > > other bugs anyway, so it's probably a good idea.
> > >
> > > I think this is probably the best way to handle this.
> >
> > okay - to move this matter from the discussion-space to the
> > solution-space, how about the patch below? (tested on x86 with a
> > non-affected compiler version.)
>
> ...or we can just merge Adrian's patch from June 2008 which also fixes
> the issue nicely.
didnt know about that patch, but yeah, sure.
> OTOH your patch has an advantage of addressing the problem in the more
> appropriate place (include/linux/compiler.h) and from what I see allows
> us to remove previous gcc 4.1.0 check from init/main.c?
Good spotting - find followup cleanup patch below.
Ingo
------------------->
>From d23cbaaa342e5555a919a543095d656415a55950 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <[email protected]>
Date: Fri, 2 Jan 2009 16:16:16 +0100
Subject: [PATCH] kbuild: Remove gcc 4.1.0 quirk from init/main.c
Impact: cleanup
We now have a cleaner check for gcc 4.1.0/4.1.1 trouble in
include/linux/compiler.h, so remove the 4.1.0 quirk from
init/main.c.
Reported-by: Bartlomiej Zolnierkiewicz <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
init/main.c | 9 ---------
1 files changed, 0 insertions(+), 9 deletions(-)
diff --git a/init/main.c b/init/main.c
index 2a7ce0f..5ced153 100644
--- a/init/main.c
+++ b/init/main.c
@@ -75,15 +75,6 @@
#include <asm/smp.h>
#endif
-/*
- * This is one of the first .c files built. Error out early if we have compiler
- * trouble.
- */
-
-#if __GNUC__ == 4 && __GNUC_MINOR__ == 1 && __GNUC_PATCHLEVEL__ == 0
-#warning gcc-4.1.0 is known to miscompile the kernel. A different compiler version is recommended.
-#endif
-
static int kernel_init(void *);
extern void init_IRQ(void);
On Fri, 2 Jan 2009, Ingo Molnar wrote:
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -36,12 +36,25 @@ extern void __chk_io_ptr(const volatile void __iomem *);
>
> #ifdef __KERNEL__
>
> -#if __GNUC__ >= 4
> +/*
> + * GCC 4.1.0 and 4.1.1 has a bug that can miscompile __weak symbols,
> + * by inlining __weak functions into same-file call sites - breaking the
> + * kernel if the __weak symbol is overriden later on.
> + *
> + * We have not found a clean way to work around this bug on the source
> + * code level, so we do not allow these compilers (which are quite
> + * rare these days, have other bugs and are superceded by the 4.1.2
> + * bugfix release anyway):
> + */
> +#define gcc41_inlining_bug \
> + (__GNUC__ == 4 && __GNUC_MINOR__ == 1 && __GNUC_PATCHLEVEL__ <= 1)
> +
> +#if __GNUC__ >= 4 && !gcc41_inlining_bug
> # include <linux/compiler-gcc4.h>
I think this is wrong.
Just move the check into <linux/compiler-gcc4.h>
It makes no sense to do stuff that is specific to gcc4 in the general gcc
header file. It seems you did this just in order to re-use a (bad) generic
error case.
Linus
On Fri, 2 Jan 2009, Linus Torvalds wrote:
>
> I think this is wrong.
>
> Just move the check into <linux/compiler-gcc4.h>
In fact, looking at that whole mess, I redid it all. It was disgusting how
conditionals in gcc4.h needed to double-check that __GNUC__ really was 4
(rather than something bigger), which largely negated the whole nice clean
compiler version separation.
I pushed out my preferred version, which fixes up the whole thing. The
gcc4 header file now only gets included for __GNUC__ == 4, and when we
ever see a __GNUC__ of 5, it will automatically DTRT and try to include
<linux/compiler-gcc5.h> instead of #4.
And then the check in gcc4.h for 4.1.0 and 4.1.1 is much simplified. The
two patches I pushed out add functionality, but don't actually add any new
lines (the first simplification patch removes more lines than it adds, and
the second one that adds the __weak bug test adds as mahy lines as the
cleanup removed).
And it all looks more logical too, imho.
Linus
* Linus Torvalds <[email protected]> wrote:
>
>
> On Fri, 2 Jan 2009, Ingo Molnar wrote:
> > --- a/include/linux/compiler.h
> > +++ b/include/linux/compiler.h
> > @@ -36,12 +36,25 @@ extern void __chk_io_ptr(const volatile void __iomem *);
> >
> > #ifdef __KERNEL__
> >
> > -#if __GNUC__ >= 4
> > +/*
> > + * GCC 4.1.0 and 4.1.1 has a bug that can miscompile __weak symbols,
> > + * by inlining __weak functions into same-file call sites - breaking the
> > + * kernel if the __weak symbol is overriden later on.
> > + *
> > + * We have not found a clean way to work around this bug on the source
> > + * code level, so we do not allow these compilers (which are quite
> > + * rare these days, have other bugs and are superceded by the 4.1.2
> > + * bugfix release anyway):
> > + */
> > +#define gcc41_inlining_bug \
> > + (__GNUC__ == 4 && __GNUC_MINOR__ == 1 && __GNUC_PATCHLEVEL__ <= 1)
> > +
> > +#if __GNUC__ >= 4 && !gcc41_inlining_bug
> > # include <linux/compiler-gcc4.h>
>
> I think this is wrong.
>
> Just move the check into <linux/compiler-gcc4.h>
>
> It makes no sense to do stuff that is specific to gcc4 in the general
> gcc header file. It seems you did this just in order to re-use a (bad)
> generic error case.
yeah. I first hacked the generic check then saw how ugly the end result
was and moved it one level higher. Which was less ugly than where it came
from and not much worse than the starting point (so it passed my filters)
but still not clean enough (so it didnt pass your filters).
How about the patch below instead? It cleans up the generic check by
splitting all the per-major-version checks out into gcc4 and gcc3.
(still untested)
Ingo
------------->
>From bb951ce0794f0e5974b834eb14e974a0a2c119db Mon Sep 17 00:00:00 2001
From: Ingo Molnar <[email protected]>
Date: Fri, 2 Jan 2009 12:46:22 +0100
Subject: [PATCH] kbuild: Disallow GCC 4.1.0 / 4.1.1
Impact: fix crashes that can trigger if built with GCC 4.1.0 or 4.1.1
GCC 4.1.0 and 4.1.1 has a bug that can miscompile __weak symbols,
by inlining __weak functions into same-file call sites - breaking the
kernel if the __weak symbol is overriden later on.
In the past we tried to work around this problem by artificially
isolating call site and definition site - but these bugs tend to
pop up regularly:
43a2563: sparseirq: move __weak symbols into separate compilation unit
13a0c3c: sparseirq: work around compiler optimizing away __weak functions
And Linus has extended Sparse to report same-file callsites for __weak
symbols - which gave two dozen hits.
We have not found a clean way to work around this bug on the source
code level (noinline and explicit barrier()s are ignored by GCC), so we do
not allow these compilers (which are quite rare these days, have other bugs
and are superceded by the 4.1.2 bugfix release anyway).
Kernel builds under gcc 410 or 411 will now fail with this error message:
Sorry, GCC 4.1.0/4.1.1 are too buggy to build the kernel - please
upgrade to 4.1.2 or later versions.
Signed-off-by: Ingo Molnar <[email protected]>
---
include/linux/compiler-gcc3.h | 4 ++++
include/linux/compiler-gcc4.h | 14 ++++++++++++++
include/linux/compiler.h | 4 ++--
3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/include/linux/compiler-gcc3.h b/include/linux/compiler-gcc3.h
index e5eb795..a929a6d 100644
--- a/include/linux/compiler-gcc3.h
+++ b/include/linux/compiler-gcc3.h
@@ -2,6 +2,10 @@
#error "Please don't include <linux/compiler-gcc3.h> directly, include <linux/compiler.h> instead."
#endif
+#if __GNUC_MINOR__ < 2
+# error Sorry, your compiler is too old - please upgrade it.
+#endif
+
/* These definitions are for GCC v3.x. */
#include <linux/compiler-gcc.h>
diff --git a/include/linux/compiler-gcc4.h b/include/linux/compiler-gcc4.h
index 974f5b7..b1edc9d 100644
--- a/include/linux/compiler-gcc4.h
+++ b/include/linux/compiler-gcc4.h
@@ -2,6 +2,20 @@
#error "Please don't include <linux/compiler-gcc4.h> directly, include <linux/compiler.h> instead."
#endif
+/*
+ * GCC 4.1.0 and 4.1.1 has a bug that can miscompile __weak symbols,
+ * by inlining __weak functions into same-file call sites - breaking the
+ * kernel if the __weak symbol is overriden later on.
+ *
+ * We have not found a clean way to work around this bug on the source
+ * code level, so we do not allow these compilers (which are quite
+ * rare these days, have other bugs and are superceded by the 4.1.2
+ * bugfix release anyway):
+ */
+#if __GNUC__ == 4 && __GNUC_MINOR__ == 1 && __GNUC_PATCHLEVEL__ <= 1
+# error Sorry, GCC 4.1.0/4.1.1 are too buggy to build the kernel - please upgrade to 4.1.2 or later versions.
+#endif
+
/* These definitions are for GCC v4.x. */
#include <linux/compiler-gcc.h>
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index ea7c6be..18edc7a 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -38,10 +38,10 @@ extern void __chk_io_ptr(const volatile void __iomem *);
#if __GNUC__ >= 4
# include <linux/compiler-gcc4.h>
-#elif __GNUC__ == 3 && __GNUC_MINOR__ >= 2
+#elif __GNUC__ == 3
# include <linux/compiler-gcc3.h>
#else
-# error Sorry, your compiler is too old/not recognized.
+# error Sorry, your compiler is too old or not recognized.
#endif
#define notrace __attribute__((no_instrument_function))
* Linus Torvalds <[email protected]> wrote:
>
>
> On Fri, 2 Jan 2009, Linus Torvalds wrote:
> >
> > I think this is wrong.
> >
> > Just move the check into <linux/compiler-gcc4.h>
>
> In fact, looking at that whole mess, I redid it all. It was disgusting
> how conditionals in gcc4.h needed to double-check that __GNUC__ really
> was 4 (rather than something bigger), which largely negated the whole
> nice clean compiler version separation.
>
> I pushed out my preferred version, which fixes up the whole thing. The
> gcc4 header file now only gets included for __GNUC__ == 4, and when we
> ever see a __GNUC__ of 5, it will automatically DTRT and try to include
> <linux/compiler-gcc5.h> instead of #4.
>
> And then the check in gcc4.h for 4.1.0 and 4.1.1 is much simplified.
> The two patches I pushed out add functionality, but don't actually add
> any new lines (the first simplification patch removes more lines than it
> adds, and the second one that adds the __weak bug test adds as mahy
> lines as the cleanup removed).
>
> And it all looks more logical too, imho.
yeah, agreed, much cleaner.
The gcc3 check for gcc 3.0 and 3.1 can be pushed into gcc3 as well - not
sure whether you fixed that. (i fixed it in the patch i just sent - but i
didnt notice the gcc5 mess in gcc4.h that you fixed)
Ingo
* Ingo Molnar <[email protected]> wrote:
> > And it all looks more logical too, imho.
>
> yeah, agreed, much cleaner.
>
> The gcc3 check for gcc 3.0 and 3.1 can be pushed into gcc3 as well - not
> sure whether you fixed that. (i fixed it in the patch i just sent - but
> i didnt notice the gcc5 mess in gcc4.h that you fixed)
ok, i see them now:
f9d1425: Disallow gcc versions 4.1.{0,1}
f153b82: Sanitize gcc version header includes
agreed, much cleaner.
One small issue - i think this now allows gcc 3.0 and 3.1 again - which we
didnt before. Dont we need the patch below - am i missing something?
Ingo
------------------->
>From 1458f25412a838968e845ec0bc1b18db70cba7cd Mon Sep 17 00:00:00 2001
From: Ingo Molnar <[email protected]>
Date: Fri, 2 Jan 2009 18:53:14 +0100
Subject: [PATCH] Disallow gcc versions 3.{0,1}
GCC 3.0 and 3.1 are too old to build a working kernel.
Signed-off-by: Ingo Molnar <[email protected]>
---
include/linux/compiler-gcc3.h | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/include/linux/compiler-gcc3.h b/include/linux/compiler-gcc3.h
index 2befe65..8005eff 100644
--- a/include/linux/compiler-gcc3.h
+++ b/include/linux/compiler-gcc3.h
@@ -2,6 +2,10 @@
#error "Please don't include <linux/compiler-gcc3.h> directly, include <linux/compiler.h> instead."
#endif
+#if __GNUC_MINOR__ < 2
+# error Sorry, your compiler is too old - please upgrade it.
+#endif
+
#if __GNUC_MINOR__ >= 3
# define __used __attribute__((__used__))
#else
On Fri, Jan 02, 2009 at 09:38:14AM -0800, Linus Torvalds wrote:
>
>
> On Fri, 2 Jan 2009, Linus Torvalds wrote:
> >
> > I think this is wrong.
> >
> > Just move the check into <linux/compiler-gcc4.h>
>
> In fact, looking at that whole mess, I redid it all. It was disgusting how
> conditionals in gcc4.h needed to double-check that __GNUC__ really was 4
> (rather than something bigger), which largely negated the whole nice clean
> compiler version separation.
>
> I pushed out my preferred version, which fixes up the whole thing. The
> gcc4 header file now only gets included for __GNUC__ == 4, and when we
> ever see a __GNUC__ of 5, it will automatically DTRT and try to include
> <linux/compiler-gcc5.h> instead of #4.
>
> And then the check in gcc4.h for 4.1.0 and 4.1.1 is much simplified. The
> two patches I pushed out add functionality, but don't actually add any new
> lines (the first simplification patch removes more lines than it adds, and
> the second one that adds the __weak bug test adds as mahy lines as the
> cleanup removed).
>
> And it all looks more logical too, imho.
Bugger....
Now I cannot do cross compile for: alpha, arm, m68k and sparc.
Not that I actually try to run these beasts but just being able
to do cross compile has served me well.
Last I looked (only few days ago) crosstool-ng did not support sparc :-(
Sam
On Fri, 2 Jan 2009, Ingo Molnar wrote:
>
> The gcc3 check for gcc 3.0 and 3.1 can be pushed into gcc3 as well - not
> sure whether you fixed that. (i fixed it in the patch i just sent - but i
> didnt notice the gcc5 mess in gcc4.h that you fixed)
I just dropped it. Gcc 3.0 and 3.1 are so old that we simply don't care.
Nobody who has compiled a kernel in the last two years can possibly have
those versions: we've failed the build on them since December 2006. And
there obviously won't be any new users either. So consider it a
generational garbage collection event.
Of course, if we really want to check for old compiler versions, we can
add the check back (to the gcc3 header file where it belongs), but it does
seem to be entirely historical.
Linus
* Linus Torvalds <[email protected]> wrote:
>
>
> On Fri, 2 Jan 2009, Ingo Molnar wrote:
> >
> > The gcc3 check for gcc 3.0 and 3.1 can be pushed into gcc3 as well - not
> > sure whether you fixed that. (i fixed it in the patch i just sent - but i
> > didnt notice the gcc5 mess in gcc4.h that you fixed)
>
> I just dropped it. Gcc 3.0 and 3.1 are so old that we simply don't care.
we didnt support it for quite some time. If you look at that ugly version
check you replaced:
#if __GNUC__ >= 4 && !gcc41_inlining_bug
# include <linux/compiler-gcc4.h>
#elif __GNUC__ == 3 && __GNUC_MINOR__ >= 2
# include <linux/compiler-gcc3.h>
#else
# error Sorry, your compiler is too old, too buggy or not recognized.
#endif
it has a hidden 'gcc 3.0 / 3.1 is not supported' condition in it. So what
i tried to point out that your patch causes a regression here - we dont
filter out gcc 3.0/3.1 out anymore.
Ingo
> From d23cbaaa342e5555a919a543095d656415a55950 Mon Sep 17 00:00:00 2001
> From: Ingo Molnar <[email protected]>
> Date: Fri, 2 Jan 2009 16:16:16 +0100
> Subject: [PATCH] kbuild: Remove gcc 4.1.0 quirk from init/main.c
>
> Impact: cleanup
>
> We now have a cleaner check for gcc 4.1.0/4.1.1 trouble in
> include/linux/compiler.h, so remove the 4.1.0 quirk from
> init/main.c.
>
> Reported-by: Bartlomiej Zolnierkiewicz <[email protected]>
> Signed-off-by: Ingo Molnar <[email protected]>
This should be applied despite Linus' different approach in compiler.h.
Acked-by: Sam Ravnborg <[email protected]>
> ---
> init/main.c | 9 ---------
> 1 files changed, 0 insertions(+), 9 deletions(-)
>
> diff --git a/init/main.c b/init/main.c
> index 2a7ce0f..5ced153 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -75,15 +75,6 @@
> #include <asm/smp.h>
> #endif
>
> -/*
> - * This is one of the first .c files built. Error out early if we have compiler
> - * trouble.
> - */
> -
> -#if __GNUC__ == 4 && __GNUC_MINOR__ == 1 && __GNUC_PATCHLEVEL__ == 0
> -#warning gcc-4.1.0 is known to miscompile the kernel. A different compiler version is recommended.
> -#endif
> -
> static int kernel_init(void *);
>
> extern void init_IRQ(void);
On Fri, 2 Jan 2009, Sam Ravnborg wrote:
>
> Bugger....
> Now I cannot do cross compile for: alpha, arm, m68k and sparc.
>
> Not that I actually try to run these beasts but just being able
> to do cross compile has served me well.
We _could_ make a "CONFIG_COMPILE_ONLY" check, but wouldn't it be even
nicer to make sure the cross-compiles are something that might actually be
expected to work?
I realize that cross-tools tend to lag a bit - the pressure to maintain
them tends to be much lower - but I was sure we had somebody who did a
reasonable cross-compiler toolchain. Is gcc-4.1 really the most modern
thing that is easily available?
Linus
On Fri, 2 Jan 2009, Ingo Molnar wrote:
>
> So what i tried to point out that your patch causes a regression here -
> we dont filter out gcc 3.0/3.1 out anymore.
What I tried to tell you was that we don't care - nobody can have it
anyway. If somebody tried to compile the kernel with it, we've already
refused for the last 2+ years. So there are no old users out there. And
there certainly aren't any new ones.
Linus
On Fri, 2 Jan 2009, Linus Torvalds wrote:
>
> What I tried to tell you was that we don't care - nobody can have it
> anyway. If somebody tried to compile the kernel with it, we've already
> refused for the last 2+ years. So there are no old users out there. And
> there certainly aren't any new ones.
.. but we cetrtainly can add the check back easily enough if you really
want to, of course. I just don't think there is much point.
Linus
* Sam Ravnborg <[email protected]> wrote:
> > And it all looks more logical too, imho.
>
> Bugger....
> Now I cannot do cross compile for: alpha, arm, m68k and sparc.
hm, i just did a successful cross-build from x86 to alpha:
KSYM .tmp_kallsyms2.S
AS .tmp_kallsyms2.o
LD vmlinux
SYSMAP System.map
SYSMAP .tmp_System.map
phoenix:~/linux/linux> head -6 /dev/shm/tip/build/.config
#
# Automatically generated make config: don't edit
# Linux kernel version: 2.6.28
# Fri Jan 2 19:28:14 2009
#
CONFIG_ALPHA=y
with these commits present:
f9d1425: Disallow gcc versions 4.1.{0,1}
f153b82: Sanitize gcc version header includes
what type of cross-build breakage do they cause?
Ingo
On Fri, Jan 02, 2009 at 10:04:27AM -0800, Linus Torvalds wrote:
>
>
> On Fri, 2 Jan 2009, Sam Ravnborg wrote:
> >
> > Bugger....
> > Now I cannot do cross compile for: alpha, arm, m68k and sparc.
> >
> > Not that I actually try to run these beasts but just being able
> > to do cross compile has served me well.
>
> We _could_ make a "CONFIG_COMPILE_ONLY" check, but wouldn't it be even
> nicer to make sure the cross-compiles are something that might actually be
> expected to work?
>
> I realize that cross-tools tend to lag a bit - the pressure to maintain
> them tends to be much lower - but I was sure we had somebody who did a
> reasonable cross-compiler toolchain. Is gcc-4.1 really the most modern
> thing that is easily available?
Asked google and it found following page:
http://www.kernel.org/pub/tools/crosstool/
Architecuter OK? gcc version
---------------------------------
alpha No gcc 4.0.2
arm Yes gcc 3.4.5
ia64 Yes gcc 3.4.5
m68k Yes gcc 3.4.5
mips Yes gcc 3.4.5
sh4 Yes gcc 3.4.5
sparc Yes gcc 3.4.5
sparc64 Yes gcc 3.4.5
x86_64 Yes gcc 3.4.5
So from this list of tool chains we can continue to do cross builds
of all except alpha.
But the gcc version is getting ancient..
Why it shall be so hard to do cross build toolchains is above my
imagination but then I also never looked at what it involes.
Added Vegard that maintain these pages.
Sam
On Fri, Jan 02, 2009 at 07:22:48PM +0100, Ingo Molnar wrote:
>
> * Sam Ravnborg <[email protected]> wrote:
>
> > > And it all looks more logical too, imho.
> >
> > Bugger....
> > Now I cannot do cross compile for: alpha, arm, m68k and sparc.
>
> hm, i just did a successful cross-build from x86 to alpha:
>
> KSYM .tmp_kallsyms2.S
> AS .tmp_kallsyms2.o
> LD vmlinux
> SYSMAP System.map
> SYSMAP .tmp_System.map
>
> phoenix:~/linux/linux> head -6 /dev/shm/tip/build/.config
> #
> # Automatically generated make config: don't edit
> # Linux kernel version: 2.6.28
> # Fri Jan 2 19:28:14 2009
> #
> CONFIG_ALPHA=y
>
> with these commits present:
>
> f9d1425: Disallow gcc versions 4.1.{0,1}
> f153b82: Sanitize gcc version header includes
>
> what type of cross-build breakage do they cause?
The default gcc made by demo-alpha with crosstool is version 4.1.0:
/opt/crosstool/gcc-4.1.0-glibc-2.3.6/alpha-unknown-linux-gnu/bin/alpha-unknown-linux-gnu-gcc --version
alpha-unknown-linux-gnu-gcc (GCC) 4.1.0
Copyright (C) 2006 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Sam
Sam Ravnborg wrote:
> On Fri, Jan 02, 2009 at 10:04:27AM -0800, Linus Torvalds wrote:
>>
>> On Fri, 2 Jan 2009, Sam Ravnborg wrote:
>>> Bugger....
>>> Now I cannot do cross compile for: alpha, arm, m68k and sparc.
>>>
>>> Not that I actually try to run these beasts but just being able
>>> to do cross compile has served me well.
>> We _could_ make a "CONFIG_COMPILE_ONLY" check, but wouldn't it be even
>> nicer to make sure the cross-compiles are something that might actually be
>> expected to work?
>>
>> I realize that cross-tools tend to lag a bit - the pressure to maintain
>> them tends to be much lower - but I was sure we had somebody who did a
>> reasonable cross-compiler toolchain. Is gcc-4.1 really the most modern
>> thing that is easily available?
>
> Asked google and it found following page:
>
> http://www.kernel.org/pub/tools/crosstool/
>
> Architecuter OK? gcc version
> ---------------------------------
> alpha No gcc 4.0.2
> arm Yes gcc 3.4.5
> ia64 Yes gcc 3.4.5
> m68k Yes gcc 3.4.5
> mips Yes gcc 3.4.5
> sh4 Yes gcc 3.4.5
> sparc Yes gcc 3.4.5
> sparc64 Yes gcc 3.4.5
> x86_64 Yes gcc 3.4.5
>
>
> So from this list of tool chains we can continue to do cross builds
> of all except alpha.
> But the gcc version is getting ancient..
>
> Why it shall be so hard to do cross build toolchains is above my
> imagination but then I also never looked at what it involes.
>
> Added Vegard that maintain these pages.
I doubt that these are any more recent, but the filenames don't say:
http://userweb.kernel.org/~akpm/cross-compilers/
Yes, it is an ongoing problem.
--
~Randy
* Sam Ravnborg <[email protected]> wrote:
> On Fri, Jan 02, 2009 at 07:22:48PM +0100, Ingo Molnar wrote:
> >
> > * Sam Ravnborg <[email protected]> wrote:
> >
> > > > And it all looks more logical too, imho.
> > >
> > > Bugger....
> > > Now I cannot do cross compile for: alpha, arm, m68k and sparc.
> >
> > hm, i just did a successful cross-build from x86 to alpha:
> >
> > KSYM .tmp_kallsyms2.S
> > AS .tmp_kallsyms2.o
> > LD vmlinux
> > SYSMAP System.map
> > SYSMAP .tmp_System.map
> >
> > phoenix:~/linux/linux> head -6 /dev/shm/tip/build/.config
> > #
> > # Automatically generated make config: don't edit
> > # Linux kernel version: 2.6.28
> > # Fri Jan 2 19:28:14 2009
> > #
> > CONFIG_ALPHA=y
> >
> > with these commits present:
> >
> > f9d1425: Disallow gcc versions 4.1.{0,1}
> > f153b82: Sanitize gcc version header includes
> >
> > what type of cross-build breakage do they cause?
>
> The default gcc made by demo-alpha with crosstool is version 4.1.0:
>
> /opt/crosstool/gcc-4.1.0-glibc-2.3.6/alpha-unknown-linux-gnu/bin/alpha-unknown-linux-gnu-gcc --version
> alpha-unknown-linux-gnu-gcc (GCC) 4.1.0
> Copyright (C) 2006 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions. There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
ah, ok. I build cross-compilers from scratch, so i have 4.3.3-pre:
phoenix:~/linux/linux> /opt/crossgcc/cross/bin/alpha-linux-gcc -v
[...]
gcc version 4.3.3 20081123 (prerelease) (GCC)
Ingo
On Fri, Jan 02, 2009 at 10:04:27AM -0800, Linus Torvalds wrote:
>
>
> On Fri, 2 Jan 2009, Sam Ravnborg wrote:
> >
> > Bugger....
> > Now I cannot do cross compile for: alpha, arm, m68k and sparc.
> >
> > Not that I actually try to run these beasts but just being able
> > to do cross compile has served me well.
>
> We _could_ make a "CONFIG_COMPILE_ONLY" check, but wouldn't it be even
> nicer to make sure the cross-compiles are something that might actually be
> expected to work?
>
> I realize that cross-tools tend to lag a bit - the pressure to maintain
> them tends to be much lower - but I was sure we had somebody who did a
> reasonable cross-compiler toolchain. Is gcc-4.1 really the most modern
> thing that is easily available?
FWIW, I'm using 4.3 on all targets at the moment. See
git://git.kernel.org/pub/scm/linux/kernel/git/viro/toolchain.git/
for fedora-based variant of that sucker. And yes, it does include
cross-to-sparc; all but sh/sh64, in fact (sh had serious compiler
breakage around 4.3.0 and backporting from -HEAD was far beyond
what I considered reasonable at that point).
* Ingo Molnar wrote:
> * Sam Ravnborg <[email protected]> wrote:
> > > what type of cross-build breakage do they cause?
> >
> > The default gcc made by demo-alpha with crosstool is version 4.1.0:
> >
> > /opt/crosstool/gcc-4.1.0-glibc-2.3.6/alpha-unknown-linux-gnu/bin/alpha-unknown-linux-gnu-gcc --version
> > alpha-unknown-linux-gnu-gcc (GCC) 4.1.0
> > Copyright (C) 2006 Free Software Foundation, Inc.
> > This is free software; see the source for copying conditions. There is NO
> > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>
> ah, ok. I build cross-compilers from scratch, so i have 4.3.3-pre:
>
> phoenix:~/linux/linux> /opt/crossgcc/cross/bin/alpha-linux-gcc -v
> [...]
> gcc version 4.3.3 20081123 (prerelease) (GCC)
>
> Ingo
I forwarded the thread to Dan Kegel to make him aware, that the used gcc
version in crosstools makes problems.
--
Bye bye ... Detlef
Al Viro <[email protected]> writes:
>
> FWIW, I'm using 4.3 on all targets at the moment. See
> git://git.kernel.org/pub/scm/linux/kernel/git/viro/toolchain.git/
> for fedora-based variant of that sucker. And yes, it does include
> cross-to-sparc; all but sh/sh64, in fact (sh had serious compiler
> breakage around 4.3.0 and backporting from -HEAD was far beyond
> what I considered reasonable at that point).
The full opensuse distribution also has a couple of spec files for
generating cross compilers from recent versions. These are for
icecream, but can be relatively easily adapted (or the binaries
reused)
In general I don't think building cross compilers is as hard
as it used to be, so it can be reasonably done without any support
scripts too.
Really no excuse to still use the old crosstool crap :)
-Andi
--
[email protected]
On Fri, Jan 02, 2009 at 10:05:47AM -0800, Linus Torvalds wrote:
>
>
> On Fri, 2 Jan 2009, Ingo Molnar wrote:
> >
> > So what i tried to point out that your patch causes a regression here -
> > we dont filter out gcc 3.0/3.1 out anymore.
>
> What I tried to tell you was that we don't care - nobody can have it
> anyway. If somebody tried to compile the kernel with it, we've already
> refused for the last 2+ years. So there are no old users out there. And
> there certainly aren't any new ones.
Yes some people (including me) still have them and are happy that there
is a check to prevent accidental builds with an inappropriate compiler.
Willy
On Fri, 2 Jan 2009, Willy Tarreau wrote:
>
> Yes some people (including me) still have them and are happy that there
> is a check to prevent accidental builds with an inappropriate compiler.
Heh, ok. If people really do actively have that version, let's add the
check back in.
Linus
>
> Bugger....
> Now I cannot do cross compile for: alpha, arm, m68k and sparc.
>
> Not that I actually try to run these beasts but just being able
> to do cross compile has served me well.
>
> Last I looked (only few days ago) crosstool-ng did not support sparc :-(
There are also various variants of 4.1.1 around with the fix for
that inline bug backported (I think that's the case of the compiler
that ships with the Cell SDK for example).
What a f. mess :-(
Cheers,
Ben.
On Sat, Jan 03, 2009 at 09:27:33AM +1100, Benjamin Herrenschmidt wrote:
>
> >
> > Bugger....
> > Now I cannot do cross compile for: alpha, arm, m68k and sparc.
> >
> > Not that I actually try to run these beasts but just being able
> > to do cross compile has served me well.
> >
> > Last I looked (only few days ago) crosstool-ng did not support sparc :-(
>
> There are also various variants of 4.1.1 around with the fix for
> that inline bug backported (I think that's the case of the compiler
> that ships with the Cell SDK for example).
>
We could add a feature check during the early phase of the
kernel build so we know if gcc has this bug or not.
But this would be the first time to do so IIRC - today we only
do some limited check to see if certain options are supported.
Sam
On Fri, Jan 02, 2009 at 08:14:58PM +0100, Andi Kleen wrote:
> Al Viro <[email protected]> writes:
> >
> > FWIW, I'm using 4.3 on all targets at the moment. See
> > git://git.kernel.org/pub/scm/linux/kernel/git/viro/toolchain.git/
> > for fedora-based variant of that sucker. And yes, it does include
> > cross-to-sparc; all but sh/sh64, in fact (sh had serious compiler
> > breakage around 4.3.0 and backporting from -HEAD was far beyond
> > what I considered reasonable at that point).
>
> The full opensuse distribution also has a couple of spec files for
> generating cross compilers from recent versions. These are for
> icecream, but can be relatively easily adapted (or the binaries
> reused)
>
> In general I don't think building cross compilers is as hard
> as it used to be, so it can be reasonably done without any support
> scripts too.
*snort*
Well, the only support script here is "call rpmbuild with the right
arguments" (and kmk, which is about running kernel cross-build
conveniently and has nothing to do with building cross-toolchain
itself).
I'll need to update that to more current gcc anyway (and see if that
takes care of sh/sh64), so if you can dig those .spec out... throw
them my way and I'll add them to repository.
How well do they handle the targets for which you have no glibc-dev
binary rpms, BTW? I needed headers for ia64 and ppc64; both fortunately
are among the generally supported targets...
Linus Torvalds <[email protected]> writes:
> I realize that cross-tools tend to lag a bit - the pressure to maintain
> them tends to be much lower - but I was sure we had somebody who did a
> reasonable cross-compiler toolchain. Is gcc-4.1 really the most modern
> thing that is easily available?
Not sure about crosstool and other platforms but I'm using gcc 4.3.2+
for ARM (on x86_64) and building it is straightforward. Just unpack
binutils, gcc (and optionally glibc); configure; make; make install.
Prerequisites: kernel headers and minimal glibc installed.
(Some recent glibc versions require a patch or two).
--
Krzysztof Halasa