Link failures due to __bug_table in current -next

Link failures due to __bug_table in current -next

[Mark Brown]

I'm seeing linker failures in -next as of today:

`.exit.text' referenced in section `__bug_table' of fs/built-in.o:
defined in discarded section `.exit.text' of fs/built-in.o
`.exit.text' referenced in section `__bug_table' of crypto/built-in.o:
defined in discarded section `.exit.text' of crypto/built-in.o
`.exit.text' referenced in section `__bug_table' of net/built-in.o:
defined in discarded section `.exit.text' of net/built-in.o
`.exit.text' referenced in section `__bug_table' of net/built-in.o:
defined in discarded section `.exit.text' of net/built-in.o

which appears to be due to the chnage to use generic BUG() introduced in
commit 5254a3 (ARM: 7017/1: Use generic BUG() handler), reverting that
commit resolves the issue for me.

I'm wondering if we need something like 7c8a25 (CRIS: Discard exit.text
and .data at runtime) but figuring it out is a bit beyond my ld skills
right now.

Link failures due to __bug_table in current -next

[Russell King - ARM Linux]

On Mon, Sep 19, 2011 at 01:09:54PM +0100, Mark Brown wrote:
> I'm seeing linker failures in -next as of today:
> 
> `.exit.text' referenced in section `__bug_table' of fs/built-in.o:
> defined in discarded section `.exit.text' of fs/built-in.o
> `.exit.text' referenced in section `__bug_table' of crypto/built-in.o:
> defined in discarded section `.exit.text' of crypto/built-in.o
> `.exit.text' referenced in section `__bug_table' of net/built-in.o:
> defined in discarded section `.exit.text' of net/built-in.o
> `.exit.text' referenced in section `__bug_table' of net/built-in.o:
> defined in discarded section `.exit.text' of net/built-in.o
> 
> which appears to be due to the chnage to use generic BUG() introduced in
> commit 5254a3 (ARM: 7017/1: Use generic BUG() handler), reverting that
> commit resolves the issue for me.

It's a known issue - there's been an earlier thread about it:

http://lists.arm.linux.org.uk/lurker/thread/20110808.184931.a38e1c4e.en.html

The asm-generic/vmlinux.lds.S is basically relying on undefined behaviour
from the linker, which is biting us.  There is no guaranteed ordering of
matching input sections to output sections, so listing the same section
more than once in the linker script is asking for problems.

The unfortunate thing is that the linker internally groups the /DISCARD/
sections together into one section, all grouped at the first mention of
the /DISCARD/ section.  This means if you have several /DISCARD/ sections
scattered throughout the linker script, and you rely on the order in which
they appear, you're going to be disappointed because they'll all be merged
into the same position as the first one.

Practically, and observably, this means that for this linker script:

#undef DISCARD_EXIT_TEXT
#ifdef FOO
	/DISCARD/ : {
		*(.foo)
	}
#endif

	.foo : {
		*(.foo)
	}

	.exit.text : {
#ifndef DISCARD_EXIT_TEXT
		*(.exit.text)
#endif
	}

	/DISCARD/ : {
		*(.exit.text)
	}

If FOO is set, .exit.text will be discarded along with .foo.  If FOO is
unset, both .exit.text and .foo will be kept.

Practically, this is what's happening - the DISCARD macro from the
asm-generic/vmlinux.lds.S lists input sections which _may_ be discarded
with the assumption that if they are required, they'll be mentioned
_before_ any discards.  _That_ in itself is undefined linker behaviour.

> I'm wondering if we need something like 7c8a25 (CRIS: Discard exit.text
> and .data at runtime) but figuring it out is a bit beyond my ld skills
> right now.

fatal: ambiguous argument '7c8a25': unknown revision or path not in the working
tree.

Maybe you could be more specific?

Link failures due to __bug_table in current -next

[Mark Brown]

On Mon, Sep 19, 2011 at 07:40:27PM +0100, Russell King - ARM Linux wrote:
> On Mon, Sep 19, 2011 at 01:09:54PM +0100, Mark Brown wrote:

> > I'm wondering if we need something like 7c8a25 (CRIS: Discard exit.text
> > and .data at runtime) but figuring it out is a bit beyond my ld skills
> > right now.

> fatal: ambiguous argument '7c8a25': unknown revision or path not in the working
> tree.

> Maybe you could be more specific?

It's in -next as of today, presumably it comes from the CRIS tree.  I
didn't look too closely once the revert worked for me.

Link failures due to __bug_table in current -next

[Russell King - ARM Linux]

On Mon, Sep 19, 2011 at 07:42:30PM +0100, Mark Brown wrote:
> On Mon, Sep 19, 2011 at 07:40:27PM +0100, Russell King - ARM Linux wrote:
> > On Mon, Sep 19, 2011 at 01:09:54PM +0100, Mark Brown wrote:
> 
> > > I'm wondering if we need something like 7c8a25 (CRIS: Discard exit.text
> > > and .data at runtime) but figuring it out is a bit beyond my ld skills
> > > right now.
> 
> > fatal: ambiguous argument '7c8a25': unknown revision or path not in the working
> > tree.
> 
> > Maybe you could be more specific?
> 
> It's in -next as of today, presumably it comes from the CRIS tree.  I
> didn't look too closely once the revert worked for me.

Pleas elook at the error message again.  >>>ambiguous<<<.  That means
git knows about the ID but more than one commit matches it:

$ for f in 0 1 2 3 4 5 6 7 8 9 a b c d e f; do git cat-file -p 7c8a25$f >/dev/null; done
fatal: Not a valid object name 7c8a250
fatal: Not a valid object name 7c8a252
fatal: Not a valid object name 7c8a253
fatal: Not a valid object name 7c8a254
fatal: Not a valid object name 7c8a255
fatal: Not a valid object name 7c8a256
fatal: Not a valid object name 7c8a257
fatal: Not a valid object name 7c8a258
fatal: Not a valid object name 7c8a259
fatal: Not a valid object name 7c8a25a
fatal: Not a valid object name 7c8a25c
fatal: Not a valid object name 7c8a25d
fatal: Not a valid object name 7c8a25e
fatal: Not a valid object name 7c8a25f

So there's IDs 7c8a251 and 7c8a25b in existence, and git doesn't know
which one to use.  So using that, I can track down the commit you
actually meant.

What you're actually after is:

	7c8a25b (CRIS: Discard exit.text and .data at runtime)

which is what you'd get from git if you ask it to give you the shortened
git ID.  (Git knows when the shortened git ID clashes and will add
additional digits to make up a unique ID.)

You can use "git log --pretty='%h (%s)'" to get a log already formatted
with parens around the summary line.

Link failures due to __bug_table in current -next

[Mark Brown]

On Mon, Sep 19, 2011 at 07:58:09PM +0100, Russell King - ARM Linux wrote:
> On Mon, Sep 19, 2011 at 07:42:30PM +0100, Mark Brown wrote:

> > > > I'm wondering if we need something like 7c8a25 (CRIS: Discard exit.text
> > > > and .data at runtime) but figuring it out is a bit beyond my ld skills
> > > > right now.

> Pleas elook at the error message again.  >>>ambiguous<<<.  That means
> git knows about the ID but more than one commit matches it:

Oh, right - sorry.

> So there's IDs 7c8a251 and 7c8a25b in existence, and git doesn't know
> which one to use.  So using that, I can track down the commit you
> actually meant.

> What you're actually after is:

> 	7c8a25b (CRIS: Discard exit.text and .data at runtime)

Yup.

> which is what you'd get from git if you ask it to give you the shortened
> git ID.  (Git knows when the shortened git ID clashes and will add
> additional digits to make up a unique ID.)

> You can use "git log --pretty='%h (%s)'" to get a log already formatted
> with parens around the summary line.

Yeah, it's a shame it doesn't include the disambiguated short ID in the
standard git log output (which is what I always grep if I'm trying to
search commit messages).  I've never seen a way to tell git to use %H
instead of %h there by default that doesn't require me to start copying
scripts or whatever between machines all the time.

Link failures due to __bug_table in current -next

[Russell King - ARM Linux]

On Mon, Sep 19, 2011 at 08:06:03PM +0100, Mark Brown wrote:
> Yeah, it's a shame it doesn't include the disambiguated short ID in the
> standard git log output (which is what I always grep if I'm trying to
> search commit messages).  I've never seen a way to tell git to use %H
> instead of %h there by default that doesn't require me to start copying
> scripts or whatever between machines all the time.

I'm in two minds over this:

[format]
        pretty = %h %s

which I currently have in my ~/.gitconfig file.  It means that I always
get git log output as:

38c0a0c Merge branches 'apei', 'bz-13195' and 'doc' into acpi
b6fd41e Linux 3.1-rc6
8cb3ed17 Merge branch 'drm-fixes' of git://people.freedesktop.org/~airlied/linux4c75278 Merge branch 'fixes' of git://git.linaro.org/people/arnd/arm-soc
14d01ff ioctl: register LTTng ioctl

rather than the more usual medium-pretty output.  I can get that by
using git log --pretty=medium.  The alternative to that is to use
shell aliases to pass the --pretty= argument for a gitslog alias -
or I believe you can create git command aliases in the ~/.gitconfig file.
(Never tried it yet, something I need to look at.)

Link failures due to __bug_table in current -next

[Uwe Kleine-König]

On Mon, Sep 19, 2011 at 08:16:17PM +0100, Russell King - ARM Linux wrote:
> On Mon, Sep 19, 2011 at 08:06:03PM +0100, Mark Brown wrote:
> > Yeah, it's a shame it doesn't include the disambiguated short ID in the
> > standard git log output (which is what I always grep if I'm trying to
> > search commit messages).  I've never seen a way to tell git to use %H
> > instead of %h there by default that doesn't require me to start copying
> > scripts or whatever between machines all the time.
> 
> I'm in two minds over this:
> 
> [format]
>         pretty = %h %s
> 
> which I currently have in my ~/.gitconfig file.  It means that I always
> get git log output as:
> 
> 38c0a0c Merge branches 'apei', 'bz-13195' and 'doc' into acpi
> b6fd41e Linux 3.1-rc6
> 8cb3ed17 Merge branch 'drm-fixes' of git://people.freedesktop.org/~airlied/linux4c75278 Merge branch 'fixes' of git://git.linaro.org/people/arnd/arm-soc
> 14d01ff ioctl: register LTTng ioctl
> 
> rather than the more usual medium-pretty output.  I can get that by
> using git log --pretty=medium.  The alternative to that is to use
> shell aliases to pass the --pretty= argument for a gitslog alias -
> or I believe you can create git command aliases in the ~/.gitconfig file.
> (Never tried it yet, something I need to look at.)
I did:

	git config --global alias.lg "log --pretty=oneline --abbrev-commit"
	git config --global alias.one "show -s --pretty='format:%h (%s)"

which results in

	[alias]
		lg = log --pretty=oneline --abbrev-commit
		one = show -s --pretty='format:%h (%s)'

and makes available the commands

	git lg

(for summary logging) and

	git one

(to format a commit id for use in another commit message).

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Link failures due to __bug_table in current -next

[Russell King - ARM Linux]

On Mon, Sep 19, 2011 at 07:58:09PM +0100, Russell King - ARM Linux wrote:
> What you're actually after is:
> 
> 	7c8a25b (CRIS: Discard exit.text and .data at runtime)

You're talking about this:

        SECURITY_INIT

+       /* .exit.text is discarded at runtime, not link time,
+        * to deal with references from __bug_table
+        */
+       .exit.text : {
+               EXIT_TEXT
+       }
+       .exit.data : {
+               EXIT_DATA
+       }
+

This "fix" won't work for us - the exit text is being discarded
by the very first /DISCARD/ statement due to merging of /DISCARD/
output sections in the linker script, as I explained in my initial
reply to you.

include/asm-generic/vmlinux.lds.S:

#define EXIT_TEXT                                                       \
        *(.exit.text)                                                   \
        DEV_DISCARD(exit.text)                                          \
        CPU_DISCARD(exit.text)                                          \
        MEM_DISCARD(exit.text)

#define DISCARDS                                                        \
        /DISCARD/ : {                                                   \
        EXIT_TEXT                                                       \
        EXIT_DATA                                                       \
        EXIT_CALL                                                       \
        *(.discard)                                                     \
        *(.discard.*)                                                   \
        }

and our vmlinux.lds.S:

SECTIONS
{
        /*
         * unwind exit sections must be discarded before the rest of the
         * unwind sections get included.
         */
        /DISCARD/ : {
                *(.ARM.exidx.exit.text)
                *(.ARM.extab.exit.text)
                ARM_CPU_DISCARD(*(.ARM.exidx.cpuexit.text))
                ARM_CPU_DISCARD(*(.ARM.extab.cpuexit.text))
#ifndef CONFIG_HOTPLUG
                *(.ARM.exidx.devexit.text)
                *(.ARM.extab.devexit.text)
#endif
#ifndef CONFIG_MMU
                *(.fixup)
                *(__ex_table)
#endif
#ifndef CONFIG_SMP_ON_UP
                *(.alt.smp.init)
#endif
        }
...
        .exit.text : {
                ARM_EXIT_KEEP(EXIT_TEXT)
        }
...
        /* Default discards */
        DISCARDS
}

This is functionally equivalent with *current* linkers to:

SECTIONS
{
        /*
         * unwind exit sections must be discarded before the rest of the
         * unwind sections get included.
         */
        /DISCARD/ : {
                *(.ARM.exidx.exit.text)
                *(.ARM.extab.exit.text)
                ARM_CPU_DISCARD(*(.ARM.exidx.cpuexit.text))
                ARM_CPU_DISCARD(*(.ARM.extab.cpuexit.text))
#ifndef CONFIG_HOTPLUG
                *(.ARM.exidx.devexit.text)
                *(.ARM.extab.devexit.text)
#endif
#ifndef CONFIG_MMU
                *(.fixup)
                *(__ex_table)
#endif
#ifndef CONFIG_SMP_ON_UP
                *(.alt.smp.init)
#endif
	        EXIT_TEXT
        	EXIT_DATA
	        EXIT_CALL
        	*(.discard)
	        *(.discard.*)
        }
...
        .exit.text : {
                ARM_EXIT_KEEP(EXIT_TEXT)
        }
...
}

and as *current* linker behaviour is the first input section found in
order in its internal list of output sections matches, the result is
that .exit.text is _always_ discarded irrespective of whether we want
it or not.

As an additional note, I can find nothing in the linker manual which
defines the behaviour of:

	/DISCARD/ : {
		*(.this.section.should.be.discarded)
	}

	.keep : {
		*(.this.section.*)
	}

It is entirely undefined whether ".this.section.should.be.discarded"
would be discarded or whether it ends up in .keep - it all depends on
the internal - undocumented - behaviour of the linker when assigning
input sections to output sections.  As I mention above, that's
observably internal-merged-output-section-order,
input-section-order-within-output-section with current linkers but
there's no guarantee it'll remain that way.

I think we're running into some fundamental problems here, and I think
we need to get binutils people to come out with a statement concerning
how input sections are matched to output sections when there's multiple
statements in the linker script which match (eg, the most specific
match gets priority) and then we need to ensure that the kernels linker
scripts do not rely on section ordering.

Link failures due to __bug_table in current -next

[Russell King - ARM Linux]

On Mon, Sep 19, 2011 at 01:09:54PM +0100, Mark Brown wrote:
> I'm seeing linker failures in -next as of today:
> 
> `.exit.text' referenced in section `__bug_table' of fs/built-in.o:
> defined in discarded section `.exit.text' of fs/built-in.o
> `.exit.text' referenced in section `__bug_table' of crypto/built-in.o:
> defined in discarded section `.exit.text' of crypto/built-in.o
> `.exit.text' referenced in section `__bug_table' of net/built-in.o:
> defined in discarded section `.exit.text' of net/built-in.o
> `.exit.text' referenced in section `__bug_table' of net/built-in.o:
> defined in discarded section `.exit.text' of net/built-in.o
> 
> which appears to be due to the chnage to use generic BUG() introduced in
> commit 5254a3 (ARM: 7017/1: Use generic BUG() handler), reverting that
> commit resolves the issue for me.

This might solve the problem - could you check please?

 arch/arm/kernel/vmlinux.lds.S |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 7b2541e..20b3041 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -24,8 +24,10 @@
 #if (defined(CONFIG_SMP_ON_UP) && !defined(CONFIG_DEBUG_SPINLOCK)) || \
 	defined(CONFIG_GENERIC_BUG)
 #define ARM_EXIT_KEEP(x)	x
+#define ARM_EXIT_DISCARD(x)
 #else
 #define ARM_EXIT_KEEP(x)
+#define ARM_EXIT_DISCARD(x)	x
 #endif
 
 OUTPUT_ARCH(arm)
@@ -40,6 +42,11 @@ jiffies = jiffies_64 + 4;
 SECTIONS
 {
 	/*
+	 * XXX: The linker does not define how output sections are
+	 * assigned to input sections when there are multiple statements
+	 * matching the same input section name.  There is no documented
+	 * order of matching.
+	 *
 	 * unwind exit sections must be discarded before the rest of the
 	 * unwind sections get included.
 	 */
@@ -48,6 +55,9 @@ SECTIONS
 		*(.ARM.extab.exit.text)
 		ARM_CPU_DISCARD(*(.ARM.exidx.cpuexit.text))
 		ARM_CPU_DISCARD(*(.ARM.extab.cpuexit.text))
+		ARM_EXIT_DISCARD(EXIT_TEXT)
+		ARM_EXIT_DISCARD(EXIT_DATA)
+		EXIT_CALL
 #ifndef CONFIG_HOTPLUG
 		*(.ARM.exidx.devexit.text)
 		*(.ARM.extab.devexit.text)
@@ -59,6 +69,8 @@ SECTIONS
 #ifndef CONFIG_SMP_ON_UP
 		*(.alt.smp.init)
 #endif
+		*(.discard)
+		*(.discard.*)
 	}
 
 #ifdef CONFIG_XIP_KERNEL
@@ -280,9 +292,6 @@ SECTIONS
 
 	STABS_DEBUG
 	.comment 0 : { *(.comment) }
-
-	/* Default discards */
-	DISCARDS
 }
 
 /*

Link failures due to __bug_table in current -next

[Simon Glass]

Hi Russell,

On Mon, Sep 19, 2011 at 1:03 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Sep 19, 2011 at 01:09:54PM +0100, Mark Brown wrote:
>> I'm seeing linker failures in -next as of today:
>>
>> `.exit.text' referenced in section `__bug_table' of fs/built-in.o:
>> defined in discarded section `.exit.text' of fs/built-in.o
>> `.exit.text' referenced in section `__bug_table' of crypto/built-in.o:
>> defined in discarded section `.exit.text' of crypto/built-in.o
>> `.exit.text' referenced in section `__bug_table' of net/built-in.o:
>> defined in discarded section `.exit.text' of net/built-in.o
>> `.exit.text' referenced in section `__bug_table' of net/built-in.o:
>> defined in discarded section `.exit.text' of net/built-in.o
>>
>> which appears to be due to the chnage to use generic BUG() introduced in
>> commit 5254a3 (ARM: 7017/1: Use generic BUG() handler), reverting that
>> commit resolves the issue for me.

Gosh this does seem a bit odd. Ordering seems to be clearly implied by
the file syntax and I agree we should seek guidance from binutils
people.

I added the BUG condition to CONFIG_SMP_ON_UP and
CONFIG_DEBUG_SPINLOCK which were already there. If BUG is causing
problems, I wonder why these are not? Have we just been lucky, or have
I crossed a line? Or perhaps there are no spinlocks in exit text?

One option is to keep all exit text around - i.e. never discard it at
link time. From memory it is only 4-8KB. Doubtless many would be upset
with this, but it could be an option until this binutils behaviour is
resolved.

Another is to declare that it is a bug to use BUG in an exit section.
I was thinking about that at the time, but decided it was probably too
radical. There are only a small number of references in the kernel I
think (again from memory - this was back in April I think). Not
trivial to enforce, and the error you get is not exactly informative.

Yes another even stranger might be to ask the linker to leave the
references to discarded sections from the bug table unresolved, and
perhaps make them zero, like a weak reference. Is that even possible?
This would work since the bug scanning code can skip over these
entries. Even better if the linker could remove them but this is going
far beyond the job of a humble linker script.

While a bug-fix or feature change in binutils might get us there, I
worry that it would be years before it became widespread enough that
we could turn it on.

>
> This might solve the problem - could you check please?

Hope so. Thanks for your efforts on this. It would be nice not to have
to dump generic BUG due to this technicality.

Regards,
Simon

>
> ?arch/arm/kernel/vmlinux.lds.S | ? 15 ++++++++++++---
> ?1 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
> index 7b2541e..20b3041 100644
> --- a/arch/arm/kernel/vmlinux.lds.S
> +++ b/arch/arm/kernel/vmlinux.lds.S
> @@ -24,8 +24,10 @@
> ?#if (defined(CONFIG_SMP_ON_UP) && !defined(CONFIG_DEBUG_SPINLOCK)) || \
> ? ? ? ?defined(CONFIG_GENERIC_BUG)
> ?#define ARM_EXIT_KEEP(x) ? ? ? x
> +#define ARM_EXIT_DISCARD(x)
> ?#else
> ?#define ARM_EXIT_KEEP(x)
> +#define ARM_EXIT_DISCARD(x) ? ?x
> ?#endif
>
> ?OUTPUT_ARCH(arm)
> @@ -40,6 +42,11 @@ jiffies = jiffies_64 + 4;
> ?SECTIONS
> ?{
> ? ? ? ?/*
> + ? ? ? ?* XXX: The linker does not define how output sections are
> + ? ? ? ?* assigned to input sections when there are multiple statements
> + ? ? ? ?* matching the same input section name. ?There is no documented
> + ? ? ? ?* order of matching.
> + ? ? ? ?*
> ? ? ? ? * unwind exit sections must be discarded before the rest of the
> ? ? ? ? * unwind sections get included.
> ? ? ? ? */
> @@ -48,6 +55,9 @@ SECTIONS
> ? ? ? ? ? ? ? ?*(.ARM.extab.exit.text)
> ? ? ? ? ? ? ? ?ARM_CPU_DISCARD(*(.ARM.exidx.cpuexit.text))
> ? ? ? ? ? ? ? ?ARM_CPU_DISCARD(*(.ARM.extab.cpuexit.text))
> + ? ? ? ? ? ? ? ARM_EXIT_DISCARD(EXIT_TEXT)
> + ? ? ? ? ? ? ? ARM_EXIT_DISCARD(EXIT_DATA)
> + ? ? ? ? ? ? ? EXIT_CALL
> ?#ifndef CONFIG_HOTPLUG
> ? ? ? ? ? ? ? ?*(.ARM.exidx.devexit.text)
> ? ? ? ? ? ? ? ?*(.ARM.extab.devexit.text)
> @@ -59,6 +69,8 @@ SECTIONS
> ?#ifndef CONFIG_SMP_ON_UP
> ? ? ? ? ? ? ? ?*(.alt.smp.init)
> ?#endif
> + ? ? ? ? ? ? ? *(.discard)
> + ? ? ? ? ? ? ? *(.discard.*)
> ? ? ? ?}
>
> ?#ifdef CONFIG_XIP_KERNEL
> @@ -280,9 +292,6 @@ SECTIONS
>
> ? ? ? ?STABS_DEBUG
> ? ? ? ?.comment 0 : { *(.comment) }
> -
> - ? ? ? /* Default discards */
> - ? ? ? DISCARDS
> ?}
>
> ?/*
>
>

Link failures due to __bug_table in current -next

[Russell King - ARM Linux]

On Tue, Sep 20, 2011 at 12:06:22AM -0700, Simon Glass wrote:
> Hi Russell,
> 
> On Mon, Sep 19, 2011 at 1:03 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Mon, Sep 19, 2011 at 01:09:54PM +0100, Mark Brown wrote:
> >> I'm seeing linker failures in -next as of today:
> >>
> >> `.exit.text' referenced in section `__bug_table' of fs/built-in.o:
> >> defined in discarded section `.exit.text' of fs/built-in.o
> >> `.exit.text' referenced in section `__bug_table' of crypto/built-in.o:
> >> defined in discarded section `.exit.text' of crypto/built-in.o
> >> `.exit.text' referenced in section `__bug_table' of net/built-in.o:
> >> defined in discarded section `.exit.text' of net/built-in.o
> >> `.exit.text' referenced in section `__bug_table' of net/built-in.o:
> >> defined in discarded section `.exit.text' of net/built-in.o
> >>
> >> which appears to be due to the chnage to use generic BUG() introduced in
> >> commit 5254a3 (ARM: 7017/1: Use generic BUG() handler), reverting that
> >> commit resolves the issue for me.
> 
> Gosh this does seem a bit odd. Ordering seems to be clearly implied by
> the file syntax and I agree we should seek guidance from binutils
> people.

I'm not sure that there's any value in seeking guidance from the linker
folk - we can see what's going on with a few experiments.  That's fine
to find out what current linker behaviour is, but unless the manual
documents it, its something that shouldn't be relied upon.

Here's where I researched what the manual says and what practically happens
with the linker:

http://lists.arm.linux.org.uk/lurker/message/20110808.195805.a073e07d.en.html

> I added the BUG condition to CONFIG_SMP_ON_UP and
> CONFIG_DEBUG_SPINLOCK which were already there. If BUG is causing
> problems, I wonder why these are not? Have we just been lucky, or have
> I crossed a line? Or perhaps there are no spinlocks in exit text?

The other stuff is also having problems.  Rob Herring's report was about
the SMP alternatives causing the same problem:

http://lists.arm.linux.org.uk/lurker/message/20110808.184931.a38e1c4e.en.html

> One option is to keep all exit text around - i.e. never discard it at
> link time. From memory it is only 4-8KB. Doubtless many would be upset
> with this, but it could be an option until this binutils behaviour is
> resolved.

We are trying to keep .exit.text around (when certain config options are
set - and they are set - but the linker is deciding to discard it for us
anyway, because asm-generic/vmlinux.lds.S always lists .exit.text in its
discard section.

As we have a discard section at the beginning of the file to discard the
unwinder information for other sections, the one from the generic file
gets merged at the _start_ of the linker file, which results in .exit.text
first appearance to be in the discard section.

It's not that simple though - if you read the quote from the linker manual,
the implication is that the linker would be entirely free to discard an
input section as a priority if it appears in a discard section anywhere
in the linker script.  There's nothing to say future linkers won't do
this.  It would still be conformant to the linker manual.

> Another is to declare that it is a bug to use BUG in an exit section.
> I was thinking about that at the time, but decided it was probably too
> radical. There are only a small number of references in the kernel I
> think (again from memory - this was back in April I think). Not
> trivial to enforce, and the error you get is not exactly informative.

When a BUG() is inside an inline function which is used in an exit path,
it becomes non-trivial to eliminate.  That means there will be hidden
BUG() instances and we really can't ask people to avoid inline functions.

Link failures due to __bug_table in current -next

[Simon Glass]

Hi Russell,

On Tue, Sep 20, 2011 at 12:59 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Tue, Sep 20, 2011 at 12:06:22AM -0700, Simon Glass wrote:
>> Hi Russell,
>>
>> On Mon, Sep 19, 2011 at 1:03 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Mon, Sep 19, 2011 at 01:09:54PM +0100, Mark Brown wrote:
>> >> I'm seeing linker failures in -next as of today:
>> >>
>> >> `.exit.text' referenced in section `__bug_table' of fs/built-in.o:
>> >> defined in discarded section `.exit.text' of fs/built-in.o
>> >> `.exit.text' referenced in section `__bug_table' of crypto/built-in.o:
>> >> defined in discarded section `.exit.text' of crypto/built-in.o
>> >> `.exit.text' referenced in section `__bug_table' of net/built-in.o:
>> >> defined in discarded section `.exit.text' of net/built-in.o
>> >> `.exit.text' referenced in section `__bug_table' of net/built-in.o:
>> >> defined in discarded section `.exit.text' of net/built-in.o
>> >>
>> >> which appears to be due to the chnage to use generic BUG() introduced in
>> >> commit 5254a3 (ARM: 7017/1: Use generic BUG() handler), reverting that
>> >> commit resolves the issue for me.
>>
>> Gosh this does seem a bit odd. Ordering seems to be clearly implied by
>> the file syntax and I agree we should seek guidance from binutils
>> people.
>
> I'm not sure that there's any value in seeking guidance from the linker
> folk - we can see what's going on with a few experiments. ?That's fine
> to find out what current linker behaviour is, but unless the manual
> documents it, its something that shouldn't be relied upon.
>
> Here's where I researched what the manual says and what practically happens
> with the linker:
>
> http://lists.arm.linux.org.uk/lurker/message/20110808.195805.a073e07d.en.html

Yes that was what I read first (at least twice :-). It is
counter-intuitive given the way the linker encourages us to lay out
scripts - as I said I believe that some sort of ordering is at least
implied by the docs.

>
>> I added the BUG condition to CONFIG_SMP_ON_UP and
>> CONFIG_DEBUG_SPINLOCK which were already there. If BUG is causing
>> problems, I wonder why these are not? Have we just been lucky, or have
>> I crossed a line? Or perhaps there are no spinlocks in exit text?
>
> The other stuff is also having problems. ?Rob Herring's report was about
> the SMP alternatives causing the same problem:
>
> http://lists.arm.linux.org.uk/lurker/message/20110808.184931.a38e1c4e.en.html

OK I see.

>
>> One option is to keep all exit text around - i.e. never discard it at
>> link time. From memory it is only 4-8KB. Doubtless many would be upset
>> with this, but it could be an option until this binutils behaviour is
>> resolved.
>
> We are trying to keep .exit.text around (when certain config options are
> set - and they are set - but the linker is deciding to discard it for us
> anyway, because asm-generic/vmlinux.lds.S always lists .exit.text in its
> discard section.

Yes what I meant was to not discard it - i.e. remove it from that link
script altogether.

>
> As we have a discard section at the beginning of the file to discard the
> unwinder information for other sections, the one from the generic file
> gets merged at the _start_ of the linker file, which results in .exit.text
> first appearance to be in the discard section.
>
> It's not that simple though - if you read the quote from the linker manual,
> the implication is that the linker would be entirely free to discard an
> input section as a priority if it appears in a discard section anywhere
> in the linker script. ?There's nothing to say future linkers won't do
> this. ?It would still be conformant to the linker manual.

Oh dear. That is why it might be a good idea to hassle the linker
people, since relying on experiments on how things currently work
might be risky if someone leaps in and changes the algorithm.

>
>> Another is to declare that it is a bug to use BUG in an exit section.
>> I was thinking about that at the time, but decided it was probably too
>> radical. There are only a small number of references in the kernel I
>> think (again from memory - this was back in April I think). Not
>> trivial to enforce, and the error you get is not exactly informative.
>
> When a BUG() is inside an inline function which is used in an exit path,
> it becomes non-trivial to eliminate. ?That means there will be hidden
> BUG() instances and we really can't ask people to avoid inline functions.
>

You mean that the BUG() call is not obvious. However, it can be found,
if necessary by inspection of assembler output :-) An easy workaround
is to put that code into a non-__exit function and call it from the
__exit function. If we enforced that then I think it would be one
solution to the problem. Given the tiny size of exit code in the
kernel it can be hoped that the impact would be minimal. My main
concern with this approach is that it introduces a build problem which
will only occur on some machines and builds, which can be painful.

Sorry if I restate the obvious, but with generic BUG patch, BUG inside
an inline function (or anywhere else) just becomes an undef
instruction. There is no function call. The only reason we have a
problem at all is that we want to eliminate the code section
containing this undef instruction, but every one of these instructions
also creates an entry in the bug table (which exists in its own
separate section), and we cannot selectively eliminate those entries
in the linker. Specifically there is an entry which points to the PC
of the undef instruction. If this could be made a weak reference then
perhaps it might fix things.

It is easy enough for the handler to just not report the information
if it doesn't have it. This is hypothetical anyway since if you are
eliminating exit code it is presumably because you will never exit.

Hmm even more out there, I wonder if we can modify the BUG macro to
put the bug table entry into one of two separate depending on whether
BUG is in an __exit function or not? Then at link time, either concat
the two tables, or just ignore the exit one...

In any case, it sounds from the next email in this thread that your
patch has fixed the problem! So, where does that leave us?

Regards,
Simon

Link failures due to __bug_table in current -next

[Russell King - ARM Linux]

On Tue, Sep 20, 2011 at 10:00:06AM -0700, Simon Glass wrote:
> On Tue, Sep 20, 2011 at 12:59 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > It's not that simple though - if you read the quote from the linker manual,
> > the implication is that the linker would be entirely free to discard an
> > input section as a priority if it appears in a discard section anywhere
> > in the linker script. ?There's nothing to say future linkers won't do
> > this. ?It would still be conformant to the linker manual.
> 
> Oh dear. That is why it might be a good idea to hassle the linker
> people, since relying on experiments on how things currently work
> might be risky if someone leaps in and changes the algorithm.

Or we just ensure that we conform to the apparant looseness of the
manual, and make the addition of EXIT_TEXT etc in DISCARDS conditional
(which is effectively what I'm doing with my patch.)

That means we're no longer reliant on trusting the linker to do what
we want for this (we _do_ still trust it for the unwind information,
but I think that's less of an issue.)

> Hmm even more out there, I wonder if we can modify the BUG macro to
> put the bug table entry into one of two separate depending on whether
> BUG is in an __exit function or not? Then at link time, either concat
> the two tables, or just ignore the exit one...

That was the same thought for the SMP alternatives problem - I think
Nicolas proposed that there should be some way that the linker can
do sections based on the current section name.  That'd allow us to
have .alt.smp.exit.text and __bug_table.exit.text etc.

> In any case, it sounds from the next email in this thread that your
> patch has fixed the problem! So, where does that leave us?

I think we apply the patch, which resolves the problem, and point it
out to whoever looks after the asm-generic/vmlinux.lds.S file (Arnd?
I'll check when I re-dock the laptop later this evening.)  I suspect
Arnd is already reading some of these messages...

Link failures due to __bug_table in current -next

[Arnd Bergmann]

On Tuesday 20 September 2011, Russell King - ARM Linux wrote:
> On Tue, Sep 20, 2011 at 10:00:06AM -0700, Simon Glass wrote:
>
> That was the same thought for the SMP alternatives problem - I think
> Nicolas proposed that there should be some way that the linker can
> do sections based on the current section name.  That'd allow us to
> have .alt.smp.exit.text and __bug_table.exit.text etc.

Right. also *.pv_table, which is another section that has this problem.

> > In any case, it sounds from the next email in this thread that your
> > patch has fixed the problem! So, where does that leave us?
> 
> I think we apply the patch, which resolves the problem, and point it
> out to whoever looks after the asm-generic/vmlinux.lds.S file (Arnd?
> I'll check when I re-dock the laptop later this evening.)  I suspect
> Arnd is already reading some of these messages...

While I'm looking after asm-generic in general, I tend to just ack
the patches and let them go through someone else's tree most of
the time, because there are usually dependencies and the number of
patches for asm-generic is not high enough to require a tree on
its own.

For the vmlinux.lds.h file, I think the people that have contributed
to it the most are Sam Ravnborg and Tim Abbott (added to Cc).
The best way to get the right people to hear about it should be
to send the patch to linux-arch with these two on Cc.

	Arnd

Link failures due to __bug_table in current -next

[Nicolas Pitre]

On Tue, 20 Sep 2011, Russell King - ARM Linux wrote:

> On Tue, Sep 20, 2011 at 10:00:06AM -0700, Simon Glass wrote:
> > Hmm even more out there, I wonder if we can modify the BUG macro to
> > put the bug table entry into one of two separate depending on whether
> > BUG is in an __exit function or not? Then at link time, either concat
> > the two tables, or just ignore the exit one...
> 
> That was the same thought for the SMP alternatives problem - I think
> Nicolas proposed that there should be some way that the linker can
> do sections based on the current section name.  That'd allow us to
> have .alt.smp.exit.text and __bug_table.exit.text etc.

Initially that was proposed by Dave Martin, then I tried to push things 
ahead a bit in the Linaro toolchain work group.

The thread with the latest exchanges can be viewed here:
http://news.gmane.org/group/gmane.linux.linaro.toolchain/thread=1270

In a nutshell, it seems that enhancing gas to support this isn't 
trivial, and we are asked how beneficial (i.e. how much smaller the 
kernel image would be if all those sections could actually be split and 
discarded at link time) this would be to justify the effort.


Nicolas

Link failures due to __bug_table in current -next

[Simon Glass]

Hi Nicolas,

On Tue, Sep 20, 2011 at 12:57 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
> On Tue, 20 Sep 2011, Russell King - ARM Linux wrote:
>
>> On Tue, Sep 20, 2011 at 10:00:06AM -0700, Simon Glass wrote:
>> > Hmm even more out there, I wonder if we can modify the BUG macro to
>> > put the bug table entry into one of two separate depending on whether
>> > BUG is in an __exit function or not? Then at link time, either concat
>> > the two tables, or just ignore the exit one...
>>
>> That was the same thought for the SMP alternatives problem - I think
>> Nicolas proposed that there should be some way that the linker can
>> do sections based on the current section name. ?That'd allow us to
>> have .alt.smp.exit.text and __bug_table.exit.text etc.
>
> Initially that was proposed by Dave Martin, then I tried to push things
> ahead a bit in the Linaro toolchain work group.
>
> The thread with the latest exchanges can be viewed here:
> http://news.gmane.org/group/gmane.linux.linaro.toolchain/thread=1270

The thread is from July. Does that mean it is dead?

>
> In a nutshell, it seems that enhancing gas to support this isn't
> trivial, and we are asked how beneficial (i.e. how much smaller the
> kernel image would be if all those sections could actually be split and
> discarded at link time) this would be to justify the effort.

Well it's probably not a huge benefit in terms of kernel size. Exit
code tends to be relatively small, and many distributions use modules
extensively now anyway. But there is also the issue of the workarounds
in the lds file, and where this might lead in the years ahead.

Would it be easier / less intrusive to add a new .pushsubsection
directive which prepends the current section name to the name you give
it?

Regards,
Simon
>
>
> Nicolas
>

Link failures due to __bug_table in current -next

[Simon Glass]

Hi Russell,

On Tue, Sep 20, 2011 at 11:51 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Tue, Sep 20, 2011 at 10:00:06AM -0700, Simon Glass wrote:
>> On Tue, Sep 20, 2011 at 12:59 AM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > It's not that simple though - if you read the quote from the linker manual,
>> > the implication is that the linker would be entirely free to discard an
>> > input section as a priority if it appears in a discard section anywhere
>> > in the linker script. ?There's nothing to say future linkers won't do
>> > this. ?It would still be conformant to the linker manual.
>>
>> Oh dear. That is why it might be a good idea to hassle the linker
>> people, since relying on experiments on how things currently work
>> might be risky if someone leaps in and changes the algorithm.
>
> Or we just ensure that we conform to the apparant looseness of the
> manual, and make the addition of EXIT_TEXT etc in DISCARDS conditional
> (which is effectively what I'm doing with my patch.)

Yes that is safer for the moment.

>
> That means we're no longer reliant on trusting the linker to do what
> we want for this (we _do_ still trust it for the unwind information,
> but I think that's less of an issue.)
>
>> Hmm even more out there, I wonder if we can modify the BUG macro to
>> put the bug table entry into one of two separate depending on whether
>> BUG is in an __exit function or not? Then at link time, either concat
>> the two tables, or just ignore the exit one...
>
> That was the same thought for the SMP alternatives problem - I think
> Nicolas proposed that there should be some way that the linker can
> do sections based on the current section name. ?That'd allow us to
> have .alt.smp.exit.text and __bug_table.exit.text etc.

Yes

>
>> In any case, it sounds from the next email in this thread that your
>> patch has fixed the problem! So, where does that leave us?
>
> I think we apply the patch, which resolves the problem, and point it
> out to whoever looks after the asm-generic/vmlinux.lds.S file (Arnd?
> I'll check when I re-dock the laptop later this evening.) ?I suspect
> Arnd is already reading some of these messages...
>

Sounds good to me, and thanks for finding that solution.

Regards,
Simon

Link failures due to __bug_table in current -next

[Mark Brown]

On Mon, Sep 19, 2011 at 09:03:17PM +0100, Russell King - ARM Linux wrote:
> On Mon, Sep 19, 2011 at 01:09:54PM +0100, Mark Brown wrote:

> > `.exit.text' referenced in section `__bug_table' of net/built-in.o:
> > defined in discarded section `.exit.text' of net/built-in.o

> This might solve the problem - could you check please?

Yes, that works for me - thanks!