Bug 14052 - binutils 2.22.52.0.2 breaks 3.3.x kernel on i686
Summary: binutils 2.22.52.0.2 breaks 3.3.x kernel on i686
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: 2.22
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-02 19:29 UTC by Jan Rękorajski
Modified: 2012-05-05 14:15 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments
A patch (812 bytes, patch)
2012-05-03 00:33 UTC, H.J. Lu
Details | Diff
A new patch (1.78 KB, patch)
2012-05-03 22:34 UTC, H.J. Lu
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Rękorajski 2012-05-02 19:29:41 UTC
On Wed, May 2, 2012 at 11:58 AM, Paweł Sikora <pluto@pld-linux.org> wrote:
> Hi,
>
> with the latest binutils release we're observing weird kernel oopses on *i686* architecture.
> i've compared the vmlinux and all kernel modules produced by binutils 2.22.52.0.{1,2}
> and the only vmlinux differs. afaics there's a diff in size of few elf sections
> (.rodata, .rel.rodata, __modver). is it expected or a potential bug?
>
> here're vmlinux binaries: http://pluto.agmk.net/b.tar.bz2
>

Reverting patches from this report: http://sourceware.org/bugzilla/show_bug.cgi?id=13621
(modulo changes to linker.c since then) seem to fix the problem.

We're using gcc 4.6.3 and the ooops here happens when something hits the swap.

The similiar problem was spotted in Arch linux https://bugs.archlinux.org/task/29694
and their solution was to remove the 13621 patch from arch binutils.
Comment 1 H.J. Lu 2012-05-02 19:48:39 UTC
The differences are

--- good/System.map	2012-05-02 12:43:53.775355097 -0700
+++ bad/System.map	2012-05-02 12:34:59.780776694 -0700
@@ -50740,7 +50740,7 @@ c0be2640 D dsa_packet_type
 c0be2680 D edsa_packet_type
 c0be26c0 D trailer_packet_type
 c0be2700 D _edata
-c0be3000 D __init_begin
+c0be3000 A __init_begin
 c0be3000 T _sinittext
 c0be3000 T i386_start_kernel
 c0be307d T reserve_ebda_region
@@ -55299,7 +55299,7 @@ c0c7d6c0 d cfd_data
 c0c7d700 d csd_data
 c0c7d740 D softnet_data
 c0c7d800 D __per_cpu_end
-c0c7e000 D __init_end
+c0c7e000 A __init_end
 c0c7e000 R __smp_locks
 c0c84000 B __bss_start
 c0c84000 R __smp_locks_end

The new linker puts __init_begin/__init_end in ABS section.
Comment 2 H.J. Lu 2012-05-02 22:12:29 UTC
Does your bootloader relocate the kernel? Absolute symbols
break the relocated kernel.  Linker may need to keep the
output section if there is a symbol defined for it.
Comment 3 Jan Rękorajski 2012-05-02 22:40:57 UTC
My bootloader is good old grub (0.97).
To tell the truth - I have no knowledge of its inner workings and no idea if it relocates the kernel :(
Comment 4 H.J. Lu 2012-05-03 00:33:11 UTC
Created attachment 6388 [details]
A patch

Please try this patch.
Comment 5 Alan Modra 2012-05-03 01:51:53 UTC
See http://sourceware.org/ml/binutils/2012-05/msg00019.html
I suspect your boot loader is broken.  However, this sort of problem is precisely why I put up some resistance to the pr13621 patch.
Comment 6 Alan Modra 2012-05-03 02:03:10 UTC
HJ, your patch is cunning enough but really no better than reverting the pr13621 change, since your patch will likely result in elflint complaints.
Comment 7 Allan McRae 2012-05-03 02:05:06 UTC
FYI, I have reports of the pr13621 patch causing kernel issues using both grub and syslinux.  Of course, both bootloaders could be broken...
Comment 8 Jan Rękorajski 2012-05-03 10:25:44 UTC
Add lilo to the list of broken bootloaders...

HJ, your patch does the job, my kernel is deep into swap and running fine.
Comment 9 H.J. Lu 2012-05-03 12:35:04 UTC
(In reply to comment #6)
> HJ, your patch is cunning enough but really no better than reverting the
> pr13621 change, since your patch will likely result in elflint complaints

My patch avoids ABS symbols only when they are defined in linker
script and passes the testcase for PR 13621.
Comment 10 Alan Modra 2012-05-03 13:01:21 UTC
HJ, you no doubt saw my attempt at trying to interpret the ELF spec to rationalize GNU ld generation of SHN_ABS symbols.  Cary demolished my argument.  So for one, my claim that there might be a bug in the kernel loaders in their treatment of SHN_ABS symbols is rubbish.  Also, it means that the pr13621 patch should be completely reverted, because that patch made GNU ld create SHN_ABS symbols from non-SHN_ABS input symbols.  Your patch prevents some specific cases of wrong SHN_ABS symbols, but we should be getting rid of all of them.
Comment 11 H.J. Lu 2012-05-03 13:47:35 UTC
(In reply to comment #10)
> HJ, you no doubt saw my attempt at trying to interpret the ELF spec to
> rationalize GNU ld generation of SHN_ABS symbols.  Cary demolished my argument.
>  So for one, my claim that there might be a bug in the kernel loaders in their
> treatment of SHN_ABS symbols is rubbish.  Also, it means that the pr13621 patch
> should be completely reverted, because that patch made GNU ld create SHN_ABS
> symbols from non-SHN_ABS input symbols.  Your patch prevents some specific
> cases of wrong SHN_ABS symbols, but we should be getting rid of all of them.

Let's revert the whole PR 13621 patch.  Thanks.
Comment 12 H.J. Lu 2012-05-03 22:34:15 UTC
Created attachment 6389 [details]
A new patch

This patch reverts the PR 13621 change and keeps the zero size
section if there is a symbol for it.
Comment 13 H.J. Lu 2012-05-04 15:37:54 UTC
(In reply to comment #12)
> Created attachment 6389 [details]
> A new patch
> 
> This patch reverts the PR 13621 change and keeps the zero size
> section if there is a symbol for it.

Please verify if this patch works.
Comment 14 Jan Rękorajski 2012-05-04 16:55:40 UTC
(In reply to comment #13)
> (In reply to comment #12)
> > Created attachment 6389 [details]
> > A new patch
> > 
> > This patch reverts the PR 13621 change and keeps the zero size
> > section if there is a symbol for it.
> 
> Please verify if this patch works.

Works for me. kernel 3.3.4 built with this patch runs fine.

FYI:
c15a3000 T __init_begin
c162d000 T __init_end
Comment 15 Sourceware Commits 2012-05-05 04:51:23 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	amodra@sourceware.org	2012-05-05 04:51:16

Modified files:
	bfd            : ChangeLog linker.c 
	ld/testsuite   : ChangeLog 
	ld/testsuite/ld-elf: warn2.d 
Removed files:
	ld/testsuite/ld-elf: zerosize1.d zerosize1.s 

Log message:
	PR ld/14052
	PR ld/13621
	bfd/
	* linker.c (_bfd_nearby_section): Revert 2012-02-13 change.
	ld/testsuite/
	* ld-elf/warn2.d: Revert 2012-02-13 change.
	* ld-elf/zerosize1.d, ld-elf/zerosize1.s: Delete.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/ChangeLog.diff?cvsroot=src&r1=1.5669&r2=1.5670
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/linker.c.diff?cvsroot=src&r1=1.94&r2=1.95
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ChangeLog.diff?cvsroot=src&r1=1.1518&r2=1.1519
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-elf/warn2.d.diff?cvsroot=src&r1=1.7&r2=1.8
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-elf/zerosize1.d.diff?cvsroot=src&r1=1.2&r2=NONE
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-elf/zerosize1.s.diff?cvsroot=src&r1=1.1&r2=NONE
Comment 16 Alan Modra 2012-05-05 04:52:02 UTC
Fixed.
Comment 17 Sourceware Commits 2012-05-05 14:15:26 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	hjl@sourceware.org	2012-05-05 14:15:21

Modified files:
	ld/testsuite   : ChangeLog 
Added files:
	ld/testsuite/ld-elf: pr14052.d pr14052.t 

Log message:
	Add a testcase for PR ld/14052
	
	PR ld/14052
	* ld-elf/pr14052.d: New file.
	* ld-elf/pr14052.t: Likewise.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ChangeLog.diff?cvsroot=src&r1=1.1519&r2=1.1520
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-elf/pr14052.d.diff?cvsroot=src&r1=NONE&r2=1.1
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-elf/pr14052.t.diff?cvsroot=src&r1=NONE&r2=1.1