2005-01-24 15:43:33

by Russell King

[permalink] [raw]
Subject: ARM undefined symbols. Again.

Sam,

Where did the hacks go which detect the silent failure of the ARM binutils?

I'm asking because I've just spent all of today trying to work out why
the hell code isn't working, only to find that it's all down to the
toolchain problem not being detected by kbuild. IOW, the code sequence:

mov r0, #CR1A_SMPE

where CR1A_SMPE is undefined, assembles to:

c001224c: e3a00000 mov r0, #0 ; 0x0

without any warnings or errors, and links a successful kernel, leaving
this in System.map:

U CR1A_SMPE

I absolutely must have the kernel build system detecting this binutils
problem when it occurs.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core


2005-01-31 16:16:13

by Sam Ravnborg

[permalink] [raw]
Subject: Re: ARM undefined symbols. Again.

On Mon, Jan 24, 2005 at 03:43:26PM +0000, Russell King wrote:
> Sam,
>
> Where did the hacks go which detect the silent failure of the ARM binutils?

They weant away because it caused lots of troubles with sparc and um.
Can you use this (untested patch) for arm?

Sam

===== Makefile 1.85 vs edited =====
--- 1.85/arch/arm/Makefile 2004-10-24 00:50:16 +02:00
+++ edited/Makefile 2005-01-31 17:15:55 +01:00
@@ -181,10 +181,19 @@
# Convert bzImage to zImage
bzImage: zImage

-zImage Image xipImage bootpImage uImage: vmlinux
+# Check for undefined symbols. Broken arm binutils may leave symbols undefined
+.PHONY: check-vmlinux
+vmlinux: check-vmlinux
+ if [ "`$(NM) -u vmlinux`" != "" ]; then \
+ echo "vmlinux: error: undefined symbol(s) found:"; \
+ $(NM) -u vmlinux; \
+ exit 1; \
+ fi
+
+zImage Image xipImage bootpImage uImage: check-vmlinux
$(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $(boot)/$@

-zinstall install: vmlinux
+zinstall install: check-vmlinux
$(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $@

CLEAN_FILES += include/asm-arm/constants.h* include/asm-arm/mach-types.h \

2005-02-07 11:44:07

by Russell King

[permalink] [raw]
Subject: Re: ARM undefined symbols. Again.

On Mon, Jan 31, 2005 at 05:17:53PM +0100, Sam Ravnborg wrote:
> On Mon, Jan 24, 2005 at 03:43:26PM +0000, Russell King wrote:
> > Sam,
> >
> > Where did the hacks go which detect the silent failure of the ARM binutils?
>
> They weant away because it caused lots of troubles with sparc and um.
> Can you use this (untested patch) for arm?

After fixing the obvious problems with this patch, it leaves one remaining.
make vmlinux doesn't invoke this check, and people actually use vmlinux
in the embedded world (even though some of us would prefer them not to.)

Maybe we need an architecture hook or something for post-processing
vmlinux?

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core

2005-02-08 19:42:40

by Sam Ravnborg

[permalink] [raw]
Subject: Re: ARM undefined symbols. Again.

On Mon, Feb 07, 2005 at 11:43:59AM +0000, Russell King wrote:
>
> Maybe we need an architecture hook or something for post-processing
> vmlinux?
Makes sense.
For now arm can provide an arm specific cmd_vmlinux__ like um does.

The ?= used in Makefile snippet below allows an ARCH to override the
definition of quiet_cmd_vmlinux__ and cmd_vmlinux__



quiet_cmd_vmlinux__ ?= LD $@
cmd_vmlinux__ ?= $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) -o $@ \
-T $(vmlinux-lds) $(vmlinux-init) \
--start-group $(vmlinux-main) --end-group \
$(filter-out $(vmlinux-lds) $(vmlinux-init) $(vmlinux-main) FORCE ,$^)


Sam

2005-02-08 20:05:19

by Russell King

[permalink] [raw]
Subject: Re: ARM undefined symbols. Again.

On Tue, Feb 08, 2005 at 08:42:43PM +0100, Sam Ravnborg wrote:
> On Mon, Feb 07, 2005 at 11:43:59AM +0000, Russell King wrote:
> >
> > Maybe we need an architecture hook or something for post-processing
> > vmlinux?
> Makes sense.
> For now arm can provide an arm specific cmd_vmlinux__ like um does.
>
> The ?= used in Makefile snippet below allows an ARCH to override the
> definition of quiet_cmd_vmlinux__ and cmd_vmlinux__

Great - I'll merge your previous idea with this one and throw a patch
here.

Thanks.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core

2005-02-08 20:11:56

by Alex Muradin

[permalink] [raw]
Subject: Re: ARM undefined symbols. Again.

I'm getting your mail!

Check out you code cause if I'm getting your mail, then you're sending
it out to all your customers.

-Alex
[email protected]

Gmail user


On Tue, 8 Feb 2005 20:42:43 +0100, Sam Ravnborg <[email protected]> wrote:
> On Mon, Feb 07, 2005 at 11:43:59AM +0000, Russell King wrote:
> >
> > Maybe we need an architecture hook or something for post-processing
> > vmlinux?
> Makes sense.
> For now arm can provide an arm specific cmd_vmlinux__ like um does.
>
> The ?= used in Makefile snippet below allows an ARCH to override the
> definition of quiet_cmd_vmlinux__ and cmd_vmlinux__
>
> quiet_cmd_vmlinux__ ?= LD $@
> cmd_vmlinux__ ?= $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) -o $@ \
> -T $(vmlinux-lds) $(vmlinux-init) \
> --start-group $(vmlinux-main) --end-group \
> $(filter-out $(vmlinux-lds) $(vmlinux-init) $(vmlinux-main) FORCE ,$^)
>
> Sam
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2005-02-09 10:41:05

by Russell King

[permalink] [raw]
Subject: Re: ARM undefined symbols. Again.

On Tue, Feb 08, 2005 at 08:05:01PM +0000, Russell King wrote:
> On Tue, Feb 08, 2005 at 08:42:43PM +0100, Sam Ravnborg wrote:
> > On Mon, Feb 07, 2005 at 11:43:59AM +0000, Russell King wrote:
> > >
> > > Maybe we need an architecture hook or something for post-processing
> > > vmlinux?
> > Makes sense.
> > For now arm can provide an arm specific cmd_vmlinux__ like um does.
> >
> > The ?= used in Makefile snippet below allows an ARCH to override the
> > definition of quiet_cmd_vmlinux__ and cmd_vmlinux__
>
> Great - I'll merge your previous idea with this one and throw a patch
> here.

Well, this was a great idea until you find that this is also used for
linking the intermediate vmlinux objects for kallsyms, and kallsyms
uses weak (== undefined) symbols:

LD .tmp_vmlinux1
.tmp_vmlinux1: error: undefined symbol(s) found:
w kallsyms_addresses
w kallsyms_markers
w kallsyms_names
w kallsyms_num_syms
w kallsyms_token_index
w kallsyms_token_table

Maybe kallsyms needs to provide an empty object with these symbols
defined for the first linker pass, instead of using weak symbols?

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core

2005-02-13 17:30:06

by Russell King

[permalink] [raw]
Subject: Re: ARM undefined symbols. Again.

On Wed, Feb 09, 2005 at 10:40:53AM +0000, Russell King wrote:
> On Tue, Feb 08, 2005 at 08:05:01PM +0000, Russell King wrote:
> > On Tue, Feb 08, 2005 at 08:42:43PM +0100, Sam Ravnborg wrote:
> > > On Mon, Feb 07, 2005 at 11:43:59AM +0000, Russell King wrote:
> > > >
> > > > Maybe we need an architecture hook or something for post-processing
> > > > vmlinux?
> > > Makes sense.
> > > For now arm can provide an arm specific cmd_vmlinux__ like um does.
> > >
> > > The ?= used in Makefile snippet below allows an ARCH to override the
> > > definition of quiet_cmd_vmlinux__ and cmd_vmlinux__
> >
> > Great - I'll merge your previous idea with this one and throw a patch
> > here.
>
> Well, this was a great idea until you find that this is also used for
> linking the intermediate vmlinux objects for kallsyms, and kallsyms
> uses weak (== undefined) symbols:
>
> LD .tmp_vmlinux1
> .tmp_vmlinux1: error: undefined symbol(s) found:
> w kallsyms_addresses
> w kallsyms_markers
> w kallsyms_names
> w kallsyms_num_syms
> w kallsyms_token_index
> w kallsyms_token_table
>
> Maybe kallsyms needs to provide an empty object with these symbols
> defined for the first linker pass, instead of using weak symbols?

So, what's the answer? Maybe this patch? With this, we can drop the
__attribute__((weak)) from the kallsyms symbols since they're always
provided.

diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x *.orig -x *.rej orig/Makefile linux/Makefile
--- orig/Makefile Sun Feb 13 17:26:38 2005
+++ linux/Makefile Sun Feb 13 17:24:17 2005
@@ -702,14 +702,20 @@ quiet_cmd_kallsyms = KSYM $@
cmd_kallsyms = $(NM) -n $< | $(KALLSYMS) \
$(if $(CONFIG_KALLSYMS_ALL),--all-symbols) > $@

-.tmp_kallsyms1.o .tmp_kallsyms2.o .tmp_kallsyms3.o: %.o: %.S scripts FORCE
+.tmp_kallsyms0.o .tmp_kallsyms1.o .tmp_kallsyms2.o .tmp_kallsyms3.o: %.o: %.S scripts FORCE
$(call if_changed_dep,as_o_S)

+.tmp_kallsyms0.S: FORCE
+ @( echo ".data"; \
+ for sym in addresses markers names num_syms token_index token_table; \
+ do echo -e ".globl kallsyms_$$sym\nkallsyms_$$sym:\n"; done; \
+ echo ".word 0"; ) > $@
+
.tmp_kallsyms%.S: .tmp_vmlinux% $(KALLSYMS)
$(call cmd,kallsyms)

# .tmp_vmlinux1 must be complete except kallsyms, so update vmlinux version
-.tmp_vmlinux1: $(vmlinux-lds) $(vmlinux-all) FORCE
+.tmp_vmlinux1: $(vmlinux-lds) $(vmlinux-all) .tmp_kallsyms0.o FORCE
$(call if_changed_rule,ksym_ld)

.tmp_vmlinux2: $(vmlinux-lds) $(vmlinux-all) .tmp_kallsyms1.o FORCE
diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x *.orig -x *.rej orig/arch/arm/Makefile linux/arch/arm/Makefile
--- orig/arch/arm/Makefile Mon Nov 15 09:15:02 2004
+++ linux/arch/arm/Makefile Wed Feb 9 10:09:36 2005
@@ -156,6 +156,17 @@ else
all: zImage
endif

+# Override the default command for generating vmlinux, so we can check for
+# the assembler bug with undefined symbols.
+cmd_vmlinux__ = $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) -o $@ \
+ -T $(vmlinux-lds) $(vmlinux-init) \
+ --start-group $(vmlinux-main) --end-group \
+ $(filter-out $(vmlinux-lds) $(vmlinux-init) $(vmlinux-main) FORCE ,$^); \
+ if [ "`$(NM) -u $@`" != "" ]; then \
+ echo "$@: error: undefined symbol(s) found:"; \
+ $(NM) -u $@; exit 1; \
+ fi
+
boot := arch/arm/boot

# Update machine arch and proc symlinks if something which affects


--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core

2005-02-14 13:11:26

by Paulo Marques

[permalink] [raw]
Subject: Re: ARM undefined symbols. Again.

Russell King wrote:
> On Wed, Feb 09, 2005 at 10:40:53AM +0000, Russell King wrote:
>>On Tue, Feb 08, 2005 at 08:05:01PM +0000, Russell King wrote:
>>[...]
>> LD .tmp_vmlinux1
>>.tmp_vmlinux1: error: undefined symbol(s) found:
>> w kallsyms_addresses
>> w kallsyms_markers
>> w kallsyms_names
>> w kallsyms_num_syms
>> w kallsyms_token_index
>> w kallsyms_token_table
>>
>>Maybe kallsyms needs to provide an empty object with these symbols
>>defined for the first linker pass, instead of using weak symbols?
>
>
> So, what's the answer? Maybe this patch? With this, we can drop the
> __attribute__((weak)) from the kallsyms symbols since they're always
> provided.
>
> diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x *.orig -x *.rej orig/Makefile linux/Makefile
> --- orig/Makefile Sun Feb 13 17:26:38 2005
> +++ linux/Makefile Sun Feb 13 17:24:17 2005
> @@ -702,14 +702,20 @@ quiet_cmd_kallsyms = KSYM $@
> cmd_kallsyms = $(NM) -n $< | $(KALLSYMS) \
> $(if $(CONFIG_KALLSYMS_ALL),--all-symbols) > $@
>
> -.tmp_kallsyms1.o .tmp_kallsyms2.o .tmp_kallsyms3.o: %.o: %.S scripts FORCE
> +.tmp_kallsyms0.o .tmp_kallsyms1.o .tmp_kallsyms2.o .tmp_kallsyms3.o: %.o: %.S scripts FORCE
> $(call if_changed_dep,as_o_S)
>
> +.tmp_kallsyms0.S: FORCE
> + @( echo ".data"; \
> + for sym in addresses markers names num_syms token_index token_table; \
> + do echo -e ".globl kallsyms_$$sym\nkallsyms_$$sym:\n"; done; \
> + echo ".word 0"; ) > $@

I think it would be better to pass an argument to scripts/kallsyms (like
-0 or something) that instructed it to generate the same file it usually
generates but without any actual symbol information.

This way it will be easier to maintain this code in case new symbols are
needed in the future. Having this done in the Makefile means that to add
a new symbol we will have to change scripts/kallsyms, kernel/kallsyms
and a not so clearly related Makefile.

I'll try to make a small patch if I can get some time (maybe later this
week).

--
Paulo Marques - http://www.grupopie.com

All that is necessary for the triumph of evil is that good men do nothing.
Edmund Burke (1729 - 1797)

2005-02-25 19:49:06

by Russell King

[permalink] [raw]
Subject: Re: ARM undefined symbols. Again.

On Mon, Feb 14, 2005 at 01:10:29PM +0000, Paulo Marques wrote:
> Russell King wrote:
> > On Wed, Feb 09, 2005 at 10:40:53AM +0000, Russell King wrote:
> >>On Tue, Feb 08, 2005 at 08:05:01PM +0000, Russell King wrote:
> >>[...]
> >> LD .tmp_vmlinux1
> >>.tmp_vmlinux1: error: undefined symbol(s) found:
> >> w kallsyms_addresses
> >> w kallsyms_markers
> >> w kallsyms_names
> >> w kallsyms_num_syms
> >> w kallsyms_token_index
> >> w kallsyms_token_table
> >>
> >>Maybe kallsyms needs to provide an empty object with these symbols
> >>defined for the first linker pass, instead of using weak symbols?
> >
> >
> > So, what's the answer? Maybe this patch? With this, we can drop the
> > __attribute__((weak)) from the kallsyms symbols since they're always
> > provided.
> >
> > diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x *.orig -x *.rej orig/Makefile linux/Makefile
> > --- orig/Makefile Sun Feb 13 17:26:38 2005
> > +++ linux/Makefile Sun Feb 13 17:24:17 2005
> > @@ -702,14 +702,20 @@ quiet_cmd_kallsyms = KSYM $@
> > cmd_kallsyms = $(NM) -n $< | $(KALLSYMS) \
> > $(if $(CONFIG_KALLSYMS_ALL),--all-symbols) > $@
> >
> > -.tmp_kallsyms1.o .tmp_kallsyms2.o .tmp_kallsyms3.o: %.o: %.S scripts FORCE
> > +.tmp_kallsyms0.o .tmp_kallsyms1.o .tmp_kallsyms2.o .tmp_kallsyms3.o: %.o: %.S scripts FORCE
> > $(call if_changed_dep,as_o_S)
> >
> > +.tmp_kallsyms0.S: FORCE
> > + @( echo ".data"; \
> > + for sym in addresses markers names num_syms token_index token_table; \
> > + do echo -e ".globl kallsyms_$$sym\nkallsyms_$$sym:\n"; done; \
> > + echo ".word 0"; ) > $@
>
> I think it would be better to pass an argument to scripts/kallsyms (like
> -0 or something) that instructed it to generate the same file it usually
> generates but without any actual symbol information.
>
> This way it will be easier to maintain this code in case new symbols are
> needed in the future. Having this done in the Makefile means that to add
> a new symbol we will have to change scripts/kallsyms, kernel/kallsyms
> and a not so clearly related Makefile.
>
> I'll try to make a small patch if I can get some time (maybe later this
> week).

So, what's happening about this?

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core

2005-02-25 19:58:45

by Linus Torvalds

[permalink] [raw]
Subject: Re: ARM undefined symbols. Again.



On Fri, 25 Feb 2005, Russell King wrote:
>
> So, what's happening about this?

Btw, is there any real reason why the ARM _tools_ can't just be fixed? I
don't see why this isn't a tools bug?

Linus

2005-02-25 20:24:00

by Russell King

[permalink] [raw]
Subject: Re: ARM undefined symbols. Again.

On Fri, Feb 25, 2005 at 11:59:01AM -0800, Linus Torvalds wrote:
> On Fri, 25 Feb 2005, Russell King wrote:
> > So, what's happening about this?
>
> Btw, is there any real reason why the ARM _tools_ can't just be fixed? I
> don't see why this isn't a tools bug?

It is a tools bug. But the issue is that *all* versions of binutils
currently available which are kernel-capable (since the inclusion of
the kbuild .incbin requirement on binutils) have this bug, with the
exception of maybe CVS versions.

We can't say "you must use the current CVS binutils to build the
kernel" because that's not a sane toolchain base to build products
on.

I've been wanting to see a version of binutils released pretty damn
quick so I can say "kernel only builds with latest toolchain" but
I suspect even that's going to be seen as being unreasonable.

So, my only option is to ensure that the problem with current toolchains
*is* detectable, rather than having what appears to be a perfectly good
kernel built, which may appear to run fine for the most part, but may
randomly fail due to wrongly built assembly code.

And yes, the toolchain peoples point of view is "fix the kernel".

Sorry, I'm stuck between a rock and a hard place, much like everything
else I'm faced with at the moment... ARMv6 cache patch for example.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core

2005-02-25 20:31:21

by Linus Torvalds

[permalink] [raw]
Subject: Re: ARM undefined symbols. Again.



On Fri, 25 Feb 2005, Russell King wrote:
>
> We can't say "you must use the current CVS binutils to build the
> kernel" because that's not a sane toolchain base to build products
> on.

Sure. But it's probably enough that just a couple of core developers would
have a CVS version to make sure that when it occasionally happens, it gets
noticed quickly enough.

In other words, I don't think you can say "get the CVS version" to most
users, but I do not see for example you you or some of the people around
you don't have at least one setup set up with that fixed version.

This has been going on for at least a year, probably longer. I could
understand it if it was a "we found this old bug, and haven't had time to
get around it", but what I don't understand is when there's been a tools
bug that's been known about for a long time, and apparently nobody has
ever even bothered to try the fixed versions.

> And yes, the toolchain peoples point of view is "fix the kernel".

For a known bug where just having _one_ active developer using a fixed
tool would mean that this doesn't happen?

That makes no sense. Or, more likely, it means that the toolchain people
are incompetent bastards who don't care about bugs and have no pride at
all in what they do.

Linus

2005-02-25 20:55:26

by Paulo Marques

[permalink] [raw]
Subject: Re: ARM undefined symbols. Again.

diff -uprN -X dontdiff linux-2.6.11-rc5-vanilla/kernel/kallsyms.c linux-2.6.11-rc5/kernel/kallsyms.c
--- linux-2.6.11-rc5-vanilla/kernel/kallsyms.c 2005-02-25 20:36:44.000000000 +0000
+++ linux-2.6.11-rc5/kernel/kallsyms.c 2005-02-25 20:15:19.000000000 +0000
@@ -29,14 +29,14 @@
#endif

/* These will be re-linked against their real values during the second link stage */
-extern unsigned long kallsyms_addresses[] __attribute__((weak));
-extern unsigned long kallsyms_num_syms __attribute__((weak,section("data")));
-extern u8 kallsyms_names[] __attribute__((weak));
+extern unsigned long kallsyms_addresses[];
+extern unsigned long kallsyms_num_syms;
+extern u8 kallsyms_names[];

-extern u8 kallsyms_token_table[] __attribute__((weak));
-extern u16 kallsyms_token_index[] __attribute__((weak));
+extern u8 kallsyms_token_table[];
+extern u16 kallsyms_token_index[];

-extern unsigned long kallsyms_markers[] __attribute__((weak));
+extern unsigned long kallsyms_markers[];

static inline int is_kernel_inittext(unsigned long addr)
{
diff -uprN -X dontdiff linux-2.6.11-rc5-vanilla/Makefile linux-2.6.11-rc5/Makefile
--- linux-2.6.11-rc5-vanilla/Makefile 2005-02-25 20:36:15.000000000 +0000
+++ linux-2.6.11-rc5/Makefile 2005-02-25 20:25:44.000000000 +0000
@@ -702,14 +702,20 @@ quiet_cmd_kallsyms = KSYM $@
cmd_kallsyms = $(NM) -n $< | $(KALLSYMS) \
$(if $(CONFIG_KALLSYMS_ALL),--all-symbols) > $@

-.tmp_kallsyms1.o .tmp_kallsyms2.o .tmp_kallsyms3.o: %.o: %.S scripts FORCE
+quiet_cmd_kallsyms0 = KSYM $@
+ cmd_kallsyms0 = $(KALLSYMS) -0 > $@
+
+.tmp_kallsyms0.o .tmp_kallsyms1.o .tmp_kallsyms2.o .tmp_kallsyms3.o: %.o: %.S scripts FORCE
$(call if_changed_dep,as_o_S)

+.tmp_kallsyms0.S: $(KALLSYMS) FORCE
+ $(call cmd,kallsyms0)
+
.tmp_kallsyms%.S: .tmp_vmlinux% $(KALLSYMS)
$(call cmd,kallsyms)

# .tmp_vmlinux1 must be complete except kallsyms, so update vmlinux version
-.tmp_vmlinux1: $(vmlinux-lds) $(vmlinux-all) FORCE
+.tmp_vmlinux1: $(vmlinux-lds) $(vmlinux-all) .tmp_kallsyms0.o FORCE
$(call if_changed_rule,ksym_ld)

.tmp_vmlinux2: $(vmlinux-lds) $(vmlinux-all) .tmp_kallsyms1.o FORCE
diff -uprN -X dontdiff linux-2.6.11-rc5-vanilla/scripts/kallsyms.c linux-2.6.11-rc5/scripts/kallsyms.c
--- linux-2.6.11-rc5-vanilla/scripts/kallsyms.c 2005-02-25 20:36:45.000000000 +0000
+++ linux-2.6.11-rc5/scripts/kallsyms.c 2005-02-25 20:33:25.000000000 +0000
@@ -93,7 +93,7 @@ unsigned char best_table_len[256];
static void
usage(void)
{
- fprintf(stderr, "Usage: kallsyms [--all-symbols] < in.map > out.S\n");
+ fprintf(stderr, "Usage: kallsyms [--all-symbols] [-0] < in.map > out.S\n");
exit(1);
}

@@ -230,6 +230,20 @@ static void output_label(char *label)
printf("%s:\n",label);
}

+static void output_header(void)
+{
+ printf("#include <asm/types.h>\n");
+ printf("#if BITS_PER_LONG == 64\n");
+ printf("#define PTR .quad\n");
+ printf("#define ALGN .align 8\n");
+ printf("#else\n");
+ printf("#define PTR .long\n");
+ printf("#define ALGN .align 4\n");
+ printf("#endif\n");
+
+ printf(".data\n");
+}
+
/* uncompress a compressed symbol. When this function is called, the best table
* might still be compressed itself, so the function needs to be recursive */
static int expand_symbol(unsigned char *data, int len, char *result)
@@ -257,6 +271,25 @@ static int expand_symbol(unsigned char *
return total;
}

+/* this function writes an empty assembly output with just the definitions
+ * of the variables */
+
+static void write_src_zero_pass(void)
+{
+ output_header();
+
+ output_label("kallsyms_addresses");
+ output_label("kallsyms_num_syms");
+ output_label("kallsyms_names");
+ output_label("kallsyms_markers");
+ output_label("kallsyms_token_table");
+ output_label("kallsyms_token_index");
+
+ printf("\t.byte\t0\n");
+}
+
+/* this one writes the real deal */
+
static void
write_src(void)
{
@@ -265,16 +298,7 @@ write_src(void)
unsigned int *markers;
char buf[KSYM_NAME_LEN+1];

- printf("#include <asm/types.h>\n");
- printf("#if BITS_PER_LONG == 64\n");
- printf("#define PTR .quad\n");
- printf("#define ALGN .align 8\n");
- printf("#else\n");
- printf("#define PTR .long\n");
- printf("#define ALGN .align 4\n");
- printf("#endif\n");
-
- printf(".data\n");
+ output_header();

output_label("kallsyms_addresses");
valid = 0;
@@ -672,11 +696,18 @@ static void optimize_token_table(void)
int
main(int argc, char **argv)
{
- if (argc == 2 && strcmp(argv[1], "--all-symbols") == 0)
- all_symbols = 1;
- else if (argc != 1)
- usage();
+ int i;

+ for (i = 1; i < argc; i++) {
+ if (strcmp(argv[i], "--all-symbols") == 0)
+ all_symbols = 1;
+ else if(strcmp(argv[i], "-0") == 0) {
+ write_src_zero_pass();
+ return 0;
+ } else
+ usage();
+ }
+
read_map(stdin);
optimize_token_table();
write_src();


Attachments:
kallpatch (4.72 kB)

2005-02-25 21:19:50

by Paulo Marques

[permalink] [raw]
Subject: Re: ARM undefined symbols. Again.

Sam Ravnborg wrote:
> On Fri, Feb 25, 2005 at 08:54:56PM +0000, Paulo Marques wrote:
>
>>The patch (against 2.6.11-rc5) is attached, should you decide to use it.
>
> How does the patch help rmk with respect to the tools issue?

From the thread I gathered that the problem Russell King was having was
caused by the fact that kallsyms used "weak" symbols.

He proposed a solution where he created an empty assembly file with just
the symbols defined to make the symbols exist already on the first pass.
This way they wouldn't have to be defined as weak anymore.

His patch created the assembly file using "echo's" from the Makefile. I
just suggested that it would be better to do it from scripts/kallsyms.c
directly, so that it would be easier to maintain in case new symbols
need to be added in the future.

That's what this patch does.

By the way, I'm not really sure about my changes to the Makefile,
although they comply with Linus Confidence Level 3(*). I think you're
the one with the best understanding on the kbuild process to comment on
them.

--
Paulo Marques - http://www.grupopie.com

All that is necessary for the triumph of evil is that good men do nothing.
Edmund Burke (1729 - 1797)

(*) It works

2005-02-25 22:04:30

by Daniel Jacobowitz

[permalink] [raw]
Subject: Re: ARM undefined symbols. Again.

On Fri, Feb 25, 2005 at 08:23:49PM +0000, Russell King wrote:
> On Fri, Feb 25, 2005 at 11:59:01AM -0800, Linus Torvalds wrote:
> > On Fri, 25 Feb 2005, Russell King wrote:
> > > So, what's happening about this?
> >
> > Btw, is there any real reason why the ARM _tools_ can't just be fixed? I
> > don't see why this isn't a tools bug?
>
> It is a tools bug. But the issue is that *all* versions of binutils
> currently available which are kernel-capable (since the inclusion of
> the kbuild .incbin requirement on binutils) have this bug, with the
> exception of maybe CVS versions.
>
> We can't say "you must use the current CVS binutils to build the
> kernel" because that's not a sane toolchain base to build products
> on.
>
> I've been wanting to see a version of binutils released pretty damn
> quick so I can say "kernel only builds with latest toolchain" but
> I suspect even that's going to be seen as being unreasonable.

Not sure who you asked, but since I run the binutils releases...

I am fairly positive that this bug has been fixed in the binutils CVS:

2004-07-02 Nick Clifton <[email protected]>

* config/tc-arm.c (md_apply_fix3:BFD_RELOC_ARM_IMMEDIATE): Do not
allow values which have come from undefined symbols.
Always consider this fixup to have been processed as a reloc
cannot be generated for it.

I know several ARM kernel developers who are using tools with this
patch applied already. Also, I anticipate the release of binutils 2.16
including the fix in about a month.

> And yes, the toolchain peoples point of view is "fix the kernel".

Huh? Obviously the kernel isn't broken, unless you're talking about
the kallsyms checks now.

--
Daniel Jacobowitz
CodeSourcery, LLC

2005-02-25 22:48:52

by Linus Torvalds

[permalink] [raw]
Subject: Re: ARM undefined symbols. Again.



On Fri, 25 Feb 2005, Russell King wrote:
>
> That's fine until you consider the wide number of machines for ARM,
> any of which could have this problem.

Fair enough. "ARM" doesn't end up being just one architecture, and that's
a good point.

> Unless of course, you believe that one person should carry everything,
> which is what I feel your above comment is effectively saying.

No, let me be the last to argue for centralized Q&A. Doesn't work. I'd
rather argue that it's not an issue of trying to get everybody to upgrade
and making old versions "not supported". It seems more benign than that,
in that it should be sufficient if there were just enough new versions out
there, for some arbitrary value of "enough".

In particular, it seems downright _wrong_ that an issue like this has been
around forever, and nothing has actually been done about the fundamental
problem. At some point, "kernel build bandages" are just not worth it any
more, if people aren't even trying to actually fix the real issue.

And one year of apparently "no progress" smells. Bad.

Linus

2005-02-25 22:52:37

by Linus Torvalds

[permalink] [raw]
Subject: Re: ARM undefined symbols. Again.



On Fri, 25 Feb 2005, Linus Torvalds wrote:
>
> And one year of apparently "no progress" smells. Bad.

Side note: I don't actually remember how long it's been. Maybe it just
feels a lot longer than it actually is.

Linus

2005-02-26 11:17:57

by Russell King

[permalink] [raw]
Subject: Re: ARM undefined symbols. Again.

On Fri, Feb 25, 2005 at 02:49:05PM -0800, Linus Torvalds wrote:
> On Fri, 25 Feb 2005, Russell King wrote:
> > That's fine until you consider the wide number of machines for ARM,
> > any of which could have this problem.
>
> Fair enough. "ARM" doesn't end up being just one architecture, and that's
> a good point.
>
> > Unless of course, you believe that one person should carry everything,
> > which is what I feel your above comment is effectively saying.
>
> No, let me be the last to argue for centralized Q&A. Doesn't work. I'd
> rather argue that it's not an issue of trying to get everybody to upgrade
> and making old versions "not supported". It seems more benign than that,
> in that it should be sufficient if there were just enough new versions out
> there, for some arbitrary value of "enough".
>
> In particular, it seems downright _wrong_ that an issue like this has been
> around forever, and nothing has actually been done about the fundamental
> problem. At some point, "kernel build bandages" are just not worth it any
> more, if people aren't even trying to actually fix the real issue.

Unfortunately, the makeup of the ARM community is mostly developer-based,
where developers are working away at getting something running on some
custom platform. Since all the platforms are different, it means that
they're working in their own unique space.

If we had a single platform, or even a small number of platforms, then
your approach does make sense - a small number of people could use a
CVS version and be sure to cover 99 if not 100% of the cases.

However, with such a large number of platforms, there will always be
a significant chance where a small number of people will never see the
problems seen by the majority.

To put it another way, the code coverage achieved by a small number of
people running the fixed toolchains would be no where near good enough.

Having a properly working toolchain is of upmost importance for us. In
reality though, we don't have sufficient weight of people with the right
mindset behind the toolchain to ensure bugs in or problems with it are
found quickly. There are unfortunately plenty of users who are happy to
try to work around bugs without reporting them.

Another problem with the toolchain is the constant feature churn, with
old features which are in use vanishing, or documentated behaviour
suddenly being turned upon its head and being called a bug, fixed in
the toolchain code and then sometime later the documentation may get
updated if anyone noticed. This, in itself, provides _me_ at least with
a big disincentive to upgrade from a version of the toolchain which has
been proven to work. History has taught me that at every step.

So, I have to do _something_ to ensure that we have a reasonable status
quo in place. Correction: _I_ don't have to do anything at all if I
don't care about Linux kernels standing a chance of being built correctly
by less experienced developers using buggy toolchains. Then again, maybe
_that_ is the correct approach.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core

2005-02-26 11:29:22

by Russell King

[permalink] [raw]
Subject: Re: ARM undefined symbols. Again.

On Sat, Feb 26, 2005 at 11:17:48AM +0000, Russell King wrote:
> So, I have to do _something_ to ensure that we have a reasonable status
> quo in place. Correction: _I_ don't have to do anything at all if I
> don't care about Linux kernels standing a chance of being built correctly
> by less experienced developers using buggy toolchains. Then again, maybe
> _that_ is the correct approach.

Just to make it 100% clear: I don't particularly like to work around
the shortcomings of the ARM toolchain by adding hacks to the kernel.
It should be first and foremost up to the toolchain people to fix the
problem. If someone maintaining the ARM toolchain could be persuaded
to maintain a bugfix only branch, this would be a huge step forwards.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core