12931 2011-06-24 19:29:00 +0000 ARM: gas fails to set the proper alignment on code sections, causing broken output 2011-07-01 04:19:31 +0000 1 1 1 Unclassified binutils gas 2.22 All All RESOLVED FIXED P2 normal --- 1 dave.martin unassigned hp mgretton nickc 49475 dave.martin 2011-06-24 19:29:40 +0000 Whenever an instruction is assembled, gas should increase the alignment of the output section. Currently the output section alignment is often left untouched, which may result in invalid alignment when sections are merged at link-time. This appears to be responsible for a faulty WebM codec behaviour when firefox-5 is built in Thumb, as a result of ARM code being linked into .text alongside Thumb code. The ARM instructions are placed at halfword alignment and so can't be executed correctly. I don't know if this is a recent regression or an older bug. It appears present in trunk as of 2011-06-24. The bug is also present in linaro binutils 2.21.0.20110327-2ubuntu2cross1.62 In both of the cases below, the .text section alignment should not be 2**0. The alignment should probably be set to 2**2 (though it could in principle be 2**1 for some Thumb code, some Thumb instructions are alignment-sensitive modulo 1 word; the 16-bit PC-relative add and ldr instructions have this restriction). $ cat <<EOF >tst-align.s .type f, %function .globl f f: nop EOF $ arm-linux-gnueabi-as -o tst-align.o tst-align.s $ arm-linux-gnueabi-objdump -hd tst-align.o tst-align.o: file format elf32-littlearm Sections: Idx Name Size VMA LMA File off Algn 0 .text 00000004 00000000 00000000 00000034 2**0 CONTENTS, ALLOC, LOAD, READONLY, CODE 1 .data 00000000 00000000 00000000 00000038 2**0 CONTENTS, ALLOC, LOAD, DATA 2 .bss 00000000 00000000 00000000 00000038 2**0 ALLOC 3 .ARM.attributes 00000016 00000000 00000000 00000038 2**0 CONTENTS, READONLY Disassembly of section .text: 00000000 <f>: 0: e1a00000 nop ; (mov r0, r0) $ arm-linux-gnueabi-as mthumb $ arm-linux-gnueabi-objdump -hd tst-align.o tst-align.o: file format elf32-littlearm Sections: Idx Name Size VMA LMA File off Algn 0 .text 00000002 00000000 00000000 00000034 2**0 CONTENTS, ALLOC, LOAD, READONLY, CODE 1 .data 00000000 00000000 00000000 00000036 2**0 CONTENTS, ALLOC, LOAD, DATA 2 .bss 00000000 00000000 00000000 00000036 2**0 ALLOC 3 .ARM.attributes 00000016 00000000 00000000 00000036 2**0 CONTENTS, READONLY Disassembly of section .text: 00000000 <f>: 0: 46c0 nop ; (mov r8, r8) 49476 dave.martin 2011-06-24 19:30:19 +0000 For details of the Firefox/WebM bug, see: https://bugs.launchpad.net/ubuntu/+source/firefox/+bug/789198 49506 5824 dave.martin 2011-06-28 12:57:37 +0000 Created attachment 5824 possible fix The attached patch hooks into mapping_state() to mark the alignment requirement on the current output section whenever we start emitting instructions. It might be safer to make the alignment 4 for Thumb code as well as ARM code; the current patch gives Thumb code an alignment of 2 bytes, based on the principles that (a) this makes the current situation no worse, and (b) all realistic situations where a Thumb code section requires 4-byte alignment will involve an explicit .align or literal pool somewhere, so we don't need to worry. 49549 cvs-commit 2011-06-29 16:29:43 +0000 CVSROOT: /cvs/src Module name: src Changes by: nickc@sourceware.org 2011-06-29 16:29:38 Modified files: gas : ChangeLog gas/config : tc-arm.c Log message: PR gas/12931 * config/tc-arm.c (mapping_state): When changing to ARM or THUMB state set the minimum required alignment of the section. Patches: http://sourceware.org/cgi-bin/cvsweb.cgi/src/gas/ChangeLog.diff?cvsroot=src&r1=1.4527&r2=1.4528 http://sourceware.org/cgi-bin/cvsweb.cgi/src/gas/config/tc-arm.c.diff?cvsroot=src&r1=1.495&r2=1.496 49550 nickc 2011-06-29 16:33:44 +0000 Hi Dave, Thanks for reporting this problem and providing a patch to fix it too. I think that 2 byte alignment should be fine for thumb-code-containing sections, but if someone does come up with a scenario where this assumption does not work then we can always fix the assembler again. Cheers Nick 49551 dave.martin 2011-06-29 16:47:08 +0000 On Wed, Jun 29, 2011 at 5:33 PM, nickc at redhat dot com <sourceware-bugzilla@sourceware.org> wrote: > http://sourceware.org/bugzilla/show_bug.cgi?id=12931 > > Nick Clifton <nickc at redhat dot com> changed: > >           What    |Removed                     |Added > ---------------------------------------------------------------------------- >             Status|NEW                         |RESOLVED >                 CC|                            |nickc at redhat dot com >         Resolution|                            |FIXED > > --- Comment #4 from Nick Clifton <nickc at redhat dot com> 2011-06-29 16:33:44 UTC --- > Hi Dave, > >  Thanks for reporting this problem and providing a patch to fix it too.  I > think that 2 byte alignment should be fine for thumb-code-containing sections, > but if someone does come up with a scenario where this assumption does not work > then we can always fix the assembler again. That seems reasonable -- I think the proposed fix should be a useful interim step, since it's pretty simple and makes the situation no worse. Did the patch look sensible to you? I'm no gas hacker... Cheers ---Dave 49552 nickc 2011-06-29 16:53:42 +0000 Hi Dave, > Did the patch look sensible to you? I'm no gas hacker... Are you sure ? ... The patch was fine. :-) Cheers Nick 49579 cvs-commit 2011-06-30 13:07:24 +0000 CVSROOT: /cvs/src Module name: src Changes by: nickc@sourceware.org 2011-06-30 13:07:21 Modified files: gas/testsuite : ChangeLog gas/testsuite/gas/arm: blx-bad.d inst-po-be.d inst-po.d Log message: PR gas/12931 * gas/arm/blx-bad.d: Add exrta nop at end of disassembly. * gas/arm/inst-po-be.d: Add exrta nop at end of disassembly. * gas/arm/inst-po.d: Add exrta nop at end of disassembly. Patches: http://sourceware.org/cgi-bin/cvsweb.cgi/src/gas/testsuite/ChangeLog.diff?cvsroot=src&r1=1.1925&r2=1.1926 http://sourceware.org/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/blx-bad.d.diff?cvsroot=src&r1=1.2&r2=1.3 http://sourceware.org/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/inst-po-be.d.diff?cvsroot=src&r1=1.1&r2=1.2 http://sourceware.org/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/inst-po.d.diff?cvsroot=src&r1=1.1&r2=1.2 49607 hp 2011-07-01 04:19:31 +0000 Nick (being already CC:ed on the referred PR) or anyone else ARM-knowledgeable, pretty please have a quick look to see if the fix for this bug was the proper fix for PR sim/12737. Thanks. 5824 2011-06-28 12:57:00 +0000 2011-06-28 12:57:37 +0000 possible fix align-code-sections.diff text/plain 1259 dave.martin ZGlmZiAtLWdpdCBhL2dhcy9jb25maWcvdGMtYXJtLmMgYi9nYXMvY29uZmlnL3RjLWFybS5jCmlu ZGV4IDFkOTY0YzMuLmYwNGMzY2YgMTAwNjQ0Ci0tLSBhL2dhcy9jb25maWcvdGMtYXJtLmMKKysr IGIvZ2FzL2NvbmZpZy90Yy1hcm0uYwpAQCAtMjU4OSw3ICsyNTg5LDIzIEBAIG1hcHBpbmdfc3Rh dGUgKGVudW0gbXN0YXRlIHN0YXRlKQogICAgIC8qIFRoZSBtYXBwaW5nIHN5bWJvbCBoYXMgYWxy ZWFkeSBiZWVuIGVtaXR0ZWQuCiAgICAgICAgVGhlcmUgaXMgbm90aGluZyBlbHNlIHRvIGRvLiAg Ki8KICAgICByZXR1cm47Ci0gIGVsc2UgaWYgKFRSQU5TSVRJT04gKE1BUF9VTkRFRklORUQsIE1B UF9EQVRBKSkKKworICBpZiAoc3RhdGUgPT0gTUFQX0FSTSB8fCBzdGF0ZSA9PSBNQVBfVEhVTUIp CisgICAgLyogQWxsIEFSTSBpbnN0cnVjdGlvbnMgcmVxdWlyZSA0LWJ5dGUgYWxpZ25tZW50Lgor ICAgICAgIChBbG1vc3QpIGFsbCBUaHVtYiBpbnN0cnVjdGlvbnMgcmVxdWlyZSAyLWJ5dGUgYWxp Z25tZW50LgorCisgICAgICAgV2hlbiBlbWl0dGluZyBpbnN0cnVjdGlvbnMgaW50byBhbnkgc2Vj dGlvbiwgbWFyayB0aGUgc2VjdGlvbgorICAgICAgIGFwcHJvcHJpYXRlbHkuCisKKyAgICAgICBT b21lIFRodW1iIGluc3RydWN0aW9ucyBhcmUgYWxpZ25tZW50LXNlbnNpdGl2ZSBtb2R1bG8gNCBi eXRlcywKKyAgICAgICBidXQgdGhlbXNlbHZlcyByZXF1aXJlIDItYnl0ZSBhbGlnbm1lbnQ7IHRo aXMgYXBwbGllcyB0byBzb21lCisgICAgICAgUEMtIHJlbGF0aXZlIGZvcm1zLiAgSG93ZXZlciwg dGhlc2UgY2FzZXMgd2lsbCBpbnZvdmxlIGltcGxpY2l0CisgICAgICAgbGl0ZXJhbCBwb29sIGdl bmVyYXRpb24gb3IgYW4gZXhwbGljaXQgLmFsaWduID49MiwgYm90aCBvZgorICAgICAgIHdoaWNo IHdpbGwgY2F1c2UgdGhlIHNlY3Rpb24gdG8gbWUgbWFya2VkIHdpdGggc3VmZmljaWVudAorICAg ICAgIGFsaWdubWVudC4gIFRodXMsIHdlIGRvbid0IGhhbmRsZSB0aG9zZSBjYXNlcyBoZXJlLiAq LworICAgIHJlY29yZF9hbGlnbm1lbnQgKG5vd19zZWcsIHN0YXRlID09IE1BUF9BUk0gPyAyIDog MSk7CisKKyAgaWYgKFRSQU5TSVRJT04gKE1BUF9VTkRFRklORUQsIE1BUF9EQVRBKSkKICAgICAv KiBUaGlzIGNhc2Ugd2lsbCBiZSBldmFsdWF0ZWQgbGF0ZXIgaW4gdGhlIG5leHQgZWxzZS4gICov CiAgICAgcmV0dXJuOwogICBlbHNlIGlmIChUUkFOU0lUSU9OIChNQVBfVU5ERUZJTkVELCBNQVBf QVJNKQo=