The following changes since commit 80f506918fdaaca6b574ba931536a58ce015c7be:
Linus Torvalds (1):
Merge branch 'for-linus' of git://git.kernel.dk/linux-2.6-block
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/frob/linux-2.6-roland.git topic/x86-lds-nits
Roland McGrath (1):
x86: linker script syntax nits
arch/x86/kernel/acpi/realmode/wakeup.lds.S | 4 ++--
arch/x86/kernel/vmlinux.lds.S | 17 ++++++++---------
2 files changed, 10 insertions(+), 11 deletions(-)
---
[PATCH] x86: linker script syntax nits
The linker scripts grew some use of weirdly wrong linker script syntax.
It happens to work, but it's not what the syntax is documented to be.
Clean it up to use the official syntax.
Signed-off-by: Roland McGrath <[email protected]>
CC: Ian Lance Taylor <[email protected]>
---
arch/x86/kernel/acpi/realmode/wakeup.lds.S | 4 ++--
arch/x86/kernel/vmlinux.lds.S | 17 ++++++++---------
2 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/arch/x86/kernel/acpi/realmode/wakeup.lds.S b/arch/x86/kernel/acpi/realmode/wakeup.lds.S
index 7da00b7..0e50e1e 100644
--- a/arch/x86/kernel/acpi/realmode/wakeup.lds.S
+++ b/arch/x86/kernel/acpi/realmode/wakeup.lds.S
@@ -56,6 +56,6 @@ SECTIONS
/DISCARD/ : {
*(.note*)
}
-
- . = ASSERT(_end <= WAKEUP_SIZE, "Wakeup too big!");
}
+
+ASSERT(_end <= WAKEUP_SIZE, "Wakeup too big!");
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 92929fb..8d6001a 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -305,8 +305,8 @@ SECTIONS
#ifdef CONFIG_X86_32
-. = ASSERT((_end - LOAD_OFFSET <= KERNEL_IMAGE_SIZE),
- "kernel image bigger than KERNEL_IMAGE_SIZE");
+ASSERT((_end - LOAD_OFFSET <= KERNEL_IMAGE_SIZE),
+ "kernel image bigger than KERNEL_IMAGE_SIZE");
#else
/*
* Per-cpu symbols which need to be offset from __per_cpu_load
@@ -319,12 +319,12 @@ INIT_PER_CPU(irq_stack_union);
/*
* Build-time check on the image size:
*/
-. = ASSERT((_end - _text <= KERNEL_IMAGE_SIZE),
- "kernel image bigger than KERNEL_IMAGE_SIZE");
+ASSERT((_end - _text <= KERNEL_IMAGE_SIZE),
+ "kernel image bigger than KERNEL_IMAGE_SIZE");
#ifdef CONFIG_SMP
-. = ASSERT((per_cpu__irq_stack_union == 0),
- "irq_stack_union is not at start of per-cpu area");
+ASSERT((per_cpu__irq_stack_union == 0),
+ "irq_stack_union is not at start of per-cpu area");
#endif
#endif /* CONFIG_X86_32 */
@@ -332,7 +332,6 @@ INIT_PER_CPU(irq_stack_union);
#ifdef CONFIG_KEXEC
#include <asm/kexec.h>
-. = ASSERT(kexec_control_code_size <= KEXEC_CONTROL_CODE_MAX_SIZE,
- "kexec control code size is too big");
+ASSERT(kexec_control_code_size <= KEXEC_CONTROL_CODE_MAX_SIZE,
+ "kexec control code size is too big");
#endif
-
On 10/14/2009 03:10 PM, Roland McGrath wrote:
> The following changes since commit 80f506918fdaaca6b574ba931536a58ce015c7be:
> Linus Torvalds (1):
> Merge branch 'for-linus' of git://git.kernel.dk/linux-2.6-block
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/frob/linux-2.6-roland.git topic/x86-lds-nits
>
> Roland McGrath (1):
> x86: linker script syntax nits
>
> arch/x86/kernel/acpi/realmode/wakeup.lds.S | 4 ++--
> arch/x86/kernel/vmlinux.lds.S | 17 ++++++++---------
> 2 files changed, 10 insertions(+), 11 deletions(-)
Severely-Nacked-by: H. Peter Anvin <[email protected]>
> - . = ASSERT(_end <= WAKEUP_SIZE, "Wakeup too big!");
> }
> +
> +ASSERT(_end <= WAKEUP_SIZE, "Wakeup too big!");
Sinkless asserts are broken in older versions of ld.
-hpa
On Wed, Oct 14, 2009 at 03:10:24PM -0700, Roland McGrath wrote:
> The following changes since commit 80f506918fdaaca6b574ba931536a58ce015c7be:
> Linus Torvalds (1):
> Merge branch 'for-linus' of git://git.kernel.dk/linux-2.6-block
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/frob/linux-2.6-roland.git topic/x86-lds-nits
>
> Roland McGrath (1):
> x86: linker script syntax nits
>
> arch/x86/kernel/acpi/realmode/wakeup.lds.S | 4 ++--
> arch/x86/kernel/vmlinux.lds.S | 17 ++++++++---------
> 2 files changed, 10 insertions(+), 11 deletions(-)
> ---
> [PATCH] x86: linker script syntax nits
>
> The linker scripts grew some use of weirdly wrong linker script syntax.
> It happens to work, but it's not what the syntax is documented to be.
> Clean it up to use the official syntax.
>
> Signed-off-by: Roland McGrath <[email protected]>
> CC: Ian Lance Taylor <[email protected]>
> ---
> arch/x86/kernel/acpi/realmode/wakeup.lds.S | 4 ++--
> arch/x86/kernel/vmlinux.lds.S | 17 ++++++++---------
> 2 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/kernel/acpi/realmode/wakeup.lds.S b/arch/x86/kernel/acpi/realmode/wakeup.lds.S
> index 7da00b7..0e50e1e 100644
> --- a/arch/x86/kernel/acpi/realmode/wakeup.lds.S
> +++ b/arch/x86/kernel/acpi/realmode/wakeup.lds.S
> @@ -56,6 +56,6 @@ SECTIONS
> /DISCARD/ : {
> *(.note*)
> }
> -
> - . = ASSERT(_end <= WAKEUP_SIZE, "Wakeup too big!");
> }
> +
> +ASSERT(_end <= WAKEUP_SIZE, "Wakeup too big!");
This breaks with older binutils.
See d2ba8b211bb8abc29aa627dbd4dce08cfbc8082b for reference.
Same goes for the other cahnges in this post.
Yup - it looks ugly :-(
Sam
Commit-ID: db8590f5043f3436a65b24155a3a7af2604df876
Gitweb: http://git.kernel.org/tip/db8590f5043f3436a65b24155a3a7af2604df876
Author: Ingo Molnar <[email protected]>
AuthorDate: Thu, 15 Oct 2009 08:08:12 +0200
Committer: Ingo Molnar <[email protected]>
CommitDate: Thu, 15 Oct 2009 08:09:55 +0200
Revert "x86: linker script syntax nits"
This reverts commit e9a63a4e559fbdc522072281d05e6b13c1022f4b.
This breaks older binutils, where sink-less asserts are broken.
See this commit for further details:
d2ba8b2: x86: Fix assert syntax in vmlinux.lds.S
Acked-by: "H. Peter Anvin" <[email protected]>
Acked-by: Sam Ravnborg <[email protected]>
Cc: Linus Torvalds <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/acpi/realmode/wakeup.lds.S | 4 ++--
arch/x86/kernel/vmlinux.lds.S | 17 +++++++++--------
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/arch/x86/kernel/acpi/realmode/wakeup.lds.S b/arch/x86/kernel/acpi/realmode/wakeup.lds.S
index 0e50e1e..7da00b7 100644
--- a/arch/x86/kernel/acpi/realmode/wakeup.lds.S
+++ b/arch/x86/kernel/acpi/realmode/wakeup.lds.S
@@ -56,6 +56,6 @@ SECTIONS
/DISCARD/ : {
*(.note*)
}
-}
-ASSERT(_end <= WAKEUP_SIZE, "Wakeup too big!");
+ . = ASSERT(_end <= WAKEUP_SIZE, "Wakeup too big!");
+}
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 8d6001a..92929fb 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -305,8 +305,8 @@ SECTIONS
#ifdef CONFIG_X86_32
-ASSERT((_end - LOAD_OFFSET <= KERNEL_IMAGE_SIZE),
- "kernel image bigger than KERNEL_IMAGE_SIZE");
+. = ASSERT((_end - LOAD_OFFSET <= KERNEL_IMAGE_SIZE),
+ "kernel image bigger than KERNEL_IMAGE_SIZE");
#else
/*
* Per-cpu symbols which need to be offset from __per_cpu_load
@@ -319,12 +319,12 @@ INIT_PER_CPU(irq_stack_union);
/*
* Build-time check on the image size:
*/
-ASSERT((_end - _text <= KERNEL_IMAGE_SIZE),
- "kernel image bigger than KERNEL_IMAGE_SIZE");
+. = ASSERT((_end - _text <= KERNEL_IMAGE_SIZE),
+ "kernel image bigger than KERNEL_IMAGE_SIZE");
#ifdef CONFIG_SMP
-ASSERT((per_cpu__irq_stack_union == 0),
- "irq_stack_union is not at start of per-cpu area");
+. = ASSERT((per_cpu__irq_stack_union == 0),
+ "irq_stack_union is not at start of per-cpu area");
#endif
#endif /* CONFIG_X86_32 */
@@ -332,6 +332,7 @@ ASSERT((per_cpu__irq_stack_union == 0),
#ifdef CONFIG_KEXEC
#include <asm/kexec.h>
-ASSERT(kexec_control_code_size <= KEXEC_CONTROL_CODE_MAX_SIZE,
- "kexec control code size is too big");
+. = ASSERT(kexec_control_code_size <= KEXEC_CONTROL_CODE_MAX_SIZE,
+ "kexec control code size is too big");
#endif
+
* Roland McGrath <[email protected]> wrote:
> The following changes since commit 80f506918fdaaca6b574ba931536a58ce015c7be:
> Linus Torvalds (1):
> Merge branch 'for-linus' of git://git.kernel.dk/linux-2.6-block
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/frob/linux-2.6-roland.git topic/x86-lds-nits
>
> Roland McGrath (1):
> x86: linker script syntax nits
>
> arch/x86/kernel/acpi/realmode/wakeup.lds.S | 4 ++--
> arch/x86/kernel/vmlinux.lds.S | 17 ++++++++---------
> 2 files changed, 10 insertions(+), 11 deletions(-)
> ---
> [PATCH] x86: linker script syntax nits
>
> The linker scripts grew some use of weirdly wrong linker script syntax.
> It happens to work, but it's not what the syntax is documented to be.
> Clean it up to use the official syntax.
I've queued up a revert based on Peter's and Sam's objections.
I'd also like to point out a few commit log quality nits. A standard x86
commit should have the following title:
x86: Clean up linker script syntax nits
not:
x86: linker script syntax nits
Note that we always try to add a verb to that sentence, so that it's
more natural-language alike. (also note the capitalization (we
capitalize the sentence after the 'x86:' prefix))
(Plus i always try to line-wrap the commit log to 65 columns, to make it
easy to quote to several levels and still be within the various viewer
column widths that people use - and to allow for the +4 columns used up
by git log's indentation.)
Thanks,
Ingo
* Sam Ravnborg <[email protected]> wrote:
> On Wed, Oct 14, 2009 at 03:10:24PM -0700, Roland McGrath wrote:
> > The following changes since commit 80f506918fdaaca6b574ba931536a58ce015c7be:
> > Linus Torvalds (1):
> > Merge branch 'for-linus' of git://git.kernel.dk/linux-2.6-block
> >
> > are available in the git repository at:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/frob/linux-2.6-roland.git topic/x86-lds-nits
> >
> > Roland McGrath (1):
> > x86: linker script syntax nits
> >
> > arch/x86/kernel/acpi/realmode/wakeup.lds.S | 4 ++--
> > arch/x86/kernel/vmlinux.lds.S | 17 ++++++++---------
> > 2 files changed, 10 insertions(+), 11 deletions(-)
> > ---
> > [PATCH] x86: linker script syntax nits
> >
> > The linker scripts grew some use of weirdly wrong linker script syntax.
> > It happens to work, but it's not what the syntax is documented to be.
> > Clean it up to use the official syntax.
> >
> > Signed-off-by: Roland McGrath <[email protected]>
> > CC: Ian Lance Taylor <[email protected]>
> > ---
> > arch/x86/kernel/acpi/realmode/wakeup.lds.S | 4 ++--
> > arch/x86/kernel/vmlinux.lds.S | 17 ++++++++---------
> > 2 files changed, 10 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/x86/kernel/acpi/realmode/wakeup.lds.S b/arch/x86/kernel/acpi/realmode/wakeup.lds.S
> > index 7da00b7..0e50e1e 100644
> > --- a/arch/x86/kernel/acpi/realmode/wakeup.lds.S
> > +++ b/arch/x86/kernel/acpi/realmode/wakeup.lds.S
> > @@ -56,6 +56,6 @@ SECTIONS
> > /DISCARD/ : {
> > *(.note*)
> > }
> > -
> > - . = ASSERT(_end <= WAKEUP_SIZE, "Wakeup too big!");
> > }
> > +
> > +ASSERT(_end <= WAKEUP_SIZE, "Wakeup too big!");
>
> This breaks with older binutils. See
> d2ba8b211bb8abc29aa627dbd4dce08cfbc8082b for reference.
>
> Same goes for the other cahnges in this post.
>
> Yup - it looks ugly :-(
We could introduce a COMPAT_ASSERT() wrapper perhaps, to move it more in
line with the 'official' syntax.
Or we could wrap ASSERT() itself (this runs through the preprocessor
before going to the linker) - although that would be a pretty obfuscated
move.
At minimum we should add a comment to the first use of ASSERT() here
that we assign the current address due to compatibility reasons. (same
goes for arch/x86/boot/setup.ld)
Anyway - any such cleanup would be .33 material.
Ingo
On 10/14/2009 11:38 PM, Ingo Molnar wrote:
>
> We could introduce a COMPAT_ASSERT() wrapper perhaps, to move it more in
> line with the 'official' syntax.
>
> Or we could wrap ASSERT() itself (this runs through the preprocessor
> before going to the linker) - although that would be a pretty obfuscated
> move.
>
> At minimum we should add a comment to the first use of ASSERT() here
> that we assign the current address due to compatibility reasons. (same
> goes for arch/x86/boot/setup.ld)
>
> Anyway - any such cleanup would be .33 material.
>
The tricky part about wrapping ASSERT() is that you have to have a sink
for the old versions of ld. We use . as the sink, but that changes .
which you don't want to do silently.
The other -- and somewhat less cantankerous way -- is to use a dummy symbol:
#define LINUX_ASSERT(expr, name, string) name = ASSERT(expr, string)
"name" would have to be unique for each instance.
-hpa
> The other -- and somewhat less cantankerous way -- is to use a dummy symbol:
>
> #define LINUX_ASSERT(expr, name, string) name = ASSERT(expr, string)
>
> "name" would have to be unique for each instance.
I wouldn't like to pollute the symtabs with those useless symbols.
#define LINUX_ASSERT(expr, string) PROVIDE(_assert_bogon = ASSERT(expr, string))
works with a recent ld. I don't know what the problematic old ld versions
were. Do you have such on hand to test?
Ian may have another suggestion.
Thanks,
Roland
On 10/15/2009 11:26 AM, Roland McGrath wrote:
>> The other -- and somewhat less cantankerous way -- is to use a dummy symbol:
>>
>> #define LINUX_ASSERT(expr, name, string) name = ASSERT(expr, string)
>>
>> "name" would have to be unique for each instance.
>
> I wouldn't like to pollute the symtabs with those useless symbols.
>
> #define LINUX_ASSERT(expr, string) PROVIDE(_assert_bogon = ASSERT(expr, string))
>
> works with a recent ld. I don't know what the problematic old ld versions
> were. Do you have such on hand to test?
>
> Ian may have another suggestion.
>
I don't remember how old they have to be... I think RHEL 4 timeframe,
though.
-hpa
On Thu, Oct 15, 2009 at 11:26:16AM -0700, Roland McGrath wrote:
> > The other -- and somewhat less cantankerous way -- is to use a dummy symbol:
> >
> > #define LINUX_ASSERT(expr, name, string) name = ASSERT(expr, string)
> >
> > "name" would have to be unique for each instance.
>
> I wouldn't like to pollute the symtabs with those useless symbols.
>
> #define LINUX_ASSERT(expr, string) PROVIDE(_assert_bogon = ASSERT(expr, string))
>
> works with a recent ld. I don't know what the problematic old ld versions
> were. Do you have such on hand to test?
Jean reported binutils 2.14.90.0.6.
See: http://marc.info/?l=linux-kbuild&m=124930110427870&w=2
Sam
Commit-ID: a5912f6b3e20c137172460e6d4dd180866c00963
Gitweb: http://git.kernel.org/tip/a5912f6b3e20c137172460e6d4dd180866c00963
Author: Ingo Molnar <[email protected]>
AuthorDate: Fri, 16 Oct 2009 07:18:46 +0200
Committer: Ingo Molnar <[email protected]>
CommitDate: Fri, 16 Oct 2009 07:18:46 +0200
x86: Document linker script ASSERT() quirk
Older binutils breaks if ASSERT() is used without a sink
for the output.
For example 2.14.90.0.6 is known to be broken, the link
fails with:
LD .tmp_vmlinux1
ld:arch/x86/kernel/vmlinux.lds:678: parse error
Document this quirk in all three files that use it.
See: http://marc.info/?l=linux-kbuild&m=124930110427870&w=2
See[2]: d2ba8b2 ("x86: Fix assert syntax in vmlinux.lds.S")
Cc: Linus Torvalds <[email protected]>
Cc: Roland McGrath <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Sam Ravnborg <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/boot/setup.ld | 3 +++
arch/x86/kernel/acpi/realmode/wakeup.lds.S | 3 +++
arch/x86/kernel/vmlinux.lds.S | 3 +++
3 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld
index 0f6ec45..03c0683 100644
--- a/arch/x86/boot/setup.ld
+++ b/arch/x86/boot/setup.ld
@@ -53,6 +53,9 @@ SECTIONS
/DISCARD/ : { *(.note*) }
+ /*
+ * The ASSERT() sink to . is intentional, for binutils 2.14 compatibility:
+ */
. = ASSERT(_end <= 0x8000, "Setup too big!");
. = ASSERT(hdr == 0x1f1, "The setup header has the wrong offset!");
/* Necessary for the very-old-loader check to work... */
diff --git a/arch/x86/kernel/acpi/realmode/wakeup.lds.S b/arch/x86/kernel/acpi/realmode/wakeup.lds.S
index 7da00b7..060fff8 100644
--- a/arch/x86/kernel/acpi/realmode/wakeup.lds.S
+++ b/arch/x86/kernel/acpi/realmode/wakeup.lds.S
@@ -57,5 +57,8 @@ SECTIONS
*(.note*)
}
+ /*
+ * The ASSERT() sink to . is intentional, for binutils 2.14 compatibility:
+ */
. = ASSERT(_end <= WAKEUP_SIZE, "Wakeup too big!");
}
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 92929fb..3c68fe2 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -305,6 +305,9 @@ SECTIONS
#ifdef CONFIG_X86_32
+/*
+ * The ASSERT() sink to . is intentional, for binutils 2.14 compatibility:
+ */
. = ASSERT((_end - LOAD_OFFSET <= KERNEL_IMAGE_SIZE),
"kernel image bigger than KERNEL_IMAGE_SIZE");
#else