Created attachment 6832 [details] proposed fix Since f2494eee (integrate nacl into gold) gold does unaligned memory accesses on armv5 and earlier when recognizing nacl files, due to unaligned allocation of Ehdr. (From Debian bug #696284): ehdr here is only 16-bit aligned (0x405a7536 % 4 == 2), which comes from #1 0x0008ce74 in Elf_file (file=0xbe89bfc8, this=0xbe89bf68) at ../../gold/../elfcpp/elfcpp_file.h:397 whch is using an offset into the open file. encountered building chromium, but I was unable to build anything without turning on unaligned fault fixup (echo 2 > /proc/cpu/alignment) invocation: gdb --args g++-4.6.real -pthread -Wl,-z,noexecstack -fPIC -Wl,-O1 -Wl,--as-needed -Wl,--gc-sections -o out/Release/mksnapshot -Wl,--start-group out/Release/obj.host/mksnapshot/v8/src/mksnapshot.o out/Release/obj.host/v8/tools/gyp/libv8_base.a out/Release/obj.host/v8/tools/gyp/libv8_nosnapshot.a -Wl,--end-group then: set follow-fork-mode child Dump of assembler code for function elfcpp::Elf_file<32, false, gold::Sniff_file>::construct(gold::Sniff_file*, elfcpp::Ehdr<32, false> const&): 0x0002f4a4 <+0>: ldr r3, [r2] 0x0002f4a8 <+4>: push {r4, r5, r6, r7, r8, lr} 0x0002f4ac <+8>: ldrh r7, [r3, #40] ; 0x28 => 0x0002f4b0 <+12>: ldr r6, [r3, #32] 0x0002f4b4 <+16>: ldrh r12, [r3, #48] ; 0x30 0x0002f4b8 <+20>: mov r4, r2 0x0002f4bc <+24>: ldrh r2, [r3, #50] ; 0x32 0x0002f4c0 <+28>: mov r8, #0 0x0002f4c4 <+32>: cmp r7, #52 ; 0x34 0x0002f4c8 <+36>: mov r5, r1 0x0002f4cc <+40>: str r1, [r0] 0x0002f4d0 <+44>: str r6, [r0, #8] 0x0002f4d4 <+48>: str r8, [r0, #12] 0x0002f4d8 <+52>: str r12, [r0, #16] 0x0002f4dc <+56>: str r2, [r0, #20] 0x0002f4e0 <+60>: str r8, [r0, #24] (gdb) info registers r0 0xbe89bf70 3196698480 r1 0xbe89bfd0 3196698576 r2 0xbe89bff8 3196698616 r3 0x405a7536 1079670070 r4 0x536 1334 r5 0x0 0 r6 0x20 32 r7 0x34 52 r8 0x27d0dc 2609372 r9 0x0 0 r10 0x536 1334 r11 0x0 0 r12 0x0 0 sp 0xbe89bf20 0xbe89bf20 lr 0x8ce74 577140 pc 0x2f4b0 0x2f4b0 <elfcpp::Elf_file<32, false, gold::Sniff_file>::construct(gold::Sniff_file*, elfcpp::Ehdr<32, false> const&)+12> cpsr 0x60000010 1610612752 (gdb) bt full #0 elfcpp::Elf_file<32, false, gold::Sniff_file>::construct (this=0xbe89bf70, file=0xbe89bfd0, ehdr=...) at ../../gold/../elfcpp/elfcpp_file.h:378 No locals. #1 0x0008ce74 in Elf_file (file=0xbe89bfc8, this=0xbe89bf68) at ../../gold/../elfcpp/elfcpp_file.h:397 No locals. #2 do_recognize_nacl_file<32, false> (offset=1334, input_file=<optimized out>, this=0x27d0dc) at ../../gold/nacl.h:198 file = {file_ = @0xd14a30, offset_ = 1334} elf_file = {static ehdr_size = <optimized out>, static phdr_size = <optimized out>, static shdr_size = <optimized out>, static sym_size = <optimized out>, static rel_size = <optimized out>, static rela_size = <optimized out>, file_ = 0x0, shoff_ = 3845902709115484, shnum_ = 13716016, shstrndx_ = 13712800, large_shndx_offset_ = 13716016} shnum = <optimized out> #3 recognize_nacl_file (offset=1334, input_file=<optimized out>, this=0x27d0dc) at ../../gold/nacl.h:182 No locals. #4 gold::Target_selector_nacl<{anonymous}::Target_selector_arm<false>, {anonymous}::Target_arm_nacl<false> >::do_recognize(gold::Input_file *, off_t, int, int, int) (this=0x27d0dc, file=<optimized out>, offset=<optimized out>, machine=40, osabi=0, abiversion=0) at ../../gold/nacl.h:116 No locals. #5 0x001e9470 in recognize (abiversion=0, osabi=32, machine=2622660, offset=1334, input_file=0xd14a28, this=0x27d0dc) at ../../gold/target-select.h:83 No locals. #6 gold::select_target (input_file=0xd14a28, offset=<optimized out>, machine=40, size=32, is_big_endian=false, osabi=0, abiversion=0) at ../../gold/target-select.cc:114 ret = <optimized out>
It looks like that fix will work fine. But it is far from obvious that it's what's required or if it's the ideal thing. The comment on File_read::get_view says the flag "is true if the data must be naturally aligned". What the fileread.cc code actually does is ensure that it is aligned to the word-size of the ELF file (i.e. only ever to 32 bits or to 64 bits). What "naturally aligned" means to me is aligned to the size of the datum in question, so in get_view it looks like it would require it to be aligned to the SIZE parameter. That would be unnecessarily large alignment for ELFCLASS64 (Elf64_Ehdr is 64 bytes) and nonsensical for ELFCLASS32 (Elf32_Ehdr is 52 bytes, not a power of two). So I think the comment on File_read::get_view in fileread.h should be changed to be more clear about the meaning of its ALIGNED parameter. That aside, it's not at all clear to me across all those layers where the alignment requirement comes from. The elfcpp code is a bit light on comments, but I don't see any place that it states there are alignment requirements for the pointers that File::View::data returns. If that is the intent, then it should be stated clearly somewhere in the interface comments. I think the scenario hitting this must be looking at files in an archive. I can't think of any other situation in which the start of the ELF header could ever be misaligned. This makes me wonder if every other place in gold that's using elfcpp code with its implicit assumption of aligned header data has correctly indicated in its View implementations that the alignment must be ensured.
Created attachment 6839 [details] alternate fix
Yes, in this case "naturally aligned" means aligned to the size from the ELF class. That's the alignment that matters to gold. I'm fine with changing the comment. There are two kinds of alignment requirement in gold. First, as you say, files in archives can be misaligned when mmapped into memory. Second, many relocations must be aligned; in this case the required alignment depends on the size of the relocation. The elfcpp library requires that ELF data structures be aligned. The patch in the original comment looks right to me. It corresponds to the code in is_elf_object. The patch in comment #2 is not correct.
Comment on attachment 6839 [details] alternate fix Then can you push the first patch?
Mentioned this to you today at Tools Cauldron. Ping.
CVSROOT: /cvs/src Module name: src Changes by: ccoutant@sourceware.org 2013-07-15 16:49:20 Modified files: gold : ChangeLog fileread.h nacl.h Log message: 2013-07-15 Shawn Landden <shawnlandden@gmail.com> gold/ PR gold/15070 * fileread.h (File_read::get_view): Clarify comment about ALIGNED. * nacl.h (Sniff_file::View::View): Request aligned view. Patches: http://sourceware.org/cgi-bin/cvsweb.cgi/src/gold/ChangeLog.diff?cvsroot=src&r1=1.1090&r2=1.1091 http://sourceware.org/cgi-bin/cvsweb.cgi/src/gold/fileread.h.diff?cvsroot=src&r1=1.49&r2=1.50 http://sourceware.org/cgi-bin/cvsweb.cgi/src/gold/nacl.h.diff?cvsroot=src&r1=1.1&r2=1.2
I've applied Shawn's patch and updated the comment in fileread.h. -cary