[REGRESSION] um: CONFIG_STATIC_LINK=y broken

[REGRESSION] um: CONFIG_STATIC_LINK=y broken

[richard -rw- weinberger]

Hi,

CONFIG_STATIC_LINK=y is broken since 2.6.32.
The linux binary segfaults immediately.

This patch introduced the regression (bisected):
5d150a97f9391f5bcd7ba0d59d7a11c3de3cea80 is first bad commit
commit 5d150a97f9391f5bcd7ba0d59d7a11c3de3cea80
Author: Tim Abbott <tabbott@ksplice.com>
Date:   Thu Sep 24 10:36:20 2009 -0400

    um: Clean up linker script using standard macros.

    Signed-off-by: Tim Abbott <tabbott@ksplice.com>
    Cc: Jeff Dike <jdike@addtoit.com>
    Cc: user-mode-linux-devel@lists.sourceforge.net
    Acked-by: Sam Ravnborg <sam@ravnborg.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

:040000 040000 43c1b7afe756beb0dc5073195916d54ac41e7546
fe33dda7c1b15c61a6a65195cc6522beb25e7ba2 M      arch

Cheers,
//richard

Re: [REGRESSION] um: CONFIG_STATIC_LINK=y broken

[Tim Abbott]

The individual chunks of that patch are all independent; could you 
determine which one of the changes causes the problem?

	-Tim Abbott

On Tue, 22 Dec 2009, richard -rw- weinberger wrote:

> Hi,
> 
> CONFIG_STATIC_LINK=y is broken since 2.6.32.
> The linux binary segfaults immediately.
> 
> This patch introduced the regression (bisected):
> 5d150a97f9391f5bcd7ba0d59d7a11c3de3cea80 is first bad commit
> commit 5d150a97f9391f5bcd7ba0d59d7a11c3de3cea80
> Author: Tim Abbott <tabbott@ksplice.com>
> Date:   Thu Sep 24 10:36:20 2009 -0400
> 
>     um: Clean up linker script using standard macros.
> 
>     Signed-off-by: Tim Abbott <tabbott@ksplice.com>
>     Cc: Jeff Dike <jdike@addtoit.com>
>     Cc: user-mode-linux-devel@lists.sourceforge.net
>     Acked-by: Sam Ravnborg <sam@ravnborg.org>
>     Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> 
> :040000 040000 43c1b7afe756beb0dc5073195916d54ac41e7546
> fe33dda7c1b15c61a6a65195cc6522beb25e7ba2 M      arch
> 
> Cheers,
> //richard
> 

Re: [REGRESSION] um: CONFIG_STATIC_LINK=y broken

[richard -rw- weinberger]

This is the bad changeset:

diff --git a/arch/um/kernel/uml.lds.S b/arch/um/kernel/uml.lds.S
index 2ebd397..e7a6cca 100644
--- a/arch/um/kernel/uml.lds.S
+++ b/arch/um/kernel/uml.lds.S
@@ -22,11 +22,7 @@ SECTIONS
   _text = .;
   _stext = .;
   __init_begin = .;
-  .init.text : {
- _sinittext = .;
- INIT_TEXT
- _einittext = .;
-  }
+  INIT_TEXT_SECTION(PAGE_SIZE)
   . = ALIGN(PAGE_SIZE);

   .text      :

//richard
2009/12/22, Tim Abbott <tabbott@ksplice.com>:
> The individual chunks of that patch are all independent; could you
> determine which one of the changes causes the problem?
>
> 	-Tim Abbott
>
> On Tue, 22 Dec 2009, richard -rw- weinberger wrote:
>
>> Hi,
>>
>> CONFIG_STATIC_LINK=y is broken since 2.6.32.
>> The linux binary segfaults immediately.
>>
>> This patch introduced the regression (bisected):
>> 5d150a97f9391f5bcd7ba0d59d7a11c3de3cea80 is first bad commit
>> commit 5d150a97f9391f5bcd7ba0d59d7a11c3de3cea80
>> Author: Tim Abbott <tabbott@ksplice.com>
>> Date:   Thu Sep 24 10:36:20 2009 -0400
>>
>>     um: Clean up linker script using standard macros.
>>
>>     Signed-off-by: Tim Abbott <tabbott@ksplice.com>
>>     Cc: Jeff Dike <jdike@addtoit.com>
>>     Cc: user-mode-linux-devel@lists.sourceforge.net
>>     Acked-by: Sam Ravnborg <sam@ravnborg.org>
>>     Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
>>
>> :040000 040000 43c1b7afe756beb0dc5073195916d54ac41e7546
>> fe33dda7c1b15c61a6a65195cc6522beb25e7ba2 M      arch
>>
>> Cheers,
>> //richard
>>
>

Re: [REGRESSION] um: CONFIG_STATIC_LINK=y broken

[Tim Abbott]

On Tue, 22 Dec 2009, richard -rw- weinberger wrote:

> This is the bad changeset:

Thanks for tracking that down.  INIT_TEXT_SECTION is:

#define INIT_TEXT_SECTION(inittext_align)                               \
        . = ALIGN(inittext_align);                                      \
        .init.text : AT(ADDR(.init.text) - LOAD_OFFSET) {               \
                VMLINUX_SYMBOL(_sinittext) = .;                         \
                INIT_TEXT                                               \
                VMLINUX_SYMBOL(_einittext) = .;                         \
        }

So there are only 3 code changes here:
(1) wrapping _sinittext and _einittext in VMLINUX_SYMBOL
(2) Adding the AT(ADDR(.init.text) - LOAD_OFFSET)
(3) The added ALIGN(PAGE_SIZE) before the start of .init.text.

I don't yet see why any of these would be problematic; would you be 
willing to try them and figure out the precise cause?

I suspect it'd be easiest for you to try those individual changes 
interactively, but if it's not trivial for you, I'd be happy to generate a 
patch series splitting out this change into pieces for you to bisect.

	-Tim Abbott

Re: [REGRESSION] um: CONFIG_STATIC_LINK=y broken

[richard -rw- weinberger]

Adding the ALIGN(PAGE_SIZE) causes the segfault.
But I cannot tell you why. :-(

Cheers,
//richard

2009/12/22, Tim Abbott <tabbott@ksplice.com>:
> On Tue, 22 Dec 2009, richard -rw- weinberger wrote:
>
>> This is the bad changeset:
>
> Thanks for tracking that down.  INIT_TEXT_SECTION is:
>
> #define INIT_TEXT_SECTION(inittext_align)                               \
>         . = ALIGN(inittext_align);                                      \
>         .init.text : AT(ADDR(.init.text) - LOAD_OFFSET) {               \
>                 VMLINUX_SYMBOL(_sinittext) = .;                         \
>                 INIT_TEXT                                               \
>                 VMLINUX_SYMBOL(_einittext) = .;                         \
>         }
>
> So there are only 3 code changes here:
> (1) wrapping _sinittext and _einittext in VMLINUX_SYMBOL
> (2) Adding the AT(ADDR(.init.text) - LOAD_OFFSET)
> (3) The added ALIGN(PAGE_SIZE) before the start of .init.text.
>
> I don't yet see why any of these would be problematic; would you be
> willing to try them and figure out the precise cause?
>
> I suspect it'd be easiest for you to try those individual changes
> interactively, but if it's not trivial for you, I'd be happy to generate a
> patch series splitting out this change into pieces for you to bisect.
>
> 	-Tim Abbott
>

Re: [REGRESSION] um: CONFIG_STATIC_LINK=y broken

[Tim Abbott]

On Tue, 22 Dec 2009, richard -rw- weinberger wrote:

> Adding the ALIGN(PAGE_SIZE) causes the segfault.
> But I cannot tell you why. :-(

OK, then the following patch should fix the regression by reverting that 
unintentional change.

Someone should probably determine why the ALIGN causes a segfault, since 
that is probably a bug, but I don't have time to do that investigation.

Richard, can you test that this patch fixes the issue for you?

(adding the UML maintainer and list to the thread).

	-Tim Abbott

um: remove PAGE_SIZE alignment in linker script causing kernel segfault.

The linker script cleanup that I did in commit 
5d150a97f9391f5bcd7ba0d59d7a11c3de3cea80 introduced an ALIGN(PAGE_SIZE) 
when using INIT_TEXT_SECTION; this apparently causes the kernel to 
segfault with CONFIG_STATIC_LINK=y. 

I'm not certain why this would cause the kernel to segfault, but it seems 
likely it is because previously it was the case that

__init_begin = _stext = _text = _sinittext 

and with the extra ALIGN(PAGE_SIZE), _sinittext becomes different.

Signed-off-by: Tim Abbott <tabbott@ksplice.com>
Reported-by: richard -rw- weinberger <richard.weinberger@gmail.com>
Cc: Jeff Dike <jdike@addtoit.com>
Cc: user-mode-linux-devel@lists.sourceforge.net
---
 arch/um/kernel/uml.lds.S |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/um/kernel/uml.lds.S b/arch/um/kernel/uml.lds.S
index e7a6cca..664f942 100644
--- a/arch/um/kernel/uml.lds.S
+++ b/arch/um/kernel/uml.lds.S
@@ -22,7 +22,7 @@ SECTIONS
   _text = .;
   _stext = .;
   __init_begin = .;
-  INIT_TEXT_SECTION(PAGE_SIZE)
+  INIT_TEXT_SECTION(0)
   . = ALIGN(PAGE_SIZE);
 
   .text      :
-- 
1.6.5.7

Re: [REGRESSION] um: CONFIG_STATIC_LINK=y broken

[richard -rw- weinberger]

2009/12/22, Tim Abbott <tabbott@ksplice.com>:
> Richard, can you test that this patch fixes the issue for you?

Using INIT_TEXT_SECTION(0) instead of INIT_TEXT_SECTION(PAGE_SIZE)
works fine here.
(Tested with 2.6.33-rc1 and 2.6.32)

//richard

[REGRESSION] um: CONFIG_STATIC_LINK=y broken

[Tim Abbott]

Hi Linus,

The following patch fixes a regression that I caused in 2.6.32 when 
cleaning up the um architecture's linker scripts.

I've not heard anything from the um maintainers (they have had since 
Richard Weinberger reported that this patch fixed the problem on December 
22), so I'm sending this to you now (and CCing stable@ since it affects 
2.6.32).

	-Tim Abbott

--

um: remove PAGE_SIZE alignment in linker script causing kernel segfault.

The linker script cleanup that I did in commit 
5d150a97f9391f5bcd7ba0d59d7a11c3de3cea80 accidentally introduced an 
ALIGN(PAGE_SIZE) when converting to use INIT_TEXT_SECTION; Richard 
Weinberger reported that this causes the kernel to segfault with 
CONFIG_STATIC_LINK=y.

I'm not certain why this extra alignment is a problem, but it seems likely 
it is because previously

__init_begin = _stext = _text = _sinittext 

and with the extra ALIGN(PAGE_SIZE), _sinittext becomes different from the 
rest.  So there is likely a bug here where something is assuming that 
_sinittext is the same as one of those other symbols.  But reverting the 
accidental change fixes the regression, so it seems worth committing that 
now.

Signed-off-by: Tim Abbott <tabbott@ksplice.com>
Reported-by: richard -rw- weinberger <richard.weinberger@gmail.com>
Cc: Jeff Dike <jdike@addtoit.com>
Cc: user-mode-linux-devel@lists.sourceforge.net
---
 arch/um/kernel/uml.lds.S |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/um/kernel/uml.lds.S b/arch/um/kernel/uml.lds.S
index e7a6cca..664f942 100644
--- a/arch/um/kernel/uml.lds.S
+++ b/arch/um/kernel/uml.lds.S
@@ -22,7 +22,7 @@ SECTIONS
   _text = .;
   _stext = .;
   __init_begin = .;
-  INIT_TEXT_SECTION(PAGE_SIZE)
+  INIT_TEXT_SECTION(0)
   . = ALIGN(PAGE_SIZE);
 
   .text      :
-- 
1.6.5.7