* build failure in Linus' tree with gcc 4.4.3
@ 2012-02-20 2:39 Stephen Rothwell
2012-02-20 3:22 ` Stephen Rothwell
0 siblings, 1 reply; 8+ messages in thread
From: Stephen Rothwell @ 2012-02-20 2:39 UTC (permalink / raw)
To: Matt Fleming; +Cc: H. Peter Anvin, LKML
[-- Attachment #1: Type: text/plain, Size: 1037 bytes --]
Hi all,
My x86_64 allmodconfig build fail like this:
OBJCOPY arch/x86/boot/vmlinux.bin
AS arch/x86/boot/header.o
LD arch/x86/boot/setup.elf
OBJCOPY arch/x86/boot/setup.bin
BUILD arch/x86/boot/bzImage
Setup is 14768 bytes (padded to 14848 bytes).
System is 4766 kB
/bin/sh: line 1: 20126 Segmentation fault arch/x86/boot/tools/build arch/x86/boot/setup.bin arch/x86/boot/vmlinux.bin > arch/x86/boot/bzImage
I have bisected it down to commit
291f36325f9f252bd76ef5f603995f37e453fc60 ("x86, efi: EFI boot stub
support"). This may well be a compiler bug that this commit just happens
to poke.
The toolchain is PowerPC hosted cross tools:
x86_64-linux-gcc (GCC) 4.4.3
GNU ld (GNU Binutils) 2.20
I tried an allmodconfig build with my native compiler (gcc (Debian
4.6.2-14) 4.6.2, GNU gold (GNU Binutils for Debian 2.22) 1.11) and it
worked fine.
That is a large commit, so I am not sure where to go from here.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: build failure in Linus' tree with gcc 4.4.3 2012-02-20 2:39 build failure in Linus' tree with gcc 4.4.3 Stephen Rothwell @ 2012-02-20 3:22 ` Stephen Rothwell 2012-02-20 9:43 ` Matt Fleming 0 siblings, 1 reply; 8+ messages in thread From: Stephen Rothwell @ 2012-02-20 3:22 UTC (permalink / raw) To: Matt Fleming; +Cc: H. Peter Anvin, LKML [-- Attachment #1: Type: text/plain, Size: 3000 bytes --] Hi all, On Mon, 20 Feb 2012 13:39:36 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote: > > Hi all, > > My x86_64 allmodconfig build fail like this: > > OBJCOPY arch/x86/boot/vmlinux.bin > AS arch/x86/boot/header.o > LD arch/x86/boot/setup.elf > OBJCOPY arch/x86/boot/setup.bin > BUILD arch/x86/boot/bzImage > Setup is 14768 bytes (padded to 14848 bytes). > System is 4766 kB > /bin/sh: line 1: 20126 Segmentation fault arch/x86/boot/tools/build arch/x86/boot/setup.bin arch/x86/boot/vmlinux.bin > arch/x86/boot/bzImage > > I have bisected it down to commit > 291f36325f9f252bd76ef5f603995f37e453fc60 ("x86, efi: EFI boot stub > support"). This may well be a compiler bug that this commit just happens > to poke. > > The toolchain is PowerPC hosted cross tools: > x86_64-linux-gcc (GCC) 4.4.3 > GNU ld (GNU Binutils) 2.20 > > I tried an allmodconfig build with my native compiler (gcc (Debian > 4.6.2-14) 4.6.2, GNU gold (GNU Binutils for Debian 2.22) 1.11) and it > worked fine. > > That is a large commit, so I am not sure where to go from here. Actually, that commit does the following: diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c index fdc60a0..4e9bd6b 100644 --- a/arch/x86/boot/tools/build.c +++ b/arch/x86/boot/tools/build.c @@ -135,6 +135,9 @@ static void usage(void) int main(int argc, char ** argv) { +#ifdef CONFIG_EFI_STUB + unsigned int file_sz, pe_header; +#endif unsigned int i, sz, setup_sectors; int c; u32 sys_size; @@ -194,6 +197,42 @@ int main(int argc, char ** argv) buf[0x1f6] = sys_size >> 16; buf[0x1f7] = sys_size >> 24; +#ifdef CONFIG_EFI_STUB + file_sz = sz + i + ((sys_size * 16) - sz); + + pe_header = *(unsigned int *)&buf[0x3c]; + + /* Size of code */ + *(unsigned int *)&buf[pe_header + 0x1c] = file_sz; + + /* Size of image */ + *(unsigned int *)&buf[pe_header + 0x50] = file_sz; + +#ifdef CONFIG_X86_32 + /* Address of entry point */ + *(unsigned int *)&buf[pe_header + 0x28] = i; + + /* .text size */ + *(unsigned int *)&buf[pe_header + 0xb0] = file_sz; + + /* .text size of initialised data */ + *(unsigned int *)&buf[pe_header + 0xb8] = file_sz; +#else + /* + * Address of entry point. startup_32 is at the beginning and + * the 64-bit entry point (startup_64) is always 512 bytes + * after. + */ + *(unsigned int *)&buf[pe_header + 0x28] = i + 512; + + /* .text size */ + *(unsigned int *)&buf[pe_header + 0xc0] = file_sz; + + /* .text size of initialised data */ + *(unsigned int *)&buf[pe_header + 0xc8] = file_sz; +#endif /* CONFIG_X86_32 */ +#endif /* CONFIG_EFI_STUB */ + crc = partial_crc32(buf, i, crc); if (fwrite(buf, 1, i, stdout) != i) die("Writing setup failed"); Which is all endian specific code that will run on the host when building the kernel ... -- Cheers, Stephen Rothwell sfr@canb.auug.org.au http://www.canb.auug.org.au/~sfr/ [-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: build failure in Linus' tree with gcc 4.4.3 2012-02-20 3:22 ` Stephen Rothwell @ 2012-02-20 9:43 ` Matt Fleming 2012-02-21 10:01 ` Matt Fleming 0 siblings, 1 reply; 8+ messages in thread From: Matt Fleming @ 2012-02-20 9:43 UTC (permalink / raw) To: Stephen Rothwell; +Cc: H. Peter Anvin, LKML On Mon, 2012-02-20 at 14:22 +1100, Stephen Rothwell wrote: > Which is all endian specific code that will run on the host when building > the kernel ... Gah, right. I didn't think about cross-building this code. Thanks Stephen, I'll fix this up. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: build failure in Linus' tree with gcc 4.4.3 2012-02-20 9:43 ` Matt Fleming @ 2012-02-21 10:01 ` Matt Fleming 2012-02-21 10:38 ` Stephen Rothwell 0 siblings, 1 reply; 8+ messages in thread From: Matt Fleming @ 2012-02-21 10:01 UTC (permalink / raw) To: Stephen Rothwell; +Cc: H. Peter Anvin, LKML On Mon, 2012-02-20 at 09:43 +0000, Matt Fleming wrote: > On Mon, 2012-02-20 at 14:22 +1100, Stephen Rothwell wrote: > > Which is all endian specific code that will run on the host when building > > the kernel ... > > Gah, right. I didn't think about cross-building this code. > > Thanks Stephen, I'll fix this up. Looks like the segfault is caused by an unaligned access? How does this patch look? >From 54b2707a6a911330d8db2f4ec2fb1baa5e38acf9 Mon Sep 17 00:00:00 2001 From: Matt Fleming <matt.fleming@intel.com> Date: Mon, 20 Feb 2012 17:32:42 +0000 Subject: [PATCH] x86, efi: Fix segfault caused by unaligned access We need to use memcpy() instead of directly dereferencing a pointer because memcpy() correctly handles the case where the source/destination are unaligned, which can lead to a segfault when cross-building an x86 kernel on risc architectures. Stephen Rothwell noticed this bug when he hit a segfault while cross-building an x86_64 allmodconfig kernel on PowerPC. Cc: H. Peter Anvin <hpa@zytor.com> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Signed-off-by: Matt Fleming <matt.fleming@intel.com> --- arch/x86/boot/tools/build.c | 26 ++++++++++++++------------ 1 files changed, 14 insertions(+), 12 deletions(-) diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c index 4e9bd6b..b4d85b5 100644 --- a/arch/x86/boot/tools/build.c +++ b/arch/x86/boot/tools/build.c @@ -200,36 +200,38 @@ int main(int argc, char ** argv) #ifdef CONFIG_EFI_STUB file_sz = sz + i + ((sys_size * 16) - sz); - pe_header = *(unsigned int *)&buf[0x3c]; + memcpy(&pe_header, &buf[0x3c], sizeof(pe_header)); /* Size of code */ - *(unsigned int *)&buf[pe_header + 0x1c] = file_sz; + memcpy(&buf[pe_header + 0x1c], &file_sz, sizeof(file_sz)); /* Size of image */ - *(unsigned int *)&buf[pe_header + 0x50] = file_sz; + memcpy(&buf[pe_header + 0x50], &file_sz, sizeof(file_sz)); #ifdef CONFIG_X86_32 /* Address of entry point */ - *(unsigned int *)&buf[pe_header + 0x28] = i; + memcpy(&buf[pe_header + 0x28], &i, sizeof(i)); /* .text size */ - *(unsigned int *)&buf[pe_header + 0xb0] = file_sz; + memcpy(&buf[pe_header + 0xb0], &file_sz, sizeof(file_sz)); /* .text size of initialised data */ - *(unsigned int *)&buf[pe_header + 0xb8] = file_sz; + memcpy(&buf[pe_header + 0xb8], &file_sz, sizeof(file_sz)); #else + /* .text size */ + memcpy(&buf[pe_header + 0xc0], &file_sz, sizeof(file_sz)); + + /* .text size of initialised data */ + memcpy(&buf[pe_header + 0xc8], &file_sz, sizeof(file_sz)); + /* * Address of entry point. startup_32 is at the beginning and * the 64-bit entry point (startup_64) is always 512 bytes * after. */ - *(unsigned int *)&buf[pe_header + 0x28] = i + 512; + file_sz = i + 512; + memcpy(&buf[pe_header + 0x28], &file_sz, sizeof(file_sz)); - /* .text size */ - *(unsigned int *)&buf[pe_header + 0xc0] = file_sz; - - /* .text size of initialised data */ - *(unsigned int *)&buf[pe_header + 0xc8] = file_sz; #endif /* CONFIG_X86_32 */ #endif /* CONFIG_EFI_STUB */ -- 1.7.4.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: build failure in Linus' tree with gcc 4.4.3 2012-02-21 10:01 ` Matt Fleming @ 2012-02-21 10:38 ` Stephen Rothwell 2012-02-21 11:32 ` Matt Fleming 0 siblings, 1 reply; 8+ messages in thread From: Stephen Rothwell @ 2012-02-21 10:38 UTC (permalink / raw) To: Matt Fleming; +Cc: H. Peter Anvin, LKML [-- Attachment #1: Type: text/plain, Size: 406 bytes --] Hi Matt, On Tue, 21 Feb 2012 10:01:57 +0000 Matt Fleming <matt.fleming@intel.com> wrote: > > Looks like the segfault is caused by an unaligned access? How does this > patch look? I didn't try it, but it won't work. PowerPC is big endian, the data you are copying is little endian .... -- Cheers, Stephen Rothwell sfr@canb.auug.org.au http://www.canb.auug.org.au/~sfr/ [-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: build failure in Linus' tree with gcc 4.4.3 2012-02-21 10:38 ` Stephen Rothwell @ 2012-02-21 11:32 ` Matt Fleming 2012-02-21 11:51 ` Stephen Rothwell 0 siblings, 1 reply; 8+ messages in thread From: Matt Fleming @ 2012-02-21 11:32 UTC (permalink / raw) To: Stephen Rothwell; +Cc: H. Peter Anvin, LKML On Tue, 2012-02-21 at 21:38 +1100, Stephen Rothwell wrote: > Hi Matt, > > On Tue, 21 Feb 2012 10:01:57 +0000 Matt Fleming <matt.fleming@intel.com> wrote: > > > > Looks like the segfault is caused by an unaligned access? How does this > > patch look? > > I didn't try it, but it won't work. PowerPC is big endian, the data you > are copying is little endian .... Urgh, you must feel like you're talking to a brick wall. I forgot you mentioned the endian issue in your original post, sorry. Let's try this one more time... >From b3885c72be4cb06b2647a5067432da6017ea7902 Mon Sep 17 00:00:00 2001 From: Matt Fleming <matt.fleming@intel.com> Date: Mon, 20 Feb 2012 17:32:42 +0000 Subject: [PATCH] x86, efi: Fix unaligned access and endian issues We need to read from and write to 'buf' a byte at a time otherwise it's possible we'll perform an unaligned access, which can lead to a segfault when cross-building an x86 kernel on risc architectures. Also, we may need to convert the endianness of the data we read from/write to buf, so let's add some helper functions to do that. Stephen Rothwell noticed this bug when he hit a segfault while cross-building an x86_64 allmodconfig kernel on PowerPC. Cc: H. Peter Anvin <hpa@zytor.com> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Signed-off-by: Matt Fleming <matt.fleming@intel.com> --- arch/x86/boot/tools/build.c | 44 ++++++++++++++++++++++++++++++------------ 1 files changed, 31 insertions(+), 13 deletions(-) diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c index 4e9bd6b..56efa6f 100644 --- a/arch/x86/boot/tools/build.c +++ b/arch/x86/boot/tools/build.c @@ -133,6 +133,26 @@ static void usage(void) die("Usage: build setup system [> image]"); } +static inline u32 read32_le(u8 *src) +{ + u32 data; + + data = *src++; + data |= *src++ >> 8; + data |= *src++ >> 16; + data |= *src++ >> 24; + + return data; +} + +static inline void write32_le(u8 *dst, u32 data) +{ + *dst++ = data; + *dst++ = data >> 8; + *dst++ = data >> 16; + *dst++ = data >> 24; +} + int main(int argc, char ** argv) { #ifdef CONFIG_EFI_STUB @@ -192,44 +212,42 @@ int main(int argc, char ** argv) /* Patch the setup code with the appropriate size parameters */ buf[0x1f1] = setup_sectors-1; - buf[0x1f4] = sys_size; - buf[0x1f5] = sys_size >> 8; - buf[0x1f6] = sys_size >> 16; - buf[0x1f7] = sys_size >> 24; + write32_le(&buf[0x1f4], sys_size); #ifdef CONFIG_EFI_STUB file_sz = sz + i + ((sys_size * 16) - sz); - pe_header = *(unsigned int *)&buf[0x3c]; + pe_header = read32_le(&buf[0x3c]); /* Size of code */ - *(unsigned int *)&buf[pe_header + 0x1c] = file_sz; + write32_le(&buf[pe_header + 0x1c], file_sz); /* Size of image */ - *(unsigned int *)&buf[pe_header + 0x50] = file_sz; + write32_le(&buf[pe_header + 0x50], file_sz); #ifdef CONFIG_X86_32 /* Address of entry point */ - *(unsigned int *)&buf[pe_header + 0x28] = i; + write32_le(&buf[pe_header + 0x28], i); /* .text size */ - *(unsigned int *)&buf[pe_header + 0xb0] = file_sz; + write32_le(&buf[pe_header + 0xb0], file_sz); /* .text size of initialised data */ - *(unsigned int *)&buf[pe_header + 0xb8] = file_sz; + write32_le(&buf[pe_header + 0xb8], file_sz); #else /* * Address of entry point. startup_32 is at the beginning and * the 64-bit entry point (startup_64) is always 512 bytes * after. */ - *(unsigned int *)&buf[pe_header + 0x28] = i + 512; + write32_le(&buf[pe_header + 0x28], i + 512); /* .text size */ - *(unsigned int *)&buf[pe_header + 0xc0] = file_sz; + write32_le(&buf[pe_header + 0xc0], file_sz); /* .text size of initialised data */ - *(unsigned int *)&buf[pe_header + 0xc8] = file_sz; + write32_le(&buf[pe_header + 0xc8], file_sz); + #endif /* CONFIG_X86_32 */ #endif /* CONFIG_EFI_STUB */ -- 1.7.4.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: build failure in Linus' tree with gcc 4.4.3 2012-02-21 11:32 ` Matt Fleming @ 2012-02-21 11:51 ` Stephen Rothwell 2012-02-21 11:58 ` Matt Fleming 0 siblings, 1 reply; 8+ messages in thread From: Stephen Rothwell @ 2012-02-21 11:51 UTC (permalink / raw) To: Matt Fleming; +Cc: H. Peter Anvin, LKML [-- Attachment #1: Type: text/plain, Size: 401 bytes --] Hi Matt, On Tue, 21 Feb 2012 11:32:13 +0000 Matt Fleming <matt.fleming@intel.com> wrote: > > +static inline u32 read32_le(u8 *src) > +{ > + u32 data; > + > + data = *src++; > + data |= *src++ >> 8; > + data |= *src++ >> 16; > + data |= *src++ >> 24; Shouldn't these be "<<" ? -- Cheers, Stephen Rothwell sfr@canb.auug.org.au http://www.canb.auug.org.au/~sfr/ [-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: build failure in Linus' tree with gcc 4.4.3 2012-02-21 11:51 ` Stephen Rothwell @ 2012-02-21 11:58 ` Matt Fleming 0 siblings, 0 replies; 8+ messages in thread From: Matt Fleming @ 2012-02-21 11:58 UTC (permalink / raw) To: Stephen Rothwell; +Cc: H. Peter Anvin, LKML On Tue, 2012-02-21 at 22:51 +1100, Stephen Rothwell wrote: > Hi Matt, > > On Tue, 21 Feb 2012 11:32:13 +0000 Matt Fleming <matt.fleming@intel.com> wrote: > > > > +static inline u32 read32_le(u8 *src) > > +{ > > + u32 data; > > + > > + data = *src++; > > + data |= *src++ >> 8; > > + data |= *src++ >> 16; > > + data |= *src++ >> 24; > > Shouldn't these be "<<" ? Oops. Yes. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-02-21 11:58 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-02-20 2:39 build failure in Linus' tree with gcc 4.4.3 Stephen Rothwell 2012-02-20 3:22 ` Stephen Rothwell 2012-02-20 9:43 ` Matt Fleming 2012-02-21 10:01 ` Matt Fleming 2012-02-21 10:38 ` Stephen Rothwell 2012-02-21 11:32 ` Matt Fleming 2012-02-21 11:51 ` Stephen Rothwell 2012-02-21 11:58 ` Matt Fleming
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).