/dev/mem gcc weak function workaround

PATCH In review Venki Pallipadi 13 participants 29 messages Apr 30, 2008 – Jun 26, 2008 Char and misc drivers

Share & tools

Venki Pallipadi

author

2008-04-30 01:31 UTC #0 lore raw

PATCH 1/1 /dev/mem gcc weak function workaround

Some flavors of gcc 4.1.0 and 4.1.1 seems to have trouble understanding weak function definitions. Calls to function from the same file where it is defined as weak _may_ get inlined, even when there is a non-weak definition of the function elsewhere. I tried using attribute 'noinline' which does not seem to help either.

One workaround for this is to have weak functions defined in different file as below. Other possible way is to not use weak functions and go back to ifdef logic.

There are few other usages in kernel that seem to depend on weak (and noinline) working correctly, which can also potentially break and needs such workarounds. Example -
mach_reboot_fixups() in arch/x86/kernel/reboot.c is one such call which is getting inlined with a flavor of gcc 4.1.1.

Signed-off-by Venkatesh Pallipadi <[email protected]>

Signed-off-by Suresh Siddha <[email protected]>

---
 drivers/char/Makefile     |    2 +-
 drivers/char/mem.c        |   23 +----------------------
 drivers/char/mem_weak.c   |   25 +++++++++++++++++++++++++
 include/asm-x86/io.h      |    1 -
 include/asm-x86/pgtable.h |    2 --
 include/linux/mmap.h      |   15 +++++++++++++++
 6 files changed, 42 insertions(+), 26 deletions(-)

drivers/char/mem_weak.c new file +25 -0

Index: linux-2.6/drivers/char/mem_weak.c
===================================================================
@@ -0,0 +1,25 @@
+
+#include <linux/mmap.h>
+
+void __attribute__((weak)) unxlate_dev_mem_ptr(unsigned long phys, void *addr)
+{
+}
+
+int __attribute__((weak)) phys_mem_access_prot_allowed(struct file *file,
+	unsigned long pfn, unsigned long size, pgprot_t *vma_prot)
+{
+	return 1;
+}
+
+void __attribute__((weak))
+map_devmem(unsigned long pfn, unsigned long len, pgprot_t prot)
+{
+	/* nothing. architectures can override. */
+}
+
+void __attribute__((weak))
+unmap_devmem(unsigned long pfn, unsigned long len, pgprot_t prot)
+{
+	/* nothing. architectures can override. */
+}
+

drivers/char/mem.c +1 ~~-22~~

Index: linux-2.6/drivers/char/mem.c
===================================================================
@@ -26,6 +26,7 @@
 #include <linux/bootmem.h>
 #include <linux/splice.h>
 #include <linux/pfn.h>
+#include <linux/mmap.h>
 
 #include <asm/uaccess.h>
 #include <asm/io.h>

@@ -105,10 +106,6 @@ static inline int range_is_allowed(unsig
 }
 #endif
 
-void __attribute__((weak)) unxlate_dev_mem_ptr(unsigned long phys, void *addr)
-{
-}
-
 /*
  * This funcion reads the *physical* memory. The f_pos points directly to the 
  * memory location.

@@ -254,12 +251,6 @@ static ssize_t write_mem(struct file * f
 	return written;
 }
 
-int __attribute__((weak)) phys_mem_access_prot_allowed(struct file *file,
-	unsigned long pfn, unsigned long size, pgprot_t *vma_prot)
-{
-	return 1;
-}
-
 #ifndef __HAVE_PHYS_MEM_ACCESS_PROT
 static pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
 				     unsigned long size, pgprot_t vma_prot)

@@ -300,18 +291,6 @@ static inline int private_mapping_ok(str
 }
 #endif
 
-void __attribute__((weak))
-map_devmem(unsigned long pfn, unsigned long len, pgprot_t prot)
-{
-	/* nothing. architectures can override. */
-}
-
-void __attribute__((weak))
-unmap_devmem(unsigned long pfn, unsigned long len, pgprot_t prot)
-{
-	/* nothing. architectures can override. */
-}
-
 static void mmap_mem_open(struct vm_area_struct *vma)
 {
 	map_devmem(vma->vm_pgoff,  vma->vm_end - vma->vm_start,

include/linux/mmap.h new file +15 -0

Index: linux-2.6/include/linux/mmap.h
===================================================================
@@ -0,0 +1,15 @@
+#ifndef _LINUX_MMAP_H
+#define _LINUX_MMAP_H
+
+#include <linux/fs.h>
+#include <asm/page.h>
+
+extern void unxlate_dev_mem_ptr(unsigned long phys, void *addr);
+
+extern int phys_mem_access_prot_allowed(struct file *file,
+	unsigned long pfn, unsigned long size, pgprot_t *vma_prot);
+
+extern void map_devmem(unsigned long pfn, unsigned long len, pgprot_t prot);
+extern void unmap_devmem(unsigned long pfn, unsigned long len, pgprot_t prot);
+
+#endif /* _LINUX_MMAP_H */

include/asm-x86/io.h +0 -1

Index: linux-2.6/include/asm-x86/io.h
===================================================================
@@ -10,7 +10,6 @@
 #endif
 
 extern void *xlate_dev_mem_ptr(unsigned long phys);
-extern void unxlate_dev_mem_ptr(unsigned long phys, void *addr);
 
 extern int ioremap_change_attr(unsigned long vaddr, unsigned long size,
 				unsigned long prot_val);

include/asm-x86/pgtable.h +0 -2

Index: linux-2.6/include/asm-x86/pgtable.h
===================================================================
@@ -303,8 +303,6 @@ static inline pte_t pte_modify(pte_t pte
 struct file;
 pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
                               unsigned long size, pgprot_t vma_prot);
-int phys_mem_access_prot_allowed(struct file *file, unsigned long pfn,
-                              unsigned long size, pgprot_t *vma_prot);
 #endif
 
 #ifdef CONFIG_PARAVIRT

drivers/char/Makefile +1 -1

Index: linux-2.6/drivers/char/Makefile
===================================================================
@@ -7,7 +7,7 @@
 #
 FONTMAPFILE = cp437.uni
 
-obj-y	 += mem.o random.o tty_io.o n_tty.o tty_ioctl.o
+obj-y	 += mem.o mem_weak.o random.o tty_io.o n_tty.o tty_ioctl.o
 
 obj-$(CONFIG_LEGACY_PTYS)	+= pty.o
 obj-$(CONFIG_UNIX98_PTYS)	+= pty.o

David Miller

2008-04-30 04:28 UTC #1 lore raw

From: Venki Pallipadi <[email protected]>
Date: Tue, 29 Apr 2008 18:31:09 -0700

Quoted 10 lines

Some flavors of gcc 4.1.0 and 4.1.1 seems to have trouble understanding weak function definitions. Calls to function from the same file where it is defined as weak _may_ get inlined, even when there is a non-weak definition of the function elsewhere. I tried using attribute 'noinline' which does not seem to help either.
One workaround for this is to have weak functions defined in different file as below. Other possible way is to not use weak functions and go back to ifdef logic.

Quoted 8 lines

There are few other usages in kernel that seem to depend on weak (and noinline) working correctly, which can also potentially break and needs such workarounds. Example -
mach_reboot_fixups() in arch/x86/kernel/reboot.c is one such call which is getting inlined with a flavor of gcc 4.1.1.
Signed-off-by Venkatesh Pallipadi <[email protected]>
Signed-off-by Suresh Siddha <[email protected]>

This sounds like a bug. And if gcc does multi-file compilation it can in theory do the same mistake even if you move it to another file.

We need something more bulletproof here.

Also, we have a macro for marking things weak "__weak" which should be used here.

Pallipadi, Venkatesh

author

2008-04-30 12:54 UTC #2 lore raw

Quoted 38 lines

-----Original Message-----
From: David Miller [mailto:[email protected]]
Sent: Tuesday, April 29, 2008 9:29 PM
To: Pallipadi, Venkatesh
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] /dev/mem gcc weak function workaround
From: Venki Pallipadi <[email protected]>
Date: Tue, 29 Apr 2008 18:31:09 -0700
Quoted 1 line
Some flavors of gcc 4.1.0 and 4.1.1 seems to have trouble
understanding
Quoted 1 line
weak function definitions. Calls to function from the same
file where it is
Quoted 1 line
defined as weak _may_ get inlined, even when there is a
non-weak definition of
Quoted 1 line
the function elsewhere. I tried using attribute 'noinline'
which does not
Quoted 3 lines
seem to help either.
One workaround for this is to have weak functions defined in
different
Quoted 1 line
file as below. Other possible way is to not use weak
functions and go back
Quoted 3 lines
to ifdef logic.
There are few other usages in kernel that seem to depend on
weak (and noinline)
Quoted 1 line
working correctly, which can also potentially break and
needs such workarounds.
Quoted 2 lines
Example -
mach_reboot_fixups() in arch/x86/kernel/reboot.c is one such
call which
Quoted 2 lines
is getting inlined with a flavor of gcc 4.1.1.

Quoted 9 lines

Quoted 2 lines
Signed-off-by Venkatesh Pallipadi <[email protected]>
Signed-off-by Suresh Siddha <[email protected]>
This sounds like a bug. And if gcc does multi-file compilation it can in theory do the same mistake even if you move it to another file.
We need something more bulletproof here.

The references here
http://gcc.gnu.org/ml/gcc-bugs/2006-05/msg02801.html
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27781

seem to suggest that the bug is only with weak definition in the same file.
So, having them in a different file should be good enough workaround here.

Tom: Comments?

Quoted 2 lines

Also, we have a macro for marking things weak "__weak" which should be used here.

Yes. Will change that.

Thanks,
Venki

Tom Rini

2008-04-30 20:28 UTC #3 lore raw

Quoted 52 lines · Venkatesh

Quoted 38 lines
-----Original Message-----
From: David Miller [mailto:[email protected]]
Sent: Tuesday, April 29, 2008 9:29 PM
To: Pallipadi, Venkatesh
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] /dev/mem gcc weak function workaround
From: Venki Pallipadi <[email protected]>
Date: Tue, 29 Apr 2008 18:31:09 -0700
Quoted 1 line
Some flavors of gcc 4.1.0 and 4.1.1 seems to have trouble
understanding
Quoted 1 line
weak function definitions. Calls to function from the same
file where it is
Quoted 1 line
defined as weak _may_ get inlined, even when there is a
non-weak definition of
Quoted 1 line
the function elsewhere. I tried using attribute 'noinline'
which does not
Quoted 3 lines
seem to help either.
One workaround for this is to have weak functions defined in
different
Quoted 1 line
file as below. Other possible way is to not use weak
functions and go back
Quoted 3 lines
to ifdef logic.
There are few other usages in kernel that seem to depend on
weak (and noinline)
Quoted 1 line
working correctly, which can also potentially break and
needs such workarounds.
Quoted 2 lines
Example -
mach_reboot_fixups() in arch/x86/kernel/reboot.c is one such
call which
Quoted 2 lines
is getting inlined with a flavor of gcc 4.1.1.
Quoted 9 lines
Quoted 2 lines
Signed-off-by Venkatesh Pallipadi <[email protected]>
Signed-off-by Suresh Siddha <[email protected]>
This sounds like a bug. And if gcc does multi-file compilation it can in theory do the same mistake even if you move it to another file.
We need something more bulletproof here.
The references here
http://gcc.gnu.org/ml/gcc-bugs/2006-05/msg02801.html
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27781

Quoted 6 lines

seem to suggest that the bug is only with weak definition in the same file.
So, having them in a different file should be good enough workaround here.
Tom: Comments?

Yes, that matches my recollection. some versions of gcc 4.1.0 (and possibly 4.1.1, it wouldn't be hard to check) could not handle having a regular and weak function in the same file (I hit this trying to do some abstraction or another in kgdb, using includes) but it was correct if in separate files. I assume blacklisting 4.1.0 is out of the question :)

Signature · 1 line

Tom Rini

Adrian Bunk

2008-05-01 21:57 UTC #4 lore raw

huge gcc 4.1.{0,1} __weak problem

In reply to Pallipadi, Venkatesh

Quoted 44 lines · Venkatesh

Quoted 30 lines
-----Original Message----- From: David Miller
From: Venki Pallipadi <[email protected]> Date: Tue, 29 Apr 2008 18:31:09 -0700
Quoted 1 line
Some flavors of gcc 4.1.0 and 4.1.1 seems to have trouble
understanding
Quoted 1 line
weak function definitions. Calls to function from the same
file where it is
Quoted 1 line
defined as weak _may_ get inlined, even when there is a
non-weak definition of
Quoted 1 line
the function elsewhere. I tried using attribute 'noinline'
which does not
Quoted 3 lines
seem to help either.
One workaround for this is to have weak functions defined in
different
Quoted 1 line
file as below. Other possible way is to not use weak
functions and go back
Quoted 3 lines
to ifdef logic.
There are few other usages in kernel that seem to depend on
weak (and noinline)
Quoted 1 line
working correctly, which can also potentially break and
needs such workarounds.
Quoted 2 lines
Example -
mach_reboot_fixups() in arch/x86/kernel/reboot.c is one such
call which
Quoted 2 lines
is getting inlined with a flavor of gcc 4.1.1.
Quoted 9 lines
Quoted 2 lines
Signed-off-by Venkatesh Pallipadi <[email protected]>
Signed-off-by Suresh Siddha <[email protected]>
This sounds like a bug. And if gcc does multi-file compilation it can in theory do the same mistake even if you move it to another file.
We need something more bulletproof here.
The references here
http://gcc.gnu.org/ml/gcc-bugs/2006-05/msg02801.html
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27781

Quoted 5 lines

seem to suggest that the bug is only with weak definition in the same file.
So, having them in a different file should be good enough workaround here.
...

A workaround here is the wrong solution since this isn't the only place that suffers from this issue.

We currently give a #warning for 4.1.0.
But not for 4.1.1.
(Accordingto the bug >= 4.1.2 is fixed.)

And a #warning is not enough.

The huge problem is that "empty __weak function in the same file and non-weak arch function" has recently become a common pattern with several new usages added during this merge window alone.

And the breakages can be very subtle runtime breakages.

I see only the following choices:
- remove __weak and replace all current usages
- move all __weak functions into own files, and ensure that also happens for future usages
- #error for gcc 4.1.{0,1}

Quoted 2 lines

Thanks,
Venki

cu
Adrian

Signature · 4 lines

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Andrew Morton

2008-05-01 22:21 UTC #5 lore raw

Re: huge gcc 4.1.{0,1} __weak problem

Quoted 54 lines · On Fri, 2 May 2008 00:56:33 +0300 Adrian Bunk <[email protected]> wrote:

Quoted 44 lines · Venkatesh
Quoted 30 lines
-----Original Message----- From: David Miller
From: Venki Pallipadi <[email protected]> Date: Tue, 29 Apr 2008 18:31:09 -0700
Quoted 1 line
Some flavors of gcc 4.1.0 and 4.1.1 seems to have trouble
understanding
Quoted 1 line
weak function definitions. Calls to function from the same
file where it is
Quoted 1 line
defined as weak _may_ get inlined, even when there is a
non-weak definition of
Quoted 1 line
the function elsewhere. I tried using attribute 'noinline'
which does not
Quoted 3 lines
seem to help either.
One workaround for this is to have weak functions defined in
different
Quoted 1 line
file as below. Other possible way is to not use weak
functions and go back
Quoted 3 lines
to ifdef logic.
There are few other usages in kernel that seem to depend on
weak (and noinline)
Quoted 1 line
working correctly, which can also potentially break and
needs such workarounds.
Quoted 2 lines
Example -
mach_reboot_fixups() in arch/x86/kernel/reboot.c is one such
call which
Quoted 2 lines
is getting inlined with a flavor of gcc 4.1.1.
Quoted 9 lines
Quoted 2 lines
Signed-off-by Venkatesh Pallipadi <[email protected]>
Signed-off-by Suresh Siddha <[email protected]>
This sounds like a bug. And if gcc does multi-file compilation it can in theory do the same mistake even if you move it to another file.
We need something more bulletproof here.
The references here
http://gcc.gnu.org/ml/gcc-bugs/2006-05/msg02801.html
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27781
Quoted 5 lines
seem to suggest that the bug is only with weak definition in the same file.
So, having them in a different file should be good enough workaround here.
...
A workaround here is the wrong solution since this isn't the only place that suffers from this issue.

Quoted 8 lines

We currently give a #warning for 4.1.0.
But not for 4.1.1.
(Accordingto the bug >= 4.1.2 is fixed.)
And a #warning is not enough.
The huge problem is that "empty __weak function in the same file and non-weak arch function" has recently become a common pattern

Perhaps the commonest. Certainly there will be more.

Quoted 10 lines

with
several new usages added during this merge window alone.
And the breakages can be very subtle runtime breakages.
I see only the following choices:
- remove __weak and replace all current usages
- move all __weak functions into own files, and ensure that also happens for future usages
- #error for gcc 4.1.{0,1}

Can we detect the {0,1}? __GNUC_EVEN_MORE_MINOR__?

Yes, I guess we should ban 4.1.x. Ouch.

Linus Torvalds

2008-05-01 22:28 UTC #6 lore raw

Re: huge gcc 4.1.{0,1} __weak problem

Quoted 8 lines · Andrew Morton

Quoted 6 lines
I see only the following choices:
- remove __weak and replace all current usages
- move all __weak functions into own files, and ensure that also happens for future usages
- #error for gcc 4.1.{0,1}
Can we detect the {0,1}? __GNUC_EVEN_MORE_MINOR__?

It's __GNUC_PATCHLEVEL__, I believe.

So yes, we can distinguish 4.1.2 (good, and very common) from 4.1.{0,1} (bad, and rather uncommon).

And yes, considering that 4.1.1 (and even more so 4.1.0) should be rare to begin with, I think it's better to just not support it.

Linus

Andrew Morton

2008-05-01 22:35 UTC #7 lore raw

Re: huge gcc 4.1.{0,1} __weak problem

Quoted 12 lines · On Thu, 1 May 2008 15:27:26 -0700 (PDT) Linus Torvalds…

Quoted 8 lines · Andrew Morton
Quoted 6 lines
I see only the following choices:
- remove __weak and replace all current usages
- move all __weak functions into own files, and ensure that also happens for future usages
- #error for gcc 4.1.{0,1}
Can we detect the {0,1}? __GNUC_EVEN_MORE_MINOR__?

Quoted 8 lines

It's __GNUC_PATCHLEVEL__, I believe.
So yes, we can distinguish 4.1.2 (good, and very common) from 4.1.{0,1} (bad, and rather uncommon).
And yes, considering that 4.1.1 (and even more so 4.1.0) should be rare to begin with, I think it's better to just not support it.

Drat. There go my alpha, i386, m68k, s390, sparc and powerpc cross-compilers. Vagard, save me!

Meanwhile I guess I can locally unpatch that patch.

Tom Rini

2008-05-01 23:25 UTC #8 lore raw

Re: huge gcc 4.1.{0,1} __weak problem

Quoted 20 lines · Andrew Morton

Quoted 17 lines · On Thu, 1 May 2008 15:27:26 -0700 (PDT) Linus Torvalds…
Quoted 8 lines · Andrew Morton
Quoted 6 lines
I see only the following choices:
- remove __weak and replace all current usages
- move all __weak functions into own files, and ensure that also happens for future usages
- #error for gcc 4.1.{0,1}
Can we detect the {0,1}? __GNUC_EVEN_MORE_MINOR__?
It's __GNUC_PATCHLEVEL__, I believe.
So yes, we can distinguish 4.1.2 (good, and very common) from 4.1.{0,1} (bad, and rather uncommon).

Quoted 8 lines

Quoted 3 lines
And yes, considering that 4.1.1 (and even more so 4.1.0) should be rare to begin with, I think it's better to just not support it.
Drat. There go my alpha, i386, m68k, s390, sparc and powerpc cross-compilers. Vagard, save me!
Meanwhile I guess I can locally unpatch that patch.

I know I'll come off as an ass, but you can't make new ones with 4.1.2? It's not like we're talking about gcc 2.95/96 fun here :)

Signature · 1 line

Tom Rini

Andrew Morton

2008-05-02 00:00 UTC #9 lore raw

Re: huge gcc 4.1.{0,1} __weak problem

Quoted 25 lines · On Thu, 1 May 2008 16:24:47 -0700 Tom Rini <[email protected]> wrote:

Quoted 24 lines · Andrew Morton
Quoted 20 lines · On Thu, 1 May 2008 15:27:26 -0700 (PDT) Linus Torvalds…
Quoted 8 lines · Andrew Morton
Quoted 6 lines
I see only the following choices:
- remove __weak and replace all current usages
- move all __weak functions into own files, and ensure that also happens for future usages
- #error for gcc 4.1.{0,1}
Can we detect the {0,1}? __GNUC_EVEN_MORE_MINOR__?
It's __GNUC_PATCHLEVEL__, I believe.
So yes, we can distinguish 4.1.2 (good, and very common) from 4.1.{0,1} (bad, and rather uncommon).
And yes, considering that 4.1.1 (and even more so 4.1.0) should be rare to begin with, I think it's better to just not support it.

Quoted 7 lines

Quoted 4 lines
Drat. There go my alpha, i386, m68k, s390, sparc and powerpc cross-compilers. Vagard, save me!
Meanwhile I guess I can locally unpatch that patch.
I know I'll come off as an ass, but you can't make new ones with 4.1.2? It's not like we're talking about gcc 2.95/96 fun here :)

Honestly, I nearly died when I built all those cross-compilers. Sooooooo many combinations of gcc/binutils/glibc refused to work for obscure reasons. Compilation on x86_64 just didn't work at all and I ended up having to build everything on a slow i386 box, etc, etc. The stream of email to Dan got increasingly strident ;)

I think crosstool has become a lot better since then, judging from the ease with which Jens was able to spin up the powerpc compiler, but the trauma was a life-long thing.

Vegard has been making noises about (finally!) preparing and maintaining a decent set of cross-compilers for us. It would be a great service (begs).

Justin Mattock

2008-05-02 00:21 UTC #10 lore raw

Re: huge gcc 4.1.{0,1} __weak problem

Quoted 46 lines · Andrew Morton

Quoted 25 lines · On Thu, 1 May 2008 16:24:47 -0700 Tom Rini <[email protected]> wrote:
Quoted 24 lines · Andrew Morton
Quoted 20 lines · On Thu, 1 May 2008 15:27:26 -0700 (PDT) Linus Torvalds…
Quoted 8 lines · Andrew Morton
Quoted 6 lines
I see only the following choices:
- remove __weak and replace all current usages
- move all __weak functions into own files, and ensure that also happens for future usages
- #error for gcc 4.1.{0,1}
Can we detect the {0,1}? __GNUC_EVEN_MORE_MINOR__?
It's __GNUC_PATCHLEVEL__, I believe.
So yes, we can distinguish 4.1.2 (good, and very common) from 4.1.{0,1} (bad, and rather uncommon).
And yes, considering that 4.1.1 (and even more so 4.1.0) should be rare to begin with, I think it's better to just not support it.
Quoted 7 lines
Quoted 4 lines
Drat. There go my alpha, i386, m68k, s390, sparc and powerpc cross-compilers. Vagard, save me!
Meanwhile I guess I can locally unpatch that patch.
I know I'll come off as an ass, but you can't make new ones with 4.1.2? It's not like we're talking about gcc 2.95/96 fun here :)
Honestly, I nearly died when I built all those cross-compilers. Sooooooo many combinations of gcc/binutils/glibc refused to work for obscure reasons. Compilation on x86_64 just didn't work at all and I ended up having to build everything on a slow i386 box, etc, etc. The stream of email to Dan got increasingly strident ;)
I think crosstool has become a lot better since then, judging from the ease with which Jens was able to spin up the powerpc compiler, but the trauma was a life-long thing.

Quoted 9 lines

Vegard has been making noises about (finally!) preparing and maintaining a decent set of cross-compilers for us. It would be a great service (begs).
--
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/

I hate to jump in like this, but I noticed something while compiling madwifi as well with
gcc, I'm wondering if this is related to the above:
/home/name/Desktop/madwifi-trunk-r3574-20080426/Makefile:50 Makefile.inc: no such file or directory
even though the files exists,

Makefile.inc98: scripts/get_arch.mk: no such file or directory Makefile.inc:102 ath_hal/ah_target.inc: no such file or directory Makefile.inc:163 *** TARGET i386-elf is invalid, invalid target are: . Stop. make[1] *** [modules] Error 2

After modifying the Makefiles you're left with ***TARGET i386-elf problem

regards;

Signature · 1 line

Justin P. Mattock

Vegard Nossum

2008-05-02 07:18 UTC #11 lore raw

Re: huge gcc 4.1.{0,1} __weak problem

In reply to Andrew Morton

Hey :)

Quoted 27 lines · Andrew Morton

Quoted 23 lines · On Thu, 1 May 2008 16:24:47 -0700 Tom Rini <[email protected]> wrote:
Quoted 22 lines · Andrew Morton
Quoted 18 lines · On Thu, 1 May 2008 15:27:26 -0700 (PDT) Linus Torvalds…
Quoted 8 lines · Andrew Morton
Quoted 6 lines
I see only the following choices:
- remove __weak and replace all current usages
- move all __weak functions into own files, and ensure that also happens for future usages
- #error for gcc 4.1.{0,1}
Can we detect the {0,1}? __GNUC_EVEN_MORE_MINOR__?
It's __GNUC_PATCHLEVEL__, I believe.
So yes, we can distinguish 4.1.2 (good, and very common) from 4.1.{0,1} (bad, and rather uncommon).
And yes, considering that 4.1.1 (and even more so 4.1.0) should be rare to begin with, I think it's better to just not support it.

Quoted 7 lines

Quoted 7 lines
Quoted 4 lines
Drat. There go my alpha, i386, m68k, s390, sparc and powerpc cross-compilers. Vagard, save me!
Meanwhile I guess I can locally unpatch that patch.
I know I'll come off as an ass, but you can't make new ones with 4.1.2? It's not like we're talking about gcc 2.95/96 fun here :)

It's true; crosstool hasn't been much of a trouble (for me anyway). A couple of combinations that I tried failed.

Quoted 6 lines

Honestly, I nearly died when I built all those cross-compilers. Sooooooo many combinations of gcc/binutils/glibc refused to work for obscure reasons. Compilation on x86_64 just didn't work at all and I ended up having to build everything on a slow i386 box, etc, etc. The stream of email to Dan got increasingly strident ;)

Quoted 6 lines

I think crosstool has become a lot better since then, judging from the ease with which Jens was able to spin up the powerpc compiler, but the trauma was a life-long thing.
Vegard has been making noises about (finally!) preparing and maintaining a decent set of cross-compilers for us. It would be a great service (begs).

Noises? Heh...

So I've only just begun that work. For i386 host we have the following targets:
alpha, arm, ia64, sparc64, sparc, x86_64

though these are largely untested with Linux Kernel. Most of them are gcc 3.4.5 (this should in theory compile the kernel correctly), though I found an e-mail from Ingo saying that you need the -tls versions for stackprotector to work correctly. This might be a good time to ask if I should be making gcc 4.1.2s instead. I need to recompile in either case.

In other words, we are NOT fully operational yet :-)

Though H. Peter gave me a directory to stuff it in, you could try it out if you're impatient:

http://www.kernel.org/pub/tools/crosstool/

(Hm, it seems that all my .asc files disappeared in the transition.. Oh well.)

Vegard

Signature · 4 lines

"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
	-- E. W. Dijkstra, EWD1036

Theodore Tso

2008-05-02 13:45 UTC #12 lore raw

Re: huge gcc 4.1.{0,1} __weak problem

Quoted 6 lines · Vegard Nossum

though these are largely untested with Linux Kernel. Most of them are gcc 3.4.5 (this should in theory compile the kernel correctly), though I found an e-mail from Ingo saying that you need the -tls versions for stackprotector to work correctly. This might be a good time to ask if I should be making gcc 4.1.2s instead. I need to recompile in either case.

Out of curiosity, are you using the stock gcc/binutils from the FSF, or one of the distro toolchains? When we were investigating which compiler/toolchain to use for the LSB Sample Implementation, one of the comments that I got from folks like Eric Troan from rPath and others was that out-of-the-box compiler/toolchain had so many bugs, particularly on non-x86 architectures, that they found they were much better off using a distro-patched/bug-fixed compiler/toolchain and treating that as the upstream.

There has been some requests to include cross-compilation functionality into the LSB Build Environment, so I'd be interested in seeing how your work has been going, and maybe there's some opportunity to work together. Is there a mailing list you have for this project?

- Ted

Adrian Bunk

2008-05-02 08:12 UTC #13 lore raw

Re: huge gcc 4.1.{0,1} __weak problem

In reply to Andrew Morton

Quoted 12 lines · Andrew Morton

Quoted 9 lines · On Thu, 1 May 2008 16:24:47 -0700 Tom Rini <[email protected]> wrote:
Quoted 5 lines · Andrew Morton
Drat. There go my alpha, i386, m68k, s390, sparc and powerpc cross-compilers. Vagard, save me!
Meanwhile I guess I can locally unpatch that patch.
I know I'll come off as an ass, but you can't make new ones with 4.1.2? It's not like we're talking about gcc 2.95/96 fun here :)

Quoted 6 lines

Honestly, I nearly died when I built all those cross-compilers. Sooooooo many combinations of gcc/binutils/glibc refused to work for obscure reasons. Compilation on x86_64 just didn't work at all and I ended up having to build everything on a slow i386 box, etc, etc. The stream of email to Dan got increasingly strident ;)
...

I have my binutils 2.18.50.0.6 / gcc 4.3 based cross toolchains for an i386 host here (no glibc included since I'm using them only for kernel testcompiles).

If you are using an x86 machine I can rebuild them without -march=k8 and give you a copy.

cu
Adrian

Signature · 4 lines

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Andi Kleen

2008-05-02 09:09 UTC #14 lore raw

Re: huge gcc 4.1.{0,1} __weak problem

In reply to Andrew Morton

Quoted 6 lines · Andrew Morton

Honestly, I nearly died when I built all those cross-compilers. Sooooooo many combinations of gcc/binutils/glibc refused to work for obscure reasons. Compilation on x86_64 just didn't work at all and I ended up having to build everything on a slow i386 box, etc, etc. The stream of email to Dan got increasingly strident ;)

This might be pointing out the obvious, but to just update the compiler from 4.1.0 to 4.1.2 (or other) you don't need to recompile binutils and glibc.

-Andi

Venki Pallipadi

author

2008-05-01 22:35 UTC #15 lore raw

Re: huge gcc 4.1.{0,1} __weak problem

In reply to Linus Torvalds

Quoted 12 lines · Linus Torvalds

Quoted 8 lines · Andrew Morton
Quoted 6 lines
I see only the following choices:
- remove __weak and replace all current usages
- move all __weak functions into own files, and ensure that also happens for future usages
- #error for gcc 4.1.{0,1}
Can we detect the {0,1}? __GNUC_EVEN_MORE_MINOR__?

Quoted 8 lines

It's __GNUC_PATCHLEVEL__, I believe.
So yes, we can distinguish 4.1.2 (good, and very common) from 4.1.{0,1} (bad, and rather uncommon).
And yes, considering that 4.1.1 (and even more so 4.1.0) should be rare to begin with, I think it's better to just not support it.

Not sure whether #error on gcc 4.1.{0.1} is the right thing as I found atleast one distro gcc which says itself as 4.1.1, do not exhibit the problem as it most likely has fix backported.

Putting all weak functions in one file is something Suresh and I considered before sending this patch. But, looking at various users of __weak, that single file did not look very appropriate.

Thanks,
Venki

Andrew Morton

2008-05-01 22:44 UTC #16 lore raw

Re: huge gcc 4.1.{0,1} __weak problem

Quoted 22 lines · On Thu, 1 May 2008 15:35:15 -0700 Venki Pallipadi…

Quoted 12 lines · Linus Torvalds
Quoted 8 lines · Andrew Morton
Quoted 6 lines
I see only the following choices:
- remove __weak and replace all current usages
- move all __weak functions into own files, and ensure that also happens for future usages
- #error for gcc 4.1.{0,1}
Can we detect the {0,1}? __GNUC_EVEN_MORE_MINOR__?
Quoted 8 lines
It's __GNUC_PATCHLEVEL__, I believe.
So yes, we can distinguish 4.1.2 (good, and very common) from 4.1.{0,1} (bad, and rather uncommon).
And yes, considering that 4.1.1 (and even more so 4.1.0) should be rare to begin with, I think it's better to just not support it.

Quoted 8 lines

Not sure whether #error on gcc 4.1.{0.1} is the right thing as I found atleast one distro gcc which says itself as 4.1.1, do not exhibit the problem as it most likely has fix backported.
Putting all weak functions in one file is something Suresh and I considered before sending this patch. But, looking at various users of __weak, that single file did not look very appropriate.

Is there some vaguely maintainable workaround we can do? If the problem only affects completely-empty weak functions then we could put something in them to make them non-empty?

#if __GNUC__ == ...
#define weak_function_filler for ( ; ; );
#else
#define weak_function_filler()
#endif

because __weak and attribute((weak)) are pretty easy to grep for. Dunno.

Jakub Jelinek

2008-05-01 22:51 UTC #17 lore raw

Re: huge gcc 4.1.{0,1} __weak problem

Quoted 3 lines · Andrew Morton

Is there some vaguely maintainable workaround we can do? If the problem only affects completely-empty weak functions then we could put something in them to make them non-empty?

for (;;); isn't enough, the function would be still considered const and by 4.1.0 and some 4.1.1 incorrectly optimized out, without regard to weak attribute.
But e.g.
asm ("");
should be enough.

Jakub

Tom Rini

2008-05-01 23:21 UTC #18 lore raw

Re: huge gcc 4.1.{0,1} __weak problem

In reply to Andrew Morton

On Thu, May 01, 2008 at 03:42:38PM -0700, Andrew Morton wrote:

[snip]

Quoted 3 lines

Is there some vaguely maintainable workaround we can do? If the problem only affects completely-empty weak functions then we could put something in them to make them non-empty?

My memory is a tiny bit hazy (it was a while ago), but no, it's not just empty functions (again, I _think_ I hit it with a generic vs arch weak function).

Signature · 1 line

Tom Rini

Venki Pallipadi

author

2008-05-01 23:30 UTC #19 lore raw

Re: huge gcc 4.1.{0,1} __weak problem

Quoted 11 lines · Tom Rini

On Thu, May 01, 2008 at 03:42:38PM -0700, Andrew Morton wrote:
[snip]
Quoted 3 lines
Is there some vaguely maintainable workaround we can do? If the problem only affects completely-empty weak functions then we could put something in them to make them non-empty?
My memory is a tiny bit hazy (it was a while ago), but no, it's not just empty functions (again, I _think_ I hit it with a generic vs arch weak function).

Other thing we observed was: this does not just depend on the __weak function definition. It also depends on where the function is called from.

__weak function with single return statement, did not get inlined when called from say

caller()

{
	function();
}

but got inlined when called as in

caller()

{
	for (;;) {
		function();
	}
}

Thanks,
Venki

Linus Torvalds

2008-05-02 00:36 UTC #20 lore raw

Re: huge gcc 4.1.{0,1} __weak problem

Quoted 3 lines · Venki Pallipadi

__weak function with single return statement, did not get inlined when called from say

Is it always about inlining? If so, can't we add a __noinline__ to the declaration of __weak?

Something like this oneliner?

Linus

---
 include/linux/compiler-gcc.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

include/linux/compiler-gcc.h +1 -1

index 5c8351b..4061fc7 100644
@@ -41,7 +41,7 @@
 
 #define __deprecated			__attribute__((deprecated))
 #define __packed			__attribute__((packed))
-#define __weak				__attribute__((weak))
+#define __weak				__attribute__((weak)) noinline
 #define __naked				__attribute__((naked))
 #define __noreturn			__attribute__((noreturn))

Suresh Siddha

2008-05-02 00:40 UTC #21 lore raw

Re: huge gcc 4.1.{0,1} __weak problem

Quoted 2 lines · Linus Torvalds

Is it always about inlining? If so, can't we add a __noinline__ to the declaration of __weak?

We tried that and it was still getting inlined.

thanks,
suresh

Jeremy Fitzhardinge

2008-05-02 21:11 UTC #22 lore raw

Re: huge gcc 4.1.{0,1} __weak problem

Quoted 6 lines · Suresh Siddha

Quoted 2 lines · Linus Torvalds
Is it always about inlining? If so, can't we add a __noinline__ to the declaration of __weak?
We tried that and it was still getting inlined.

That's a pity. I've worked around this bug with noinline before.

David Miller

2008-05-02 22:02 UTC #23 lore raw

Re: huge gcc 4.1.{0,1} __weak problem

From: Jeremy Fitzhardinge <[email protected]>
Date: Fri, 02 May 2008 14:11:18 -0700

Quoted 10 lines

Quoted 7 lines · Suresh Siddha
Quoted 3 lines · Linus Torvalds
Is it always about inlining? If so, can't we add a __noinline__ to the declaration of __weak?
We tried that and it was still getting inlined.
That's a pity. I've worked around this bug with noinline before.

It's "constness", as Jakub mentioned, not inlineability, that triggers this bug.

That's why his workaround of using an empty asm("") to the function is an effective workaround, because the compiler can no longer internally decide that the function is "const".

Tom Rini

2008-05-01 23:23 UTC #24 lore raw

Re: huge gcc 4.1.{0,1} __weak problem

In reply to Venki Pallipadi

Quoted 1 line · Venki Pallipadi

On Thu, May 01, 2008 at 03:27:26PM -0700, Linus Torvalds wrote:

[snip]

Quoted 5 lines

Quoted 5 lines
It's __GNUC_PATCHLEVEL__, I believe.
So yes, we can distinguish 4.1.2 (good, and very common) from 4.1.{0,1} (bad, and rather uncommon).

Quoted 7 lines

Quoted 3 lines
And yes, considering that 4.1.1 (and even more so 4.1.0) should be rare to begin with, I think it's better to just not support it.
Not sure whether #error on gcc 4.1.{0.1} is the right thing as I found atleast one distro gcc which says itself as 4.1.1, do not exhibit the problem as it most likely has fix backported.

Really? At the time this was a very uncommon thing (hence the initial it's not a bug, you just didn't use the right flags) comments. I suppose it's possible of course that some distro took a 4.1 snapshot and called it 4.1.1.

Quoted 3 lines

Putting all weak functions in one file is something Suresh and I considered before sending this patch. But, looking at various users of __weak, that single file did not look very appropriate.

Indeed. I suspect that even if you go so far as to do a single patch per "feature", it's gonna be a lotta stuff.

Signature · 1 line

Tom Rini

David Miller

2008-05-01 22:52 UTC #25 lore raw

Re: huge gcc 4.1.{0,1} __weak problem

In reply to Linus Torvalds

From: Linus Torvalds <[email protected]>
Date: Thu, 1 May 2008 15:27:26 -0700 (PDT)

Quoted 5 lines

So yes, we can distinguish 4.1.2 (good, and very common) from 4.1.{0,1} (bad, and rather uncommon).
And yes, considering that 4.1.1 (and even more so 4.1.0) should be rare to begin with, I think it's better to just not support it.

This is my impression as well.

Adrian Bunk

2008-06-26 10:40 UTC #26 lore raw

PATCH 1/1 #error for gcc 4.1.{0,1}

In reply to Linus Torvalds

Quoted 12 lines · Linus Torvalds

Quoted 8 lines · Andrew Morton
Quoted 6 lines
I see only the following choices:
- remove __weak and replace all current usages
- move all __weak functions into own files, and ensure that also happens for future usages
- #error for gcc 4.1.{0,1}
Can we detect the {0,1}? __GNUC_EVEN_MORE_MINOR__?

Quoted 7 lines

It's __GNUC_PATCHLEVEL__, I believe.
So yes, we can distinguish 4.1.2 (good, and very common) from 4.1.{0,1} (bad, and rather uncommon).
And yes, considering that 4.1.1 (and even more so 4.1.0) should be rare to begin with, I think it's better to just not support it.

Some talk and one and a half months later we still don't abort the build for these gcc version.

If anyone has a better patch please step forward - otherwise I'd propose the patch below.

Quoted 1 line

Linus

cu
Adrian

<-- snip -->

gcc 4.1.0 and 4.1.1 are known to miscompile the kernel:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27781

Usage of weak functions has become a common pattern in the kernel, and usages get added in each kernel version increasing the probability of bugs with each kernel release.

This miscompilation of weak functions can result in subtle runtime errors.

#error for gcc 4.1.0 and 4.1.1 to prevent users from running into this bug.

Note:
We already printed a #warning for gcc 4.1.0 due to a different bug.

Signed-off-by Adrian Bunk <[email protected]>

---
ee78871a1d85fe60958748c208389adb4031fefe diff --git a/init/main.c b/init/main.c index f7fb200..bede344 100644

init/main.c +3 -2

@@ -76,8 +76,9 @@
  * trouble.
  */
 
-#if __GNUC__ == 4 && __GNUC_MINOR__ == 1 && __GNUC_PATCHLEVEL__ == 0
-#warning gcc-4.1.0 is known to miscompile the kernel.  A different compiler version is recommended.
+/*  due to http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27781  */
+#if __GNUC__ == 4 && __GNUC_MINOR__ == 1 && (__GNUC_PATCHLEVEL__ == 0 || __GNUC_PATCHLEVEL__ == 1)
+#error gcc 4.1.0 and 4.1.1 are known to miscompile the kernel.
 #endif
 
 static int kernel_init(void *);

Jeremy Fitzhardinge

2008-05-02 21:10 UTC #27 lore raw

Re: huge gcc 4.1.{0,1} __weak problem

In reply to Adrian Bunk

Quoted 82 lines · Adrian Bunk

Quoted 63 lines · Venkatesh
Quoted 49 lines
-----Original Message----- From: David Miller
From: Venki Pallipadi <[email protected]> Date: Tue, 29 Apr 2008 18:31:09 -0700
Quoted 2 lines
Some flavors of gcc 4.1.0 and 4.1.1 seems to have trouble
understanding
Quoted 2 lines
weak function definitions. Calls to function from the same
file where it is
Quoted 2 lines
defined as weak _may_ get inlined, even when there is a
non-weak definition of
Quoted 2 lines
the function elsewhere. I tried using attribute 'noinline'
which does not
Quoted 4 lines
seem to help either.
One workaround for this is to have weak functions defined in
different
Quoted 2 lines
file as below. Other possible way is to not use weak
functions and go back
Quoted 4 lines
to ifdef logic.
There are few other usages in kernel that seem to depend on
weak (and noinline)
Quoted 2 lines
working correctly, which can also potentially break and
needs such workarounds.
Quoted 3 lines
Example -
mach_reboot_fixups() in arch/x86/kernel/reboot.c is one such
call which
Quoted 2 lines
is getting inlined with a flavor of gcc 4.1.1.
Quoted 10 lines
Quoted 3 lines
Signed-off-by Venkatesh Pallipadi <[email protected]>
Signed-off-by Suresh Siddha <[email protected]>
This sounds like a bug. And if gcc does multi-file compilation it can in theory do the same mistake even if you move it to another file.
We need something more bulletproof here.
The references here
http://gcc.gnu.org/ml/gcc-bugs/2006-05/msg02801.html
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27781
Quoted 6 lines
seem to suggest that the bug is only with weak definition in the same file.
So, having them in a different file should be good enough workaround here.
...
A workaround here is the wrong solution since this isn't the only place that suffers from this issue.
We currently give a #warning for 4.1.0.
But not for 4.1.1.
(Accordingto the bug >= 4.1.2 is fixed.)
And a #warning is not enough.
The huge problem is that "empty __weak function in the same file and non-weak arch function" has recently become a common pattern with several new usages added during this merge window alone.

Quoted 8 lines

And the breakages can be very subtle runtime breakages.
I see only the following choices:
- remove __weak and replace all current usages
- move all __weak functions into own files, and ensure that also happens for future usages
- #error for gcc 4.1.{0,1}

- make __weak also include noinline. I think that's sufficient (at least it was when I encountered a gcc bug with these symptoms last year or so).

Adrian Bunk

2008-05-02 21:20 UTC #28 lore raw

Re: huge gcc 4.1.{0,1} __weak problem

Quoted 23 lines · Jeremy Fitzhardinge

Quoted 13 lines · Adrian Bunk
A workaround here is the wrong solution since this isn't the only place that suffers from this issue.
We currently give a #warning for 4.1.0.
But not for 4.1.1.
(Accordingto the bug >= 4.1.2 is fixed.)
And a #warning is not enough.
The huge problem is that "empty __weak function in the same file and non-weak arch function" has recently become a common pattern with several new usages added during this merge window alone.
Quoted 8 lines
And the breakages can be very subtle runtime breakages.
I see only the following choices:
- remove __weak and replace all current usages
- move all __weak functions into own files, and ensure that also happens for future usages
- #error for gcc 4.1.{0,1}

Quoted 3 lines

- make __weak also include noinline. I think that's sufficient (at least it was when I encountered a gcc bug with these symptoms last year or so).

I've tried it and it doesn't work.

Quoted 1 line

J

cu
Adrian

Signature · 4 lines

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed