heads up: apparent GAS bug in current (CVS) binutils

heads up: apparent GAS bug in current (CVS) binutils

[David Mosberger]

Just a quick heads up (it's late here and I'll be on travel tomorrow
so I don't have time for a proper investigation/bug-report at the
moment):

If I build the kernel with the assembler from the current CVS
binutils, I see these errors:

ERROR: proc_set_irq_affinity: 179742 slots, total region length = 36
ERROR: idle_regs: 68847 slots, total region length = 15

The error in proc_set_irq_affinity() is definitely real and a result
of the assembler (the assembly code generated by GCC looks fine).  I
didn't check on idle_regs.

readelf -u vmlinux shows this:

<proc_set_irq_affinity>: [0xa00000010000ef60-0xa0000001000f9000], info at +0x765
c50
  v1, flags=0x0 (), len=8 bytes
    R2:prologue_gr(mask=[rp,ar.pfs],grsave=r34,rlen=8)
        P7:pfs_when(t=0)
        P7:rp_when(t=7)
    R1:body(rlen()

<move_irq>: [0xa00000010000f0c0-0xa00000010000f250], info at +0x75da00

Note how the end-address of proc_set_irq_affinity() is _way_ beyond
the start-address of the next function, move_irq.  Clearly something
weird happened here.

If I use the standard Debian/sarge assembler, these errors go away and
the unwind-info looks sane.

	--david

RE: heads up: apparent GAS bug in current (CVS) binutils

[Lu, Hongjiu]

I tried binutils 2.15.92.0.2 20040927 and got the same result. What is
the version of assembler which you think is good?

H.J. Lu
Intel Corporation


>-----Original Message-----
>From: David Mosberger [mailto:davidm@napali.hpl.hp.com]
>Sent: Thursday, May 12, 2005 2:56 PM
>To: linux-ia64@vger.kernel.org
>Cc: Lu, Hongjiu
>Subject: heads up: apparent GAS bug in current (CVS) binutils
>
>Just a quick heads up (it's late here and I'll be on travel tomorrow
>so I don't have time for a proper investigation/bug-report at the
>moment):
>
>If I build the kernel with the assembler from the current CVS
>binutils, I see these errors:
>
>ERROR: proc_set_irq_affinity: 179742 slots, total region length = 36
>ERROR: idle_regs: 68847 slots, total region length = 15
>
>The error in proc_set_irq_affinity() is definitely real and a result
>of the assembler (the assembly code generated by GCC looks fine).  I
>didn't check on idle_regs.
>
>readelf -u vmlinux shows this:
>
><proc_set_irq_affinity>: [0xa00000010000ef60-0xa0000001000f9000], info
>at +0x765
>c50
>  v1, flags=0x0 (), len=8 bytes
>    R2:prologue_gr(mask=[rp,ar.pfs],grsave=r34,rlen=8)
>        P7:pfs_when(t=0)
>        P7:rp_when(t=7)
>    R1:body(rlen()
>
><move_irq>: [0xa00000010000f0c0-0xa00000010000f250], info at +0x75da00
>
>Note how the end-address of proc_set_irq_affinity() is _way_ beyond
>the start-address of the next function, move_irq.  Clearly something
>weird happened here.
>
>If I use the standard Debian/sarge assembler, these errors go away and
>the unwind-info looks sane.
>
>	--david

RE: heads up: apparent GAS bug in current (CVS) binutils

[Lu, Hongjiu]

Hi David,

It is a linker issue. There are 2 definitions for those functions, the
generic is weak and the ia64 one is strong. The generic one has unwind
info and the ia64 one doesn't. When linker combines them, it keeps the
strong definition, but uses the weak unwind info. I will see what I can
do about it.


H.J. Lu
Intel Corporation


>-----Original Message-----
>From: David Mosberger [mailto:davidm@napali.hpl.hp.com]
>Sent: Thursday, May 12, 2005 2:56 PM
>To: linux-ia64@vger.kernel.org
>Cc: Lu, Hongjiu
>Subject: heads up: apparent GAS bug in current (CVS) binutils
>
>Just a quick heads up (it's late here and I'll be on travel tomorrow
>so I don't have time for a proper investigation/bug-report at the
>moment):
>
>If I build the kernel with the assembler from the current CVS
>binutils, I see these errors:
>
>ERROR: proc_set_irq_affinity: 179742 slots, total region length = 36
>ERROR: idle_regs: 68847 slots, total region length = 15
>
>The error in proc_set_irq_affinity() is definitely real and a result
>of the assembler (the assembly code generated by GCC looks fine).  I
>didn't check on idle_regs.
>
>readelf -u vmlinux shows this:
>
><proc_set_irq_affinity>: [0xa00000010000ef60-0xa0000001000f9000], info
>at +0x765
>c50
>  v1, flags=0x0 (), len=8 bytes
>    R2:prologue_gr(mask=[rp,ar.pfs],grsave=r34,rlen=8)
>        P7:pfs_when(t=0)
>        P7:rp_when(t=7)
>    R1:body(rlen()
>
><move_irq>: [0xa00000010000f0c0-0xa00000010000f250], info at +0x75da00
>
>Note how the end-address of proc_set_irq_affinity() is _way_ beyond
>the start-address of the next function, move_irq.  Clearly something
>weird happened here.
>
>If I use the standard Debian/sarge assembler, these errors go away and
>the unwind-info looks sane.
>
>	--david

RE: heads up: apparent GAS bug in current (CVS) binutils

[Lu, Hongjiu]

[-- Attachment #1: Type: text/plain, Size: 1752 bytes --]

Hi David,

I think it is the result of the usage of weak function. I am enclosing a
testcase to show it. The correct approach is to use comdat section group
or linkonce section. The problem will go away. I will see if I can come
up with a kernel patch.

H.J. Lu
Intel Corporation


>-----Original Message-----
>From: David Mosberger [mailto:davidm@napali.hpl.hp.com]
>Sent: Thursday, May 12, 2005 2:56 PM
>To: linux-ia64@vger.kernel.org
>Cc: Lu, Hongjiu
>Subject: heads up: apparent GAS bug in current (CVS) binutils
>
>Just a quick heads up (it's late here and I'll be on travel tomorrow
>so I don't have time for a proper investigation/bug-report at the
>moment):
>
>If I build the kernel with the assembler from the current CVS
>binutils, I see these errors:
>
>ERROR: proc_set_irq_affinity: 179742 slots, total region length = 36
>ERROR: idle_regs: 68847 slots, total region length = 15
>
>The error in proc_set_irq_affinity() is definitely real and a result
>of the assembler (the assembly code generated by GCC looks fine).  I
>didn't check on idle_regs.
>
>readelf -u vmlinux shows this:
>
><proc_set_irq_affinity>: [0xa00000010000ef60-0xa0000001000f9000], info
>at +0x765
>c50
>  v1, flags=0x0 (), len=8 bytes
>    R2:prologue_gr(mask=[rp,ar.pfs],grsave=r34,rlen=8)
>        P7:pfs_when(t=0)
>        P7:rp_when(t=7)
>    R1:body(rlen=28)
>
><move_irq>: [0xa00000010000f0c0-0xa00000010000f250], info at +0x75da00
>
>Note how the end-address of proc_set_irq_affinity() is _way_ beyond
>the start-address of the next function, move_irq.  Clearly something
>weird happened here.
>
>If I use the standard Debian/sarge assembler, these errors go away and
>the unwind-info looks sane.
>
>	--david

[-- Attachment #2: bug.tar.gz --]
[-- Type: application/x-gzip, Size: 571 bytes --]

RE: heads up: apparent GAS bug in current (CVS) binutils

[David Mosberger]

>>>>> On Fri, 13 May 2005 10:32:19 -0700, "Lu, Hongjiu" <hongjiu.lu@intel.com> said:

  HJ> I tried binutils 2.15.92.0.2 20040927 and got the same
  HJ> result. What is the version of assembler which you think is
  HJ> good?

The standard Debian/sarge as seems fine:

 $ as -v
 GNU assembler version 2.15 (ia64-linux) using BFD version 2.15

	--david

Re: heads up: apparent GAS bug in current (CVS) binutils

[H. J. Lu]

On Fri, May 13, 2005 at 01:34:10PM -0700, David Mosberger wrote:
> >>>>> On Fri, 13 May 2005 10:32:19 -0700, "Lu, Hongjiu" <hongjiu.lu@intel.com> said:
> 
>   HJ> I tried binutils 2.15.92.0.2 20040927 and got the same
>   HJ> result. What is the version of assembler which you think is
>   HJ> good?
> 
> The standard Debian/sarge as seems fine:
> 
>  $ as -v
>  GNU assembler version 2.15 (ia64-linux) using BFD version 2.15
> 

As I have mentioned before, it is an IPF specific kernel issue.
Basically, when you use weak functions, unwind info may be wrong.
I am enclosing an updated kernel patch.


H.J.

--- linux/arch/ia64/kernel/irq.c.weak	2005-05-10 13:52:00.000000000 -0700
+++ linux/arch/ia64/kernel/irq.c	2005-05-13 13:18:36.000000000 -0700
@@ -103,7 +103,9 @@ static char irq_redir [NR_IRQS]; // = { 
  * Arch specific routine for deferred write to iosapic rte to reprogram
  * intr destination.
  */
-void proc_set_irq_affinity(unsigned int irq, cpumask_t mask_val)
+void
+__comdatfunc (proc_set_irq_affinity)
+proc_set_irq_affinity(unsigned int irq, cpumask_t mask_val)
 {
 	pending_irq_cpumask[irq] = mask_val;
 }
--- linux/arch/ia64/kernel/smpboot.c.weak	2005-05-10 13:52:00.000000000 -0700
+++ linux/arch/ia64/kernel/smpboot.c	2005-05-13 13:18:57.000000000 -0700
@@ -404,7 +404,9 @@ start_secondary (void *unused)
 	return 0;
 }
 
-struct pt_regs * __devinit idle_regs(struct pt_regs *regs)
+struct pt_regs *
+__devinit __comdatfunc (idle_regs)
+idle_regs(struct pt_regs *regs)
 {
 	return NULL;
 }
--- linux/include/linux/compiler.h.weak	2005-05-10 13:52:00.000000000 -0700
+++ linux/include/linux/compiler.h	2005-05-13 13:20:14.000000000 -0700
@@ -155,4 +155,9 @@ extern void __chk_io_ptr(void __iomem *)
 #define __always_inline inline
 #endif
 
+#ifndef __comdatfunc
+#define __comdatfunc(f) \
+  __attribute__ ((__section__ (".gnu.linkonce.t." #f )))
+#endif
+
 #endif /* __LINUX_COMPILER_H */
--- linux/kernel/fork.c.weak	2005-05-10 13:52:00.000000000 -0700
+++ linux/kernel/fork.c	2005-05-13 13:19:40.000000000 -0700
@@ -1148,7 +1148,9 @@ bad_fork_free:
 	goto fork_out;
 }
 
-struct pt_regs * __devinit __attribute__((weak)) idle_regs(struct pt_regs *regs)
+struct pt_regs *
+__devinit __comdatfunc (idle_regs)
+idle_regs(struct pt_regs *regs)
 {
 	memset(regs, 0, sizeof(struct pt_regs));
 	return regs;
--- linux/kernel/irq/proc.c.weak	2005-05-10 13:52:00.000000000 -0700
+++ linux/kernel/irq/proc.c	2005-05-13 13:19:17.000000000 -0700
@@ -19,7 +19,8 @@ static struct proc_dir_entry *root_irq_d
  */
 static struct proc_dir_entry *smp_affinity_entry[NR_IRQS];
 
-void __attribute__((weak))
+void
+__comdatfunc (proc_set_irq_affinity)
 proc_set_irq_affinity(unsigned int irq, cpumask_t mask_val)
 {
 	irq_affinity[irq] = mask_val;

Re: heads up: apparent GAS bug in current (CVS) binutils

[David Mosberger]

>>>>> On Fri, 13 May 2005 13:50:04 -0700, "H. J. Lu" <hjl@lucon.org> said:

  HJ> On Fri, May 13, 2005 at 01:34:10PM -0700, David Mosberger wrote:
  >> >>>>> On Fri, 13 May 2005 10:32:19 -0700, "Lu, Hongjiu"
  >> <hongjiu.lu@intel.com> said:

  HJ> I tried binutils 2.15.92.0.2 20040927 and got the same
  HJ> result. What is the version of assembler which you think is
  HJ> good?
  >>  The standard Debian/sarge as seems fine:

  >> $ as -v GNU assembler version 2.15 (ia64-linux) using BFD version
  >> 2.15

  HJ> As I have mentioned before, it is an IPF specific kernel issue.
  HJ> Basically, when you use weak functions, unwind info may be
  HJ> wrong.  I am enclosing an updated kernel patch.

I agree that this is weird code that needs to be fixed, preferably by
not using weak functions at all (as Andreas suggested).

However, there still seems to be a binutils issues here: if this is
something binutils cannot properly support, it should issue an error,
not silently generate wrong code, no?

	--david

Re: heads up: apparent GAS bug in current (CVS) binutils

[H. J. Lu]

On Fri, May 13, 2005 at 01:59:06PM -0700, David Mosberger wrote:
> >>>>> On Fri, 13 May 2005 13:50:04 -0700, "H. J. Lu" <hjl@lucon.org> said:
> 
>   HJ> On Fri, May 13, 2005 at 01:34:10PM -0700, David Mosberger wrote:
>   >> >>>>> On Fri, 13 May 2005 10:32:19 -0700, "Lu, Hongjiu"
>   >> <hongjiu.lu@intel.com> said:
> 
>   HJ> I tried binutils 2.15.92.0.2 20040927 and got the same
>   HJ> result. What is the version of assembler which you think is
>   HJ> good?
>   >>  The standard Debian/sarge as seems fine:
> 
>   >> $ as -v GNU assembler version 2.15 (ia64-linux) using BFD version
>   >> 2.15
> 
>   HJ> As I have mentioned before, it is an IPF specific kernel issue.
>   HJ> Basically, when you use weak functions, unwind info may be
>   HJ> wrong.  I am enclosing an updated kernel patch.
> 
> I agree that this is weird code that needs to be fixed, preferably by
> not using weak functions at all (as Andreas suggested).
> 
> However, there still seems to be a binutils issues here: if this is
> something binutils cannot properly support, it should issue an error,
> not silently generate wrong code, no?

I will see what I can do.


H.J.

Re: heads up: apparent GAS bug in current (CVS) binutils

[David Mosberger]

>>>>> On Fri, 13 May 2005 14:01:11 -0700, "H. J. Lu" <hjl@lucon.org> said:

  >> However, there still seems to be a binutils issues here: if this
  >> is something binutils cannot properly support, it should issue an
  >> error, not silently generate wrong code, no?

  HJ> I will see what I can do.

BTW: Thanks for tracking this down!  I had no idea this weird
"overloading" was going on.

	--david

RE: heads up: apparent GAS bug in current (CVS) binutils

[Luck, Tony]

>+++ linux/arch/ia64/kernel/smpboot.c	2005-05-13 
>-struct pt_regs * __devinit idle_regs(struct pt_regs *regs)
>+struct pt_regs *
>+__devinit __comdatfunc (idle_regs)
>+idle_regs(struct pt_regs *regs)

>+++ linux/kernel/fork.c	2005-05-13 13:19:40.000000000 -0700
>-struct pt_regs * __devinit __attribute__((weak)) 
>idle_regs(struct pt_regs *regs)
>+struct pt_regs *
>+__devinit __comdatfunc (idle_regs)
>+idle_regs(struct pt_regs *regs)

So is there some deeper magic that I don't understand, or is this
wrong?  After this patch is applied, both of these definitions look
identical to me.  So how does the linker know which one to use?

-Tony

RE: heads up: apparent GAS bug in current (CVS) binutils

[David Mosberger]

>>>>> On Tue, 17 May 2005 13:48:51 -0700, "Luck, Tony" <tony.luck@intel.com> said:

  >> +++ linux/arch/ia64/kernel/smpboot.c	2005-05-13 
  >> -struct pt_regs * __devinit idle_regs(struct pt_regs *regs)
  >> +struct pt_regs *
  >> +__devinit __comdatfunc (idle_regs)
  >> +idle_regs(struct pt_regs *regs)

  >> +++ linux/kernel/fork.c	2005-05-13 13:19:40.000000000 -0700
  >> -struct pt_regs * __devinit __attribute__((weak)) 
  >> idle_regs(struct pt_regs *regs)
  >> +struct pt_regs *
  >> +__devinit __comdatfunc (idle_regs)
  >> +idle_regs(struct pt_regs *regs)

  Tony> So is there some deeper magic that I don't understand, or is this
  Tony> wrong?  After this patch is applied, both of these definitions look
  Tony> identical to me.  So how does the linker know which one to use?

Please do not apply this patch.  The bug is in the toolchain.
(The kernel may be ugly in this corner and we may want to fix
that by getting rid of the weak definitions but that issue is
entirely independent of the toolchain bug.)

	--david